chungen0126 commented on PR #10764: URL: https://github.com/apache/ozone/pull/10764#issuecomment-5623609794
@szetszwo thanks for reviewing. I refactored those computation into a new class. > searchChunkByOffset(adjustedOffset + bufferLimit, ..) should use adjustedOffset + bufferLimit - 1 for the end position (inclusive) I'd like to share a concrete example to illustrate why we need the exclusive boundary (offset + responseDataSize). If this boundary perfectly aligns with the starting offset of the end chunk, the inclusive approach incorrectly truncates the result. Consider this scenario: - bytesPerChecksum = 16 (bitMask = -16) - Chunk 1: offset = 0, length = 20 - Chunk 2: offset = 20, length = 20 - Current offset = 0 - responseDataSize = 20 (meaning offset + responseDataSize = 20, exactly the start of Chunk 2) Since byte 20 is the exact end of Chunk 1, it is a perfectly valid checksum boundary (the final 4 bytes form a padded checksum). The buffer limit should remain 20. If we use the inclusive approach (end = 20 - 1 = 19): - endChunk = findChunk(19) -> returns Chunk 1 (offset = 0) - lengthExcludingEndChunk = 0 - 0 = 0. - lengthAtEndChunk = (20 - 0) & -16 = 16. - Result = 16. (It incorrectly truncates 4 bytes). If we use the exclusive approach (end = 20): - endChunk = findChunk(20) -> returns Chunk 2 (offset = 20). - lengthExcludingEndChunk = 20 - 0 = 20. - lengthAtEndChunk = (20 - 20) & -16 = 0. - Result = 20. (It correctly reads the entire Chunk 1). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
