Repository navigation
volmgr: move ten groups out of executor.rs (pure move) - #564
Merged
Merged
Conversation
ac-pplx
force-pushed
the
ac/split-volmgr-executor-1
branch
from
October 16, 2026 06:47
b660681 to
b7f2a9f
Compare
executor.rs is the largest production file in the workspace. Besides the transfer executor it holds the page-cache writeback coordinator, the decode and decrypt sinks, sparse-shadow comparison, replica provenance, cgroup codec sizing and the pull path. Move those, which no open pull request edits, into child modules: codec_concurrency, artifact_role, transfer_metrics, sparse_shadow, replica_provenance, stream_pull_timing, writeback_coordinator, stream_writeback, decode_sinks and pull. Payload, crypto staging, the artifact job and push stay for a later step. Generated by scripts/split_rust_module.py (#562) from the spec in the pull request. Items move verbatim. The only edits keep each item's reach: 210 private items, fields or methods become pub(super). The parent re-exports the moved items, so no caller changes.
ac-pplx
force-pushed
the
ac/split-volmgr-executor-1
branch
from
October 16, 2026 07:11
b7f2a9f to
2302bb2
Compare
ac-pplx
marked this pull request as ready for review
October 16, 2026 14:49
mabdelraouf-pplx
approved these changes
Oct 16, 2026
Collaborator
ReviewNo findings. Verified against What I ran:
What I didn't check: whether the grouping is the right one. That's a design call, and I took the body's word for it. I also didn't rebuild or retest the crate; CI is green on this head. |
the-gigi-pplx
approved these changes
Oct 16, 2026
the-gigi-pplx
left a comment
Collaborator
There was a problem hiding this comment.
Approved. No findings; details in the review comment above.
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.
Summary
crates/libs/vol/mgr/server/src/executor.rs(4,862 code lines) is the largest production file in the workspace. Besides the transfer executor it holds:This moves ten of those groups into child modules. It avoids every item an open PR edits, and the items next to them, so the payload, crypto staging, artifact job and push stay for a later step.
executor.rsdrops to 1,706 code lines; the largest new file ispull.rsat 662. ElevenTransferExecutormethods move intoimpl TransferExecutorblocks in the modules that own them.This is a pure move, generated by
scripts/split_rust_module.py(#562) from the spec below and proved by itsverify. It is part of splitting the largest production modules into smaller ones with one job each.What moved where
codec_concurrencyartifact_roletransfer_metricssparse_shadowreplica_provenancestream_pull_timingwriteback_coordinatorstream_writebackdecode_sinkspullexecutor.rs(parent)How to check it
The spec (item names, not lines):
The tool is
scripts/split_rust_module.pyfrom #562 (not on main yet).b9fc395e0, runsplit_rust_module.py apply spec.toml && cargo fmt --all. This reproduces the first commit.split_rust_module.py verify origin/main→regenerated and compared 11 files from 1 split parents: identical.verifyderives the spec from the diff, regenerates the split from the merge base, and requires HEAD's Rust files to equal the rustfmt'd result byte for byte.pub(super); 11 methods moved intoimplblocks with the same header. Each keeps the reach the item had: one module deeper,pub(super)reaches exactly the old module.Observable changes
Log targets: 28
tracingevents without an explicittarget:now log under the new module paths (pplx_space_volmgr::executor::<module>instead ofpplx_space_volmgr::executor). Nothing in the repository selects on these targets exactly.Smoke harness:
space_smokematches the log message "transfer job completed", whichexecute_singleemits; it stays inexecutor.rs.Diagnostics name new locations:
expect,unwrap,unreachable!, asserts) in moved code name the new file and line;code.filepath,code.linenoandcode.namespace;std::any::type_nameof a moved type names its new module;The reviews found nothing in the repository that depends on these.
Validation
Run on a Linux development machine at the PR head:
cargo check --workspace --all-targets --all-features --locked: pass, no warnings.run_clippy_checkswith-D warnings: pass.codec_concurrencyuses nothing from its parent, so it keepsuse super::*;under#[allow(unused_imports)](the spec'sallow_unused_glob).cargo check -p pplx-space-volmgr --no-default-features --all-targets: pass.cargo nextest run -p pplx-space-volmgr --all-features --run-ignored allwith Postgres andSPACE_CI_REQUIRE_TEST_DEPS=1: 219 of 220 passed, including all 116executor::tests.staging::tests::nocow_upload_and_shadow_staging_keep_reflinks. It needs a loop-backed btrfs, which this revision's machine lacked, so it did not run. It passed as root on the first version of this PR, andstagingis untouched; CI's privileged job runs it.file_sizeentry below.Interactions
Open PRs touching
executor.rs: #346, #351 (draft) and #139. Their hunks were mapped onto current main by content. Every item they edit stays inexecutor.rs, so they rebase without conflicts in moved code: the payload,TransferEncryption, crypto staging, the artifact job, push,with_store/new,TransferExecutorConfigand theJobExecutorimpl.Follow-ups, kept out of this PR because
verifyallows no other Rust edits: imports inexecutor.rsthat only the child modules use could move to them.The archcheck
file_sizebaseline entry for this file is now stale (an advisory finding). Split PRs leave that table alone, so they don't conflict with each other there; one follow-up prunes the stale entries after they land.Rollback
Revert the commit(s). No data, configuration, wire or public API change.
🤖 Generated with Claude Code