fix(transfer): interleave itemize in flist order (sync incremental)#6568
Merged
Conversation
The sync incremental-flist receiver driver run_pipelined_incremental emitted itemize (-i) rows in two batches - every directory (.d) row from its inline directory-creation loop, then every file (>f) row from the transfer pass - instead of upstream's single flist-index-order walk where a directory row immediately precedes its children. #6560 fixed this for run_pipelined by adding defer_itemize / emit_or_record_itemize / flush_itemize_rows: itemize rows are buffered keyed by flist index and drained in index order just before finalization. The candidate and pipeline record sites already route through that helper, but the incremental driver's own per-directory loop still called emit_itemize directly and never flushed, so it stayed in batch order. Route the incremental driver through the same mechanism: enable defer_itemize, record each directory row under its flist index via emit_or_record_itemize, and flush in index order before finalize. The async incremental path is intentionally out of scope (default-off).
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.
Problem
The receiver emits itemize (
-i) rows in the wrong order versus upstream rsync 3.4.4. Upstream itemizes in a single flist-index-order walk (generate_files,generator.c:2329-2344->recv_generator->itemize,generator.c:1480-1483), so a directory row lands immediately before its children's rows. For the flista/ a/f1 b/ b/f2upstream prints:oc's two-phase receiver instead emitted every directory (
.d) row first, then every file (>f) row:Prior fix
PR #6560 fixed this for the
run_pipelinedreceiver driver by adding a deferral mechanism:defer_itemize(a context flag),emit_or_record_itemize(buffers a row keyed by its flist index instead of writing it immediately), andflush_itemize_rows(drains theBTreeMapin ascending index order just before finalization). The candidate-selection and pipeline record sites already route throughemit_or_record_itemize.However, the sync incremental-flist driver
run_pipelined_incrementalwas not wired to it: its inline per-directory creation loop still calledemit_itemizedirectly and it never flushed, so it kept emitting dir-block-then-file-block.This change
Extends the exact same #6560 mechanism to the sync incremental driver:
self.defer_itemize = trueat driver start (mirrorsrun_pipelined).emit_or_record_itemize, keyed by the entry's flist index (obtained via.enumerate()), instead ofemit_itemize.flush_itemize_rowsbeforefinalize_transferto drain in flist-index order.No new interleave logic is introduced - it reuses the shared helper (DRY). The driver's responsibility is unchanged; only itemize emission is routed through the shared deferral path.
Scope
run_pipelined_incremental) is touched.run_pipelined_incremental_async,run_sync_async, any*_async) are intentionally out of scope: those receiver paths are default-off and gated.Test
Adds
incremental_deferred_itemize_rows_interleave_in_flist_index_order, mirroring #6560'srun_pipelinedtest but driving the incremental driver's own record sites (thecreate_directory_incrementalloop plusbuild_files_to_transfer). It asserts the buffered rows are keyed by flist index and drain interleaved (.d a/,>f a/f1,.d b/,>f b/f2), not batched. It fails if the batch emission returns.Validation
cargo fmt -p transfer -- --check- cleancargo check -p transfer --all-features --tests- cleancargo clippy -p transfer --all-features --all-targets --no-deps -- -D warnings- clean