Fix receive of split large blocks with a short trailing chunk#18749
Draft
MorganaFuture wants to merge 1 commit into
Draft
Fix receive of split large blocks with a short trailing chunk#18749MorganaFuture wants to merge 1 commit into
MorganaFuture wants to merge 1 commit into
Conversation
A dataset with a large recordsize can store a single-block file whose block size is not a power of two. When such a block is sent without large blocks (no -L), the sender splits it into SPA_OLD_MAXBLOCKSIZE (128K) chunks, and the final chunk is smaller than the block size. flush_write_batch_impl() already handles any WRITE record whose size differs from the object's block size by doing a normal dmu_write(), but it first asserted the record was always larger than the block size. The shorter trailing chunk violates that assertion, so receiving such a stream panicked the receive_writer thread on debug builds; production builds took the correct dmu_write() path and were unaffected. Drop the assertion and describe both size-mismatch cases in the comment; the dmu_write() path already handles a record smaller than the block size. Add an rsend test that sends such a block without -L (initial and incremental, plain and compressed) and verifies the received file matches. Signed-off-by: MorganaFuture <103630661+MorganaFuture@users.noreply.github.com> Closes openzfs#17829
0229f82 to
07aac89
Compare
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.
Motivation and Context
Closes #17829.
A dataset with a large recordsize can store a single-block file whose block
size is not a power of two (a single-block object's
dblkszis rounded up to a512-byte multiple, e.g. a 300000-byte file gets a 300032-byte block). Sending
such a file without large blocks (no
-L) makes the sender split the blockinto
SPA_OLD_MAXBLOCKSIZE(128K) chunks, and the final chunk is shorter thanthe block size.
Receiving that stream panics
receive_writeron a debug build (the reporter's6-line reproducer:
recordsize=1m, one 300000-byte file,zfs sendwithout-Lpiped tozfs recv), wedging the receive in an unkillableDstate.Description
In
flush_write_batch_impl(), a WRITE record whose logical size differs fromthe object's block size is written with a normal
dmu_write(). The code firstasserted that such a record is always larger than the block size:
But the short trailing chunk above is smaller than the block size (the object
is created with the clamped 128K block size, while the tail chunk is e.g.
37888 bytes), so the assertion fails. Production builds compile the assertion
out and already take the correct
dmu_write()path, which handles a partialfinal block; only debug builds panic.
Drop the assertion and reword the comment to describe both size-mismatch cases.
No production code path changes.
How Has This Been Tested?
receive_writerD-state hang on current master (debug build,Ubuntu aarch64 VM); confirmed the stack sits in
flush_write_batch.source byte-for-byte, for plain, compressed, and incremental sends.
rsend/send_split_large_blocktest (guards the non-power-of-2 block viazdb, then sends without-Land diffs) passes;send-L_toggle,send-cpL_varied_recsize,send_large_blocks_initialandsend_large_blocks_incrementalstill pass — no regression.Types of changes
Checklist:
Signed-off-by.