Skip to content

perf(db): logs-list index, drop redundant indexes, replica routing, hot-path write cleanups#5105

Merged
waleedlatif1 merged 2 commits into
stagingfrom
chore/db-contention-hardening
Jun 17, 2026
Merged

perf(db): logs-list index, drop redundant indexes, replica routing, hot-path write cleanups#5105
waleedlatif1 merged 2 commits into
stagingfrom
chore/db-contention-hardening

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Add a workflow_execution_logs (workspace_id, started_at DESC NULLS LAST, id DESC) index so the logs-list cursor query (WHERE workspace_id = ? ORDER BY started_at DESC, id DESC LIMIT n) runs as an index-only scan with LIMIT early-exit instead of a sort. The existing (workspace_id, started_at) index is kept — it still serves the ascending/range log queries the new DESC index does not cover.
  • Drop three redundant indexes, each a left-prefix of an existing superset: permission_group_workspace_group_id_idx, user_table_rows_table_id_idx, workspace_byok_workspace_idx.
  • Route heavy read-only list queries to the read replica: /api/v1/logs, admin audit-logs, admin organizations (GET only — POST stays on the primary).
  • Make updateWebhookProviderConfig a single atomic jsonb merge instead of SELECT-then-UPDATE (preserves the shallow-merge plus null/undefined semantics exactly).
  • Only write copilot_chats.last_seen_at when the chat is actually unread, avoiding per-read rewrites of the same row.
  • Remove dead getFileMetadataByContext.

Type of Change

  • Improvement / performance

Testing

  • Added unit tests for the last_seen_at unread guard and the webhook atomic merge.
  • bun run check:api-validation passes; biome clean.
  • The index migration uses CREATE/DROP INDEX CONCURRENTLY (replay-safe). Recommend applying on a non-prod branch first and watching the dropped indexes' query latency during rollout.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Jun 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 17, 2026 2:03am

Request Review

@cursor

cursor Bot commented Jun 17, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Replica reads may show slightly stale admin lists; the CONCURRENTLY index migration and dropped indexes need rollout monitoring, but application logic changes are narrow and tested.

Overview
Database-focused performance work: a CONCURRENTLY migration adds a descending (workspace_id, started_at, id) index on workflow_execution_logs for paginated logs-list queries and drops three redundant prefix indexes; admin audit-log and organization GET list queries move to dbReplica while writes stay on the primary.

Mark chat read now updates last_seen_at only when the chat is unread (last_seen_at null or older than updated_at), cutting redundant row writes. Webhook polling replaces SELECT-then-UPDATE on provider_config with a single atomic JSONB merge (including key removal for undefined values). Dead getFileMetadataByContext is removed from upload metadata helpers.

Unit tests cover the unread guard on mothership chat read and the atomic webhook config merge.

Reviewed by Cursor Bugbot for commit d763062. Configure here.

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR delivers a focused set of database performance improvements: a new DESC composite index for the logs-list cursor query, removal of three redundant prefix indexes, and replica routing for the two admin read-only list endpoints. Two hot-path write operations are also tightened — updateWebhookProviderConfig is converted to an atomic JSONB merge, and the copilot_chats.last_seen_at update is now gated by an unread predicate.

  • Migration (0239_wel_logs_desc_index_and_redundant_drops.sql): All operations use CREATE/DROP INDEX CONCURRENTLY IF NOT EXISTS/IF EXISTS, which is online and replay-safe. The COMMIT at the top correctly drops out of any Drizzle batch transaction so the CONCURRENTLY ops run in autocommit. lock_timeout is temporarily set to 0 for the index operations and restored to 5s afterward.
  • Replica routing: Both admin routes routed to dbReplica (audit-logs GET, organizations GET) are gated by withAdminAuth, which is a pure static safeCompare(header, env) check with no database query — confirmed in auth.ts. Neither carries a permission join, so replication lag carries no authorization risk. The /api/v1/logs route that was previously flagged was correctly reverted to primary.
  • Atomic JSONB merge: The new updateWebhookProviderConfig eliminates the SELECT-then-UPDATE race and preserves the original null/undefined semantics exactly: null values merge into the stored JSON, undefined values trigger a JSONB key-deletion (-) operator.

Confidence Score: 5/5

Safe to merge. All changes are additive or eliminate races; the migration is online-safe and idempotent; no auth boundary is weakened.

The admin replica routing is sound — withAdminAuth is a pure constant-time env comparison (safeCompare) with zero DB involvement, confirmed in auth.ts, and neither handler carries a permission join. The atomic JSONB merge in updateWebhookProviderConfig eliminates a SELECT-then-UPDATE race and preserves the original null/undefined semantics. The lastSeenAt unread guard is logically correct and covered by new tests. The migration uses CONCURRENTLY IF NOT EXISTS/IF EXISTS throughout and is idempotent for safe replay.

No files require special attention.

Important Files Changed

Filename Overview
packages/db/migrations/0239_wel_logs_desc_index_and_redundant_drops.sql New migration adding a DESC composite index for the logs-list query and dropping three redundant prefix indexes; all operations use CONCURRENTLY + IF NOT EXISTS/IF EXISTS, replay-safe.
packages/db/schema.ts Schema updated to reflect new DESC composite index on workflowExecutionLogs and removal of three dropped indexes; consistent with the migration SQL.
apps/sim/lib/webhooks/polling/utils.ts updateWebhookProviderConfig converted from SELECT-then-UPDATE to an atomic JSONB merge; null/undefined semantics are preserved correctly.
apps/sim/app/api/mothership/chats/read/route.ts lastSeenAt write now gated by an unread predicate (IS NULL OR lastSeenAt < updatedAt), avoiding redundant row rewrites for already-read chats.
apps/sim/app/api/v1/admin/audit-logs/route.ts GET handler switched to dbReplica; safe because withAdminAuth is a pure static key comparison with no DB query and no permission join in the handler.
apps/sim/app/api/v1/admin/organizations/route.ts GET switched to dbReplica; POST correctly stays on primary db. Same admin-key-only auth reasoning applies.
apps/sim/lib/uploads/server/metadata.ts Dead getFileMetadataByContext function and its FileMetadataQueryOptions interface removed cleanly; function was unexported and had no callers.
apps/sim/lib/webhooks/polling/utils.test.ts New unit tests verify the atomic JSONB merge behavior including null preservation and undefined key removal.
apps/sim/app/api/mothership/chats/read/route.test.ts New unit tests verify the unread predicate guard on the lastSeenAt write and the unauthenticated early-exit path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming Request] --> B{Route}

    B --> C["POST /api/v1/admin/organizations"]
    B --> D["GET /api/v1/admin/organizations\nGET /api/v1/admin/audit-logs"]
    B --> E["POST /api/mothership/chats/read"]
    B --> F["Webhook polling\nupdateWebhookProviderConfig"]

    C --> G["withAdminAuth\nsafeCompare header vs env\nno DB query"]
    D --> G

    G -->|authenticated| H["Primary db — POST writes"]
    G -->|authenticated| I["dbReplica — GET list queries\nno permission join"]

    E --> J{isAuthenticated?}
    J -->|no| K["401 Unauthorized"]
    J -->|yes| L{"lastSeenAt IS NULL\nOR lastSeenAt < updatedAt?"}
    L -->|no — already read| M["200 no-op — no DB write"]
    L -->|yes — unread| N["UPDATE lastSeenAt\nPrimary db"]

    F --> O{"any undefined keys\nin configUpdates?"}
    O -->|no| P["COALESCE merge only\nSingle atomic UPDATE"]
    O -->|yes| Q["COALESCE merge\nminus removedKeys\nSingle atomic UPDATE"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Incoming Request] --> B{Route}

    B --> C["POST /api/v1/admin/organizations"]
    B --> D["GET /api/v1/admin/organizations\nGET /api/v1/admin/audit-logs"]
    B --> E["POST /api/mothership/chats/read"]
    B --> F["Webhook polling\nupdateWebhookProviderConfig"]

    C --> G["withAdminAuth\nsafeCompare header vs env\nno DB query"]
    D --> G

    G -->|authenticated| H["Primary db — POST writes"]
    G -->|authenticated| I["dbReplica — GET list queries\nno permission join"]

    E --> J{isAuthenticated?}
    J -->|no| K["401 Unauthorized"]
    J -->|yes| L{"lastSeenAt IS NULL\nOR lastSeenAt < updatedAt?"}
    L -->|no — already read| M["200 no-op — no DB write"]
    L -->|yes — unread| N["UPDATE lastSeenAt\nPrimary db"]

    F --> O{"any undefined keys\nin configUpdates?"}
    O -->|no| P["COALESCE merge only\nSingle atomic UPDATE"]
    O -->|yes| Q["COALESCE merge\nminus removedKeys\nSingle atomic UPDATE"]
Loading

Reviews (3): Last reviewed commit: "fix(logs): keep /api/v1/logs on primary ..." | Re-trigger Greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2b541ce. Configure here.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Thanks — addressed the auth-on-replica concern at the source rather than waving it off.

/api/v1/logs is reverted to the primary db (commit d763062). You were right: its innerJoin permissions is the authorization gate — checkWorkspaceScope only validates API-key scope (workspace-key match / personal-key allowance), not workspace membership. So that query must run on the primary.

The two routes still on dbReplica (/api/v1/admin/audit-logs, /api/v1/admin/organizations GET) are safe under a consistent rule: route a read to the replica only when authorization is enforced separately on the primary and the query carries no auth filter. Both use withAdminAuth, which is a pure static admin-key check (safeCompare(header, env)) — no DB auth, enforced before the query — and neither query has a permission join or read-your-writes.

@greptile review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d763062. Configure here.

@waleedlatif1 waleedlatif1 merged commit a82b44d into staging Jun 17, 2026
16 checks passed
@waleedlatif1 waleedlatif1 deleted the chore/db-contention-hardening branch June 17, 2026 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant