Skip to content

fix(knowledge): release kept connector documents in the background so removing a large source cannot time out - #8165

Merged
waleedlatif1 merged 4 commits into
stagingfrom
fix/connector-remove-busy
Sep 22, 2026
Merged

waleedlatif1 merged 4 commits into
stagingfrom
fix/connector-remove-busy

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Removing a connector while keeping its documents timed out on any sizeable source and showed "Connection is busy. Try removing it again in a moment." Every retry hit the same timeout
  • Cause: the request transaction deleted the connector row, ON DELETE SET NULL nulled connector_id on every document, and projection_source_acl_sync then rewrote every enabled chunk in embedding_search and embedding_keyword_tin. Those rows are indexed on connector_id, so none of the updates could be HOT; each one re-inserted into every index. All of it had to finish inside the 10 s statement_timeout
  • Keep-documents now works like delete-documents: one short transaction admits storage against the quota, sets the new detached_at, disables both syncs, and queues a knowledge.connector.detach outbox event. The request writes no document
  • The detach worker handles the next 100 documents per transaction. It nulls at most 250 of their projection rows per table first, and flips those documents only once none of their rows still names the connector, so the trigger has nothing left to rewrite. Each flipped page is billed in its own transaction, so a document is billable exactly when it no longer names a connector. When nothing references the connector, it drains history and member rows and deletes the row, using removeDrainedConnector, which is shared with the deletion worker
  • connectorIsLive() now also requires detached_at IS NULL. A detached connector can't be synced, listed, updated, or removed again, while document visibility still checks only archived_at/deleted_at, so kept documents stay readable and searchable throughout
  • The keep path's knowledge-base lock drops from FOR UPDATE to FOR SHARE. FOR UPDATE conflicted with the FOR KEY SHARE that every in-flight document and chunk write holds, so even small sources could fail the 5 s lock_timeout
  • Knowledge-base restore no longer sets a detached connector back to active
  • Migration 0376 adds the nullable detached_at column: additive, no default, metadata-only
  • The knowledge integration runner now installs the projection source/ACL trigger (script 0021), so integration tests exercise the same trigger production runs

Type of Change

  • Bug fix

Testing

  • New unit tests for the detach worker: paging, a large document released across several transactions, billing and rollback, outdated events, cancellation. Updated tests for the request path
  • Knowledge integration suite run against real Postgres with the production triggers installed. A new test releases a document with 600 chunks and checks that no search row still names the source. search-source-progress and storage-accounting pass. Two failures remain locally (confluence-enrollment, the filename-search paging test); both also fail on unmodified staging against the same database
  • bun run type-check, bun run lint, check:audits (47), check:migrations origin/staging, docs-manifest:check

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 Sep 22, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 22, 2026 10:33pm UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

All reported issues were addressed across 26 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/lib/knowledge/orchestration/connectors.ts Outdated
Comment thread apps/sim/lib/knowledge/service.ts
Comment thread apps/sim/lib/knowledge/connectors/deletion.ts
Comment thread apps/sim/lib/knowledge/connectors/queue.ts Outdated
@greptile-apps

greptile-apps Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no outstanding correctness, security, migration, or repository-rule issue was identified.

Summary

This PR moves connector document retention out of the request transaction and into bounded, durable outbox processing while preserving storage accounting and access behavior.

  • Adds a detached connector lifecycle and migration-backed reservation state.
  • Releases projection rows and documents in bounded transactions before deleting the drained connector.
  • Carries detach reservations through payer transfers, workspace moves, and storage reconciliation.
  • Excludes detached connectors from synchronization, management, restoration, credential access, and connector listings.
  • Extends unit and PostgreSQL integration coverage for paging, billing, cancellation, projection cleanup, and large documents.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Remove connector and keep documents] --> B[Lock connector and billing state]
  B --> C[Charge retained bytes]
  C --> D[Set detached_at and reservation]
  D --> E[Queue detach outbox event]
  E --> F[Release bounded projection rows]
  F --> G{Projection rows remain?}
  G -- Yes --> F
  G -- No --> H[Detach bounded document page]
  H --> I[Consume reservation]
  I --> J{Documents remain?}
  J -- Yes --> F
  J -- No --> K[Drain history and member rows]
  K --> L[Settle remaining reservation]
  L --> M[Delete connector]
Loading

Reviews (4) · Last reviewed commit: "test(workspaces): add knowledge_connecto..."

Comment thread apps/sim/lib/knowledge/orchestration/connectors.ts Outdated
Comment thread apps/sim/lib/knowledge/connectors/detachment.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

All reported issues were addressed across 28 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/lib/knowledge/connectors/member-access.ts
Comment thread apps/sim/lib/knowledge/orchestration/connectors.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

No issues found across 31 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

No issues found across 32 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 7823118 into staging Sep 22, 2026
34 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/connector-remove-busy branch September 22, 2026 22:45

This branch was previously deployed

1 inactive deployment
Preview d6f3923b Deployed Sep 22, 2026 by vercel[bot]
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