Skip to content

Commit source sync state and jobs atomically - #1451

Open
Floating-Y wants to merge 1 commit into
deeplethe:devfrom
Floating-Y:fix/atomic-source-sync
Open

Floating-Y wants to merge 1 commit into
deeplethe:devfrom
Floating-Y:fix/atomic-source-sync

Conversation

@Floating-Y

Copy link
Copy Markdown
Contributor

Why

If a sync_source insertion fails after mark_queued commits, the source stays queued without a task and ordinary sync requests and the due-source scheduler cannot schedule it again.

What changes

  • Commit the conditional source status update, job insertion, and existing worker notification in one store transaction, reusing jobs::enqueue_with_max_attempts_tx. The payload and three-attempt budget remain the same.
  • Use the atomic operation for initial, manual, and scheduled sync. Keep a separate Drive operation that adds a follow-up task even when the source is queued/running, while preserving independent folder-config saves and running status.
  • Add five PostgreSQL regressions using the existing fixture and an insertion-failure trigger scoped to marked test sources: HTTP error/queued responses, rollback and retry, concurrent ordinary requests, initial sync, and Drive follow-up behavior.

Existing orphan states need separate recovery. Read-only candidates are sources marked queued/running with no matching active sync_source job; check job history and active workers before changing them. This PR does not reset existing states or change checkpoints, sync completion, or retry policy.

How it was checked

Validated the exact diff committed as d9bb2c8 on Windows with Rust 1.98.0 and a fresh disposable PostgreSQL 16/pgvector database with all 106 migrations. UTOPIA_TEST_REQUIRE_DB=1 and UTOPIA_TEST_REQUIRE_PDFTOTEXT=1 were set.

  • Before the fix, two fault-injection tests reproduced queued with zero jobs; the two existing-behavior tests passed. All five final source regressions ran and passed, including the real HTTP router.
  • cargo fmt --all --check and git diff --check: passed.
  • cargo clippy --workspace --all-targets -- -D warnings: passed.
  • cargo test --locked --workspace --no-fail-fast -- --test-threads=1: exit 0; the harness reported 1,746 OK, zero failures, and six pre-existing ignored tests. This includes the five source regressions and the three existing worker-notification regressions.
  • pnpm install --frozen-lockfile --offline, pnpm build, and pnpm test --maxWorkers=2: passed; 327 frontend tests across 48 files. node --test scripts/bench/lib.test.mjs: 15 passed, none skipped.

Harness OK includes optional early-return cases: eight MySQL/ClickHouse/Trino live tests were not run because their service URLs were not configured. Optional real Drive/Notion/S3/Azure/GCS/WebDAV probes and the gated large benchmark are also not claimed as executed integration coverage.

The full run used standalone Poppler 26.09.0 with its CJK data and process-only localhost proxy bypass. One harness thread avoided unrelated wall-clock contention; concurrent source requests and embedding batches still ran inside their tests. Both previously failing PDF GBK and embedding timing tests passed in the final full run, with their source and assertions unchanged.

Before review

  • Every commit is signed off (git commit -s).
  • Format, clippy, and the complete workspace test command above pass.
  • SQL under crates/utopia-store/ was tested with UTOPIA_DATABASE_URL set and UTOPIA_TEST_REQUIRE_DB=1.

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>

@WaylandYang WaylandYang 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.

Thanks, the fix is right. Please retitle to a sentence stating the behaviour, e.g. "A source's queued status and its sync job commit together, so a failed job insert no longer leaves the source stuck as queued". Please also keep only the failure/rollback regressions (the concurrent and Drive follow-up cases cover unchanged behaviour), drop the trigger and function when the test finishes instead of leaving them on jobs, and write the test comments in Chinese.

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.

2 participants