Skip to content

HBASE-30392 Exclude checksum from onDiskDataSizeWithHeader - #8661

Open
Huginn-kio wants to merge 1 commit into
apache:masterfrom
Huginn-kio:HBASE-30392
Open

Huginn-kio wants to merge 1 commit into
apache:masterfrom
Huginn-kio:HBASE-30392

Conversation

@Huginn-kio

@Huginn-kio Huginn-kio commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

HBASE-30392

Summary

Keep the size metadata of cache-on-write HFile blocks consistent with the bytes stored in the cache. See HBASE-30392.

Root cause

HFileBlock.Writer#getBlockForCaching creates the cache buffer with cloneOnDiskBufferWithHeader(), which contains only the block header and on-disk data. The metadata was nevertheless populated with onDiskBlockBytesWithHeader.size() + onDiskChecksum.length, so onDiskDataSizeWithHeader described bytes that are not present in the cache buffer.

Fix

  • Set onDiskDataSizeWithHeader to the header-plus-data buffer length.
  • Keep onDiskSizeWithoutHeader unchanged because the physical on-disk block size still includes checksums.

Compatibility

  • No public API or configuration change.
  • No HFile on-disk format change.
  • The change only corrects in-memory cache metadata for cache-on-write blocks.

Testing

  • git diff --check passed.
  • mvn -pl hbase-server -am -DskipTests -Dcheckstyle.skip=true -Drat.skip=true -Dspotbugs.skip=true compile passed with JDK 17.

@wchevreuil wchevreuil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of the property here is "onDiskDataSizeWithHeader" so it should record the proper block size on disk. If we apply this change, we won't be recording the actual block size on disk.

@wchevreuil

Copy link
Copy Markdown
Contributor

The resulting size mismatch can make cache validation or later block unpacking use an incorrect length, causing cache mismatches or read/decompression failures.

Can you prove this?

Also, there are other parts of the code that set "onDiskDataSizeWithHeader" within the checksum. If we would apply this proposal, we would probably break block comparison when checking the cache, for cacheOnRead, for example, potentially caching an already cached block again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants