hudi-agent commented on code in PR #20321:
URL: https://github.com/apache/hudi/pull/20321#discussion_r4278125951
##########
hudi-io/src/main/java/org/apache/hudi/io/hfile/HFileBlock.java:
##########
@@ -295,8 +320,15 @@ public ByteBuffer serialize() throws IOException {
dataOutputStream.write(blockType.getMagic(), 0, 8);
// 2. onDiskSizeWithoutHeader.
int compressedDataSize = compressedBlockData.limit();
- int onDiskDataSizeWithHeader = HFileBlock.HFILEBLOCK_HEADER_SIZE +
compressedDataSize;
- int numChecksumBytes = numChecksumBytes(onDiskDataSizeWithHeader,
DEFAULT_BYTES_PER_CHECKSUM);
+ long onDiskDataSizeWithHeaderLong = (long)
HFileBlock.HFILEBLOCK_HEADER_SIZE + compressedDataSize;
Review Comment:
🤖 nit: would it make sense to drop the `Long` suffix, e.g. keep
`onDiskDataSizeWithHeader` as the long and name the narrowed value
`onDiskDataSizeWithHeaderInt` (or just cast at the use site)? Two
near-identical names are easy to mix up.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-io/src/main/java/org/apache/hudi/io/hfile/HFileBlock.java:
##########
@@ -122,7 +126,9 @@ protected HFileBlock(HFileContext context,
byteBuff, startOffsetInBuff +
Header.UNCOMPRESSED_SIZE_WITHOUT_HEADER_INDEX);
this.bytesPerChecksum = readInt(
byteBuff, startOffsetInBuff + Header.BYTES_PER_CHECKSUM_INDEX);
- this.sizeCheckSum = numChecksumBytes(getOnDiskSizeWithHeader(),
bytesPerChecksum);
+ this.sizeCheckSum = bytesPerChecksum > 0
Review Comment:
🤖 nit: could you add a short comment that the `-1` is a placeholder for an
invalid `bytesPerChecksum`, which `validateHeader` rejects right after?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
