Repository navigation
bound the incoming cursor by the snapshot timestamp - #233
Merged
Merged
Conversation
The loop capped its incoming cursor at the newest commit stamp it had observed, assembled from the previous run's own commit (lastCommitTs), the completions and cancelations it read, ready pendingStart documents, and the sweep. Telling a ready document's commit stamp from a scheduled one's wall-clock key needed a `scheduled` marker on every entry. The snapshot timestamp is the frontier those approximated: everything stamped at or below it is visible, everything committing later is stamped above it. Read it once in getBatch and cap the cursor there. Drop lastCommitTs, the scheduled flag, and the stamp collection; the guard against moving the cursor backwards is no longer reachable. The scanTs sweep is unchanged: it rescues documents that are visible but keyed below the cursor, which no bound on unseen commits can cover. snapshotTs() calls the 1.0/getSnapshotTs syscall directly until a released convex exposes ctx.meta.getSnapshotTs(). convex-test comes from a pkg.pr.new build that answers the syscall.
The segment scan read up to the end of the current wall-clock millisecond and the cursor was then capped at the snapshot. Reading up to the snapshot instead means every key the scan sees is at or below it, so the cursor can rest on the last handled key and the cap goes away. Ready work is eligible as soon as it is visible, whatever the two clocks do relative to each other. Scheduled work and retries now come due by the commit clock. The keys the loop writes itself, a retry's backoff and a legacy entry's re-key, are raised to at least the snapshot, since the cursor can now reach it within the same run. endOfMs is gone with the wall-clock bound; a sub-millisecond runAt is stored exactly and starts within the next millisecond. The test drivers commit before each iteration so the fake clock reaches convex-test's snapshot the way batch-worker's own commits do in production.
The loop already raised the keys it writes (retry backoffs, legacy re-keys) to at least its snapshot. Scheduled enqueues now do the same with theirs, so every writer follows one rule: no key below its writer's snapshot. Ready enqueues satisfy it already, since the commit stamp is above the snapshot by definition. This doesn't replace the scanTs sweep, because an enqueue in flight while the loop reads can still commit a key below the loop's cursor. It does bound how far behind such a key can land to that in-flight window, rather than to however far the wall clock trails the commit clock.
convex-test@0.0.57-alpha.0 answers the 1.0/getSnapshotTs syscall, replacing the pkg.pr.new build.
getBatch and run share a transaction, so the snapshot timestamp is the same in both; carrying it in the batch and threading it through handleCompletions, rescheduleJob, and promoteScheduled added parameters without information. Each site reads it directly, and the segment scan explains why it is the eligibility bound.
Member
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
commit: |
Keep the intent the code can't show — why the bound is the snapshot, why the loop's own keys are raised to it — and drop restatements and claims about how other modules behave. The syscall override in the loop tests goes too: with convex-test's snapshot advancing only on commit, a test gets a snapshot below a due wall-clock key by not committing after advancing the fake clock.
The read timestamp only advances on commits (and on a max_repeatable bump that is hours apart while idle), so a wake-up for scheduled work in a quiet deployment could find a stale snapshot, start nothing, and wait for its own commit to move the clock. Bound eligibility by the later of the snapshot and the wall clock instead: ready work still starts on sight, and scheduled work starts as soon as either clock says it's due. The cursor now stops at the snapshot rather than the last handled key, since a wall-clock key can sit above every existing commit stamp. With the snapshot free to read, that cap is one comparison. The test drivers no longer commit before each iteration to carry the fake clock into the snapshot.
ianmacartney
marked this pull request as ready for review
September 9, 2026 02:13
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/component/stats.ts">
<violation number="1" location="src/component/stats.ts:87">
P2: When a pre-0.4.9 future-scheduled entry remains pending during reporting, this bound counts it as backlog because its 100ms bucket is below the nanosecond eligibility bound. Exclude future legacy buckets or count them only when `legacyRunAt(segment) <= Date.now()`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| q | ||
| .gte("segment", state.segmentCursors.incoming) | ||
| .lt("segment", endOfMs(Date.now())), | ||
| .lte("segment", eligibilityBound()), |
There was a problem hiding this comment.
P2: When a pre-0.4.9 future-scheduled entry remains pending during reporting, this bound counts it as backlog because its 100ms bucket is below the nanosecond eligibility bound. Exclude future legacy buckets or count them only when legacyRunAt(segment) <= Date.now().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/component/stats.ts, line 87:
<comment>When a pre-0.4.9 future-scheduled entry remains pending during reporting, this bound counts it as backlog because its 100ms bucket is below the nanosecond eligibility bound. Exclude future legacy buckets or count them only when `legacyRunAt(segment) <= Date.now()`.</comment>
<file context>
@@ -77,14 +77,14 @@ export async function generateReport(
q
.gte("segment", state.segmentCursors.incoming)
- .lt("segment", endOfMs(Date.now())),
+ .lte("segment", eligibilityBound()),
)
.paginate({
</file context>
Member
Author
There was a problem hiding this comment.
this is only a temporary scenario until it catches up, right? I'm tempted to just document it as such. we already fall back onto this definition of backlog in the calculateBacklogAndReport function
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Not reviewed (too large): example/benchmarks/2026-09-08.json (~5,865 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
This was referenced Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bound the incoming cursor by the snapshot timestamp
The loop capped its incoming cursor at the newest commit stamp it had
observed, assembled from the previous run's own commit (lastCommitTs), the
completions and cancelations it read, ready pendingStart documents, and the
sweep. Telling a ready document's commit stamp from a scheduled one's
wall-clock key needed a
scheduledmarker on every entry.The snapshot timestamp is the frontier those approximated: everything
stamped at or below it is visible, everything committing later is stamped
above it. Read it once in getBatch and cap the cursor there. Drop
lastCommitTs, the scheduled flag, and the stamp collection; the guard
against moving the cursor backwards is no longer reachable.
The scanTs sweep is unchanged: it rescues documents that are visible but
keyed below the cursor, which no bound on unseen commits can cover.
snapshotTs() calls the 1.0/getSnapshotTs syscall directly until a released
convex exposes ctx.meta.getSnapshotTs(). convex-test comes from a pkg.pr.new
build that answers the syscall.
make the snapshot the eligibility bound
The segment scan read up to the end of the current wall-clock millisecond
and the cursor was then capped at the snapshot. Reading up to the snapshot
instead means every key the scan sees is at or below it, so the cursor can
rest on the last handled key and the cap goes away. Ready work is eligible
as soon as it is visible, whatever the two clocks do relative to each other.
Scheduled work and retries now come due by the commit clock. The keys the
loop writes itself, a retry's backoff and a legacy entry's re-key, are
raised to at least the snapshot, since the cursor can now reach it within
the same run.
endOfMs is gone with the wall-clock bound; a sub-millisecond runAt is stored
exactly and starts within the next millisecond. The test drivers commit
before each iteration so the fake clock reaches convex-test's snapshot the
way batch-worker's own commits do in production.
raise scheduled enqueue keys to the enqueuer's snapshot
The loop already raised the keys it writes (retry backoffs, legacy re-keys)
to at least its snapshot. Scheduled enqueues now do the same with theirs,
so every writer follows one rule: no key below its writer's snapshot. Ready
enqueues satisfy it already, since the commit stamp is above the snapshot
by definition.
This doesn't replace the scanTs sweep, because an enqueue in flight while
the loop reads can still commit a key below the loop's cursor. It does
bound how far behind such a key can land to that in-flight window, rather
than to however far the wall clock trails the commit clock.
depend on the published convex-test alpha
convex-test@0.0.57-alpha.0 answers the 1.0/getSnapshotTs syscall, replacing
the pkg.pr.new build.
read the snapshot where it's used
getBatch and run share a transaction, so the snapshot timestamp is the same
in both; carrying it in the batch and threading it through handleCompletions,
rescheduleJob, and promoteScheduled added parameters without information.
Each site reads it directly, and the segment scan explains why it is the
eligibility bound.