Skip to content

[worker] Funnel all command proposals through LeaderState::run#5055

Draft
MohamedBassem wants to merge 2 commits into
mainfrom
pr5055
Draft

[worker] Funnel all command proposals through LeaderState::run#5055
MohamedBassem wants to merge 2 commits into
mainfrom
pr5055

Conversation

@MohamedBassem

@MohamedBassem MohamedBassem commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

…tead of taking actions inline

As part of the work on centeralizes writes to the self proposer inside one scheduler, this PR refactors the RPC handlers to return their decisions to the main PP loop where they get executed there. This is in prep for shipping the proposals to a channel in the next PR.

This PR is mostly mechanical and does the following:
1. Changes the handlers to no longer take the "Actuator" trait and instead return their `Decision` from the function.
2. The decision can be either: Propose a command (with its different modes), reply now, or the pre-vqueues invoker poking.

Tests are much simpler as they no longer need to mock the leadership state. Only meaningful change was a bug in the restart_as_new test `copy_prefix_zero_with_no_version_uses_service_invocation_command` where it was using the default `patch_deployment_id` and it didn't actually hit the assertions inside the mock handlers as it was failing with `CannotPatchDeploymentId`. The tests where not setting expectations on num calls to the mock, so it didn't fail when the call to `append_and_respond_asynchronously` didn't happen. So this was fixed along the way by using `PatchDeploymentId::PinToLatest` to hit the correct assertions. This PR anyways removes `mockall` altogether from that worker crate.
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

Test Results

  8 files  ±0    8 suites  ±0   4m 56s ⏱️ -3s
 61 tests ±0   61 ✅ ±0  0 💤 ±0  0 ❌ ±0 
268 runs  ±0  268 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b7b8cf3. ± Comparison against base commit 061157b.

♻️ This comment has been updated with latest results.

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.

1 participant