Makes vqueues creation explicit and decouple scheduler cache from state machine#4617
Makes vqueues creation explicit and decouple scheduler cache from state machine#4617AhmedSoliman merged 1 commit intomainfrom
Conversation
![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: 943b8e4b96
ℹ️ 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".
| #[bilrost(tag(1))] | ||
| pub(crate) created_at: MillisSinceEpoch, | ||
| /// The time spend in the queue before the first attempt to run. Measured by EMA of time | ||
| /// from initial scheduled run time to first "dequeue/start". | ||
| #[bilrost(tag(1))] | ||
| #[bilrost(tag(2))] |
There was a problem hiding this comment.
The new created_at field is assigned tag(1) while avg_queue_duration_ms is moved to tag(2), which renumbers an existing serialized field in VQueueStatistics. Because vqueue metadata is persisted and decoded on restart/scan, older on-disk records encoded with the previous tag layout can be misdecoded (or fail decoding when old timestamp fields are interpreted as u32), breaking upgrade compatibility for existing queues. New fields should use new tag numbers without changing prior tags.
Useful? React with 👍 / 👎.
| else { | ||
| error!("State mutation request was ignored because the vqueue {qid:?} does not exist!"); | ||
| // todo: When/if we made state mutations rpc-like, we should return the error to the | ||
| // user here. | ||
| return Ok(()); |
There was a problem hiding this comment.
This branch silently ignores external state mutations when no vqueue exists (return Ok(()) after logging). In practice, Command::PatchState for an idle service/object can now be lost entirely instead of being serialized through a newly created vqueue (the previous behavior), causing user-visible state updates to disappear. The handler should create the vqueue (or apply directly / return an explicit error) rather than acknowledging and dropping the mutation.
Useful? React with 👍 / 👎.
1 participant
Stack created with Sapling. Best reviewed with ReviewStack.