Skip to content

Revert "improvement(db): add session statement/lock timeouts; simplif…#4599

Merged
TheodoreSpeaks merged 1 commit into
stagingfrom
revert/pg-lock
May 14, 2026
Merged

Revert "improvement(db): add session statement/lock timeouts; simplif…#4599
TheodoreSpeaks merged 1 commit into
stagingfrom
revert/pg-lock

Conversation

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator

Summary

Forgot pg lock didn't work with pscale. Reverting.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

  • Revert, no code changes

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)

Screenshots/Videos

@vercel
Copy link
Copy Markdown

vercel Bot commented May 14, 2026

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 14, 2026 5:19pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 14, 2026

PR Summary

Medium Risk
Changes transactional behavior around knowledge-base document creation and workspace archival, which can affect concurrency/locking and error modes under load. Also removes global Postgres timeout safeguards, potentially allowing longer-running or stuck queries.

Overview
Reverts the DB-level lock_timeout/statement_timeout configuration in packages/db/db.ts, removing the session-wide guardrails that cancelled long-running/blocked queries.

Reworks knowledge-base document creation (createDocumentRecords and createSingleDocument) to drop the atomic insertDocumentsIfKbAlive JSONB insert helper and instead run inside a transaction that SELECT ... FOR UPDATE locks the knowledge_base row, re-checks deleted_at, then inserts documents and updates knowledge_base.updated_at.

Removes the per-transaction SET LOCAL statement_timeout/lock_timeout overrides from workspace archival (archiveWorkspace) and updates the related lifecycle test mock accordingly.

Reviewed by Cursor Bugbot for commit 32d5dc4. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 14, 2026

Greptile Summary

This PR reverts the previous "improvement(db)" commit that added PostgreSQL-specific session-level lock_timeout/statement_timeout connection parameters, SET LOCAL timeout overrides inside transactions, and a jsonb_to_recordset-based atomic insert function — none of which are supported on pscale. It restores the prior approach of using db.transaction() with SELECT ... FOR UPDATE row locks.

  • packages/db/db.ts: Removes connection: { lock_timeout: 5000, statement_timeout: 30000 } from the postgres client config, which were PostgreSQL-only session params.
  • apps/sim/lib/workspaces/lifecycle.ts: Drops SET LOCAL statement_timeout and SET LOCAL lock_timeout from the workspace archival transaction.
  • apps/sim/lib/knowledge/documents/service.ts: Removes the insertDocumentsIfKbAlive helper (which used jsonb_to_recordset::jsonb and INSERT…SELECT…WHERE EXISTS) and reverts both createDocumentRecords and createSingleDocument to a transaction + FOR UPDATE pattern.

Confidence Score: 4/5

Safe to merge once the missing processingStatus field in createSingleDocument is confirmed to have a database-level default.

The revert is clean and consistent across all four files. The only uncertainty is whether the document.processing_status column carries a DB-level default of 'pending'createSingleDocument no longer sets it explicitly while createDocumentRecords still does, and a missing default would cause every single-document insert to fail with a NOT NULL violation.

apps/sim/lib/knowledge/documents/service.ts — verify processing_status column default in the schema before merging.

Important Files Changed

Filename Overview
packages/db/db.ts Removes PostgreSQL-specific connection: { lock_timeout, statement_timeout } session-level params that aren't supported on pscale; restores clean postgres client config.
apps/sim/lib/workspaces/lifecycle.ts Removes SET LOCAL statement_timeout and SET LOCAL lock_timeout calls inside the archive transaction; these are PostgreSQL-only and were causing failures on pscale.
apps/sim/lib/workspaces/lifecycle.test.ts Removes the execute: vi.fn() mock from the transaction stub, consistent with lifecycle.ts no longer calling tx.execute() after the SET LOCAL commands are removed.
apps/sim/lib/knowledge/documents/service.ts Reverts from insertDocumentsIfKbAlive (PostgreSQL-specific jsonb_to_recordset + INSERT...SELECT...WHERE EXISTS) back to db.transaction() with SELECT ... FOR UPDATE; createSingleDocument no longer explicitly sets processingStatus: 'pending' — relies on a DB-level default.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant service.ts
    participant DB as DB (transaction)

    Note over service.ts, DB: createDocumentRecords / createSingleDocument (reverted)

    Caller->>service.ts: createDocumentRecords(docs, kbId)
    service.ts->>DB: BEGIN TRANSACTION
    service.ts->>DB: "SELECT 1 FROM knowledge_base WHERE id=kbId FOR UPDATE"
    service.ts->>DB: "SELECT id FROM knowledge_base WHERE id=kbId AND deleted_at IS NULL"
    alt KB not found / soft-deleted
        DB-->>service.ts: []
        service.ts-->>Caller: throw Error("Knowledge base not found")
    else KB alive
        service.ts->>DB: INSERT INTO document VALUES (...)
        service.ts->>DB: "UPDATE knowledge_base SET updated_at=now"
        DB-->>service.ts: COMMIT
        service.ts-->>Caller: DocumentData[]
    end
Loading

Reviews (1): Last reviewed commit: "Revert "improvement(db): add session sta..." | Re-trigger Greptile

Comment on lines 1300 to 1313
@@ -1430,21 +1307,31 @@ export async function createSingleDocument(
chunkCount: 0,
tokenCount: 0,
characterCount: 0,
processingStatus: 'pending',
enabled: true,
uploadedAt: now,
...processedTags,
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Missing processingStatus in createSingleDocument

createDocumentRecords explicitly sets processingStatus: 'pending' as const, but createSingleDocument no longer does after this revert — ...processedTags only carries tag/number/date/boolean fields. If the document table column lacks a DEFAULT 'pending' at the DB level, every single-document insert will fail with a NOT NULL violation. Worth verifying the schema default exists, or restoring the explicit field here for symmetry.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 32d5dc4. Configure here.

}

const newDocument: NewDocumentRow = {
const newDocument = {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing processingStatus in single document creation path

Low Severity

The createSingleDocument function no longer sets processingStatus: 'pending' in its newDocument object (lines 1300–1313), while the bulk createDocumentRecords path still explicitly includes processingStatus: 'pending' as const (line 816). The database column has a DEFAULT 'pending' so the insert succeeds, but the two code paths are now inconsistent — and the object returned to API callers from the single-document path will be missing the processingStatus property in the JSON response.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 32d5dc4. Configure here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Lets do a full revert ill follow up after.

@TheodoreSpeaks TheodoreSpeaks merged commit b1a87d5 into staging May 14, 2026
14 checks passed
@TheodoreSpeaks TheodoreSpeaks deleted the revert/pg-lock branch May 14, 2026 17:42
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