Prevent duplicate server-minted state timestamps#22512
Prevent duplicate server-minted state timestamps#22512devin-ai-integration[bot] wants to merge 3 commits into
Conversation
On coarse-resolution clocks (notably Windows, ~15.6ms) the server can mint identical timestamps for consecutive states of a run, violating the unique (run_id, timestamp) constraint and surfacing as a 409 IntegrityError. Add an EnsureStateTimestampIsMonotonic global universal transform, applied first in COMMON_GLOBAL_TRANSFORMS so it runs before the transforms that read proposed_state.timestamp. When a proposed state's timestamp collides with the run's current state, nudge it forward one microsecond; explicitly backdated states are left untouched. Covers flow and task runs uniformly. Closes #22511
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
desertaxle
requested review from
chrisguidry,
cicdw,
desertaxle and
zzstoatzz
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9968c92662
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Make EnsureStateTimestampIsMonotonic bump a colliding proposed timestamp past the run's most recent state instead of only deduping against the immediately preceding state. This fixes the 3+-states-in-one-tick case (t -> t -> t) flagged in review, where the third state was previously treated as backdated and still collided with the first persisted state. A fast path returns early when the proposed timestamp is already strictly newer than the current state, so normal forward transitions incur no extra query. Non-colliding backdated timestamps are still preserved.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cec34ca67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RemoveResumingIndicator, | ||
| ] | ||
| return ( | ||
| [EnsureFlowRunStateTimestampIsUnique] |
There was a problem hiding this comment.
When task-run states are set through the normal /task_runs/{id}/set_state API, the request model does not carry a timestamp and the route converts it to a full State, so the timestamp is still minted server-side. This new transform is only prepended to GlobalFlowPolicy; GlobalTaskPolicy still runs only COMMON_GLOBAL_TRANSFORMS(), leaving rapid task transitions on coarse-clock hosts able to create duplicate (task_run_id, timestamp) rows and hit the same IntegrityError this change is meant to avoid.
Useful? React with 👍 / 👎.
| ) | ||
| assert latest_timestamp is not None | ||
|
|
||
| proposed_state.timestamp = latest_timestamp + timedelta(microseconds=1) |
There was a problem hiding this comment.
When an explicitly backdated flow state collides with an old persisted state after the run already has newer states, this assignment moves the proposed state to max(existing) + 1µs. For example, with persisted states at t0 and t10, proposing a historical state at t0 becomes t10 + 1µs, so state-history reads ordered by timestamp and the run timing fields record it as the newest transition rather than a backdated one. The transform should choose the next free timestamp adjacent to the collided historical timestamp instead of always using the latest state.
Useful? React with 👍 / 👎.
| colliding_timestamp = await context.session.scalar( | ||
| sa.select(orm_models.FlowRunState.timestamp) | ||
| .where( | ||
| orm_models.FlowRunState.flow_run_id == context.run.id, | ||
| orm_models.FlowRunState.timestamp == proposed_state.timestamp, |
There was a problem hiding this comment.
For the common case where a proposed flow state is strictly newer than the current state, this new collision check still performs an extra flow_run_state lookup on every transition. These state-setting paths are hot for normal orchestration, so adding a database round trip even when a collision is impossible can regress production throughput; gate the query behind the initial_state/timestamp fast path described by the transform.
Useful? React with 👍 / 👎.
1 participant
closes #22511
this PR prevents duplicate server-minted flow-run state timestamps from violating the unique
(flow_run_id, timestamp)index on coarse-resolution clocks.Details
Flow-run state transitions are serialized under the flow-run row lock. A new flow-only global transform performs an indexed lookup for the exact proposed timestamp before timestamp-dependent bookkeeping runs. When a collision exists, it moves the proposed timestamp to one microsecond after the latest persisted state.
The lookup checks the full state history instead of comparing only with the current state, so it also handles runs whose current state was explicitly backdated. Non-colliding backdated timestamps remain unchanged.
The transform runs before
SetRunStateTimestamp,SetStartTime,SetEndTime, andIncrementRunTime, keeping the persisted state and denormalized runtime fields consistent.Tests cover three transitions in one clock tick, a collision with an older historical state after a backdated current state, and timestamp-derived start/end/runtime bookkeeping.
Checklist
flow_run_state.timestamp(server mints coarse timestamps) #22511"Link to Devin session: https://app.devin.ai/sessions/e5441eea296d4ccabca962261cebe43c