Skip to content

re-acquire global lock for transactions from scheduled fns - #114

Merged
ianmacartney merged 7 commits into
mainfrom
ian/serialize-scheduled-functions
May 13, 2026
Merged

re-acquire global lock for transactions from scheduled fns#114
ianmacartney merged 7 commits into
mainfrom
ian/serialize-scheduled-functions

Conversation

@ianmacartney

@ianmacartney ianmacartney commented May 12, 2026

Copy link
Copy Markdown
Member

Currently a scheduled function uses setTimeout, which can fire mid-way through a transaction if real timers are used.

And scheduling multiple functions can also race. This is because they detect the current global transaction lock and infer they're a nested transaction call.

This PR waits until the nested transaction has finished before starting the next scheduled function, allowing it to use the global transaction lock to serialize scheduled mutations, regardless of where they're scheduled from or whether real timers are used or not.

Fixes #112

Closes #82

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pkg-pr-new

pkg-pr-new Bot commented May 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/convex-test@114

commit: 1377e41

@ianmacartney
ianmacartney requested a review from Nicolapps May 12, 2026 05:00
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: get-convex/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d24c9574-cabe-470f-801d-7f0cec3d8bd4

📥 Commits

Reviewing files that changed from the base of the PR and between 90cffdb and 7be113a.

📒 Files selected for processing (1)
  • index.ts

📝 Walkthrough

Walkthrough

This PR replaces DatabaseFake’s listener-based scheduled-function completion with a per-test Scheduler that tracks fired scheduled-callback promises. setTimeout callbacks now run outside nested-transaction ALS via nestedTxStorage.exit and register their execution promises; scheduled-function state transitions are performed inline. finishInProgressScheduledFunctions and finishAllScheduledFunctions now drain the Scheduler while coordinating timer advancement. Added scheduler test helpers in convex/scheduler.ts and refactored scheduler tests to centralize fake/real timer lifecycle.

Sequence Diagram(s)

sequenceDiagram
  participant Test as Test Code
  participant Timer as setTimeout
  participant ALS as nestedTxStorage
  participant Scheduler as Scheduler
  participant DB as _scheduled_functions
  participant UDF as Scheduled UDF

  Test->>DB: schedule job (pending)
  DB->>Timer: setTimeout fires
  Timer->>ALS: exit() then invoke callback
  ALS->>Scheduler: add(executionPromise)
  Scheduler->>DB: read job row and patch pending->inProgress
  Scheduler->>UDF: invoke scheduled UDF
  UDF->>DB: patch inProgress->success | inProgress->failed
  UDF-->>Scheduler: executionPromise settles
  Scheduler->>Scheduler: remove(executionPromise)
  Test->>Scheduler: finishInProgressScheduledFunctions() awaits drain
Loading

Possibly Related PRs

  • get-convex/convex-test#77: Modifies index.ts scheduling and execution context handling—touches the same scheduling/control-flow code paths.
  • get-convex/convex-test#69: Adds nestedTxStorage/AsyncLocalStorage and modifies TransactionManager to support nested execution impacting scheduled-callback ALS interactions.
  • get-convex/convex-test#78: Adjusts finishAllScheduledFunctions and timer handling for orphaned-scheduled-function cleanup and overlaps with drain/timer coordination logic.

Suggested Reviewers

  • Nicolapps
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: re-acquiring the global transaction lock for scheduled functions, which directly addresses the core issue of transaction state corruption when scheduled functions run.
Description check ✅ Passed The description explains the problem (scheduled functions inheriting transaction locks, causing races), the solution (waiting for nested transactions to finish), and links to specific issues (#112, #82), all relevant to the changeset.
Linked Issues check ✅ Passed The PR addresses both linked issues: #112 by preventing stale transaction state inheritance via explicit control of scheduled function execution timing, and #82 by implementing explicit scheduling via finishInProgressScheduledFunctions() and tracking in-flight executions without relying solely on setTimeout.
Out of Scope Changes check ✅ Passed All changes are focused on the core objective: test refactoring to separate real/fake timer cases, new scheduler functions for testing, and rewriting scheduled execution tracking in index.ts to prevent transaction state inheritance through explicit async-local-storage exit before scheduling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ian/serialize-scheduled-functions

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Nicolapps Nicolapps left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! This sounds much more simple than the previous fix

@ianmacartney
ianmacartney merged commit 9ab0ef2 into main May 13, 2026
2 of 3 checks passed
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.

finishInProgressScheduledFunctions with real timers can corrupt transaction state for scheduled functions

2 participants