Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDDS-12079. Improve container merkle tree write handling #7694

Open
wants to merge 7 commits into
base: HDDS-10239-container-reconciliation
Choose a base branch
from

Conversation

errose28
Copy link
Contributor

@errose28 errose28 commented Jan 13, 2025

What changes were proposed in this pull request?

  • Rename ContainerMerkleTree to ContainerMerkleTreeWriter to better reflect its purpose.
  • Add data prefix to all checksum fields in the tree to differentiate them from other checksum types like metadata checksums that might be added later.
  • Aggregate chunk checksums as they are added as leaves to the tree to reduce scanner memory footprint. We no longer need to hold every ChunkInfo object from the container in memory while the tree is being built.
  • Use CRC32C to build the tree, regardless of the checksum used on the actual data.
    • CRC32C is a general improvement over CRC32, so we should use this to build the trees to prevent migrating later.
    • Per ChecksumByteBufferFactory#crc32CImpl, this will use a native CRC32C implementation for Java >= 9, otherwise fall back to a Java implementation.

What is the link to the Apache JIRA

HDDS-12079

How was this patch tested?

Existing tests must pass. No behavioral change is expected.

Copy link
Member

@aswinshakil aswinshakil left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @errose28. LGTM+1

Copy link
Member

@aswinshakil aswinshakil left a comment

Choose a reason for hiding this comment

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

Thanks for the update @errose28. LGTM. Pending CI.

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

Successfully merging this pull request may close these issues.

2 participants