wgtmac commented on code in PR #3559:
URL: https://github.com/apache/parquet-java/pull/3559#discussion_r3506853474
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -2327,6 +2327,10 @@ public void readFromVectoredRange(ParquetFileRange
currRange, ChunkListBuilder b
LOG.error(error, e);
throw new IOException(error, e);
}
+ // Release the vectored-read buffer back to the allocator when the row
group is closed.
+ // Requires fs.file.checksum.verify=false so the returned buffer is the
allocator buffer
+ // rather than a sliced subset (see Hadoop's fs.file.checksum.verify
docs).
+ builder.addBuffersToRelease(Collections.singletonList(buffer));
Review Comment:
This assumes the future returns the exact buffer allocated by our allocator.
With local checksum verification enabled, Hadoop can return a sliced buffer, so
strict allocators like `TrackingByteBufferAllocator` or
`ReusingByteBufferAllocator` may fail on release or still report a leak. The
new test `core-site.xml` disables that path globally, so this should either
guard against sliced vectored IO or have a test with checksum verification left
on. (reviewed by Codex)
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -2327,6 +2327,10 @@ public void readFromVectoredRange(ParquetFileRange
currRange, ChunkListBuilder b
LOG.error(error, e);
throw new IOException(error, e);
}
+ // Release the vectored-read buffer back to the allocator when the row
group is closed.
+ // Requires fs.file.checksum.verify=false so the returned buffer is the
allocator buffer
+ // rather than a sliced subset (see Hadoop's fs.file.checksum.verify
docs).
+ builder.addBuffersToRelease(Collections.singletonList(buffer));
Review Comment:
If this range is registered and a later vectored range times out or fails
before `rowGroup.setReleaser(builder.releaser)`, the builder is dropped without
closing the releaser. We should close the builder releaser on the failure path,
or transfer ownership earlier so partially-read row groups still release their
buffers. (reviewed by Codex)
--
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]
