HADOOP-19901: Release checksum buffers after vectored read verification in ChecksumFileSystem#8511
Open
iemejia wants to merge 1 commit into
Open
HADOOP-19901: Release checksum buffers after vectored read verification in ChecksumFileSystem#8511iemejia wants to merge 1 commit into
iemejia wants to merge 1 commit into
Conversation
…on in ChecksumFileSystem ChecksumFileSystem.readVectored() allocates buffers for both data ranges and checksum ranges through the caller's allocator. After verification, the checksum buffers were never released because the caller has no reference to them and the system did not release them. This causes buffer leaks when callers use a tracking/pooled allocator (e.g., Apache Parquet's TrackingByteBufferAllocator), as the checksum buffers accumulate without being returned to the pool. Fix: After all verifications for a checksum range complete (via CompletableFuture.allOf), release the checksum buffer using the caller-provided release consumer. This ensures exactly-once release even when one checksum range covers multiple data ranges. Added a regression test that verifies the release consumer is called for checksum buffers after vectored read verification completes.
|
💔 -1 overall
This message was automatically generated. |
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
2 participants
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.
Description
ChecksumFileSystem.readVectored()allocates buffers for both data ranges and checksum ranges through the caller's allocator (IntFunction<ByteBuffer>). After checksum verification completes, the checksum buffers were never released because:releaseconsumer on themThis causes buffer leaks when callers use a tracking or pooled allocator. This bug was discovered while working on Apache Parquet's
TrackingByteBufferAllocator, which detected unreleased buffers accumulating during vectored reads throughChecksumFileSystem.Root Cause
In
ChecksumFileSystem.ChecksumFSInputChecker.readVectored(), checksum ranges are read viasums.readVectored(checksumRanges, allocate, release). The checksum buffers are used inthenCombineAsynccalls for verification, but after verification completes, no code released them back to the caller's pool.A single checksum range can cover multiple data ranges, so the checksum buffer is shared across multiple
thenCombineAsynccalls — it must only be released once, after ALL verifications using it are complete.Fix
After collecting all verification futures for a checksum range, use
CompletableFuture.allOf(verifications).thenRun(...)to release the checksum buffer exactly once after all verifications complete:This ensures:
Testing
Added
testChecksumBuffersReleasedAfterVectoredRead()toTestLocalFSContractVectoredRead:LocalFileSystem(which usesChecksumFileSystem)All 54 existing vectored read contract tests continue to pass.
JIRA
https://issues.apache.org/jira/browse/HADOOP-19901