Skip to content

Commit 16ddc39

Browse files
committed
Make the build more forward-compatible (before move/testing with Java 21/25)
Hadoop upgrade (3.3.0 -> 3.4.3): - Hadoop 3.3.x uses javax.security.auth.Subject.getSubject() which was removed in JDK 23+ (JEP 471). Hadoop 3.4.x uses Subject.current() instead, restoring JDK 25 compatibility. Spotless upgrade (2.46.1 -> 3.5.1): - Spotless 2.46.1 calls com.sun.tools.javac.util.Log methods that were removed in JDK 25, causing NoSuchMethodError at format time. Spotless 3.5.1 is compatible with JDK 25. The minor formatting changes to switch/case comment indentation are from the new version. Fix ByteBuffer leak in vectored I/O reads: - Hadoop's readVectored() API accepts an IntFunction<ByteBuffer> for allocation but has no corresponding release callback. When a wrapping filesystem like ChecksumFileSystem is in the path, Hadoop allocates a buffer through the caller's allocator, uses it internally for checksum verification, then creates a different buffer for the CompletableFuture result. The originally allocated buffer is abandoned without release. - This caused TrackingByteBufferAllocator (used in tests) to throw LeakedByteBufferException for tests using vectored I/O: TestRecordLevelFilters, TestColumnIndexFiltering, TestParquetReader. - Fix: wrap the allocator in a capturing decorator that tracks every buffer allocated during readVectored(), then registers them all for release via ByteBufferReleaser. A try-finally ensures buffers are registered even if a read future times out or fails.
1 parent 8624ce5 commit 16ddc39

8 files changed

Lines changed: 46 additions & 15 deletions

File tree

parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,7 @@ public EncodingStats convertEncodingStats(List<PageEncodingStats> stats) {
778778
switch (stat.getPage_type()) {
779779
case DATA_PAGE_V2:
780780
builder.withV2Pages();
781-
// falls through
781+
// falls through
782782
case DATA_PAGE:
783783
builder.addDataEncoding(getEncoding(stat.getEncoding()), stat.getCount());
784784
break;

parquet-hadoop/src/main/java/org/apache/parquet/hadoop/CodecFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ private String cacheKey(CompressionCodecName codecName) {
456456
level = conf.get("parquet.compression.codec.zstd.level");
457457
break;
458458
default:
459-
// compression level is not supported; ignore it
459+
// compression level is not supported; ignore it
460460
}
461461
String codecClass = codecName.getHadoopCompressionCodecClassName();
462462
return level == null ? codecClass : codecClass + ":" + level;

parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ColumnIndexValidator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ private void validateBoundaryOrder(
546546
prevMaxValue::toString);
547547
break;
548548
case UNORDERED:
549-
// No checks necessary.
549+
// No checks necessary.
550550
}
551551
}
552552
}

parquet-hadoop/src/main/java/org/apache/parquet/hadoop/DirectCodecFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ protected BytesCompressor createCompressor(final CompressionCodecName codecName)
103103
return new SnappyCompressor();
104104
case ZSTD:
105105
return new ZstdCompressor();
106-
// todo: create class similar to the SnappyCompressor for zlib and exclude it as
107-
// snappy is above since it also generates allocateDirect calls.
106+
// todo: create class similar to the SnappyCompressor for zlib and exclude it as
107+
// snappy is above since it also generates allocateDirect calls.
108108
default:
109109
return super.createCompressor(codecName);
110110
}

parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import org.apache.parquet.HadoopReadOptions;
6666
import org.apache.parquet.ParquetReadOptions;
6767
import org.apache.parquet.Preconditions;
68+
import org.apache.parquet.bytes.ByteBufferAllocator;
6869
import org.apache.parquet.bytes.ByteBufferInputStream;
6970
import org.apache.parquet.bytes.ByteBufferReleaser;
7071
import org.apache.parquet.bytes.BytesInput;
@@ -1377,12 +1378,42 @@ private void readVectored(List<ConsecutivePartList> allParts, ChunkListBuilder b
13771378
totalSize += len;
13781379
}
13791380
LOG.debug("Reading {} bytes of data with vectored IO in {} ranges", totalSize, ranges.size());
1380-
// Request a vectored read;
1381-
f.readVectored(ranges, options.getAllocator());
1382-
int k = 0;
1383-
for (ConsecutivePartList consecutivePart : allParts) {
1384-
ParquetFileRange currRange = ranges.get(k++);
1385-
consecutivePart.readFromVectoredRange(currRange, builder);
1381+
// Use a capturing allocator to track all buffers allocated by Hadoop during vectored reads.
1382+
// The buffer returned from the read future may differ from the one originally allocated
1383+
// (e.g., ChecksumFileSystem wraps/copies buffers), so we must track the actual allocations.
1384+
List<ByteBuffer> allocatedBuffers = new ArrayList<>();
1385+
ByteBufferAllocator capturingAllocator = new ByteBufferAllocator() {
1386+
@Override
1387+
public ByteBuffer allocate(int size) {
1388+
ByteBuffer buf = options.getAllocator().allocate(size);
1389+
allocatedBuffers.add(buf);
1390+
return buf;
1391+
}
1392+
1393+
@Override
1394+
public void release(ByteBuffer b) {
1395+
// Use identity comparison; ByteBuffer.equals() is content-based and could match wrong buffer
1396+
allocatedBuffers.removeIf(buf -> buf == b);
1397+
options.getAllocator().release(b);
1398+
}
1399+
1400+
@Override
1401+
public boolean isDirect() {
1402+
return options.getAllocator().isDirect();
1403+
}
1404+
};
1405+
try {
1406+
// Request a vectored read;
1407+
f.readVectored(ranges, capturingAllocator);
1408+
int k = 0;
1409+
for (ConsecutivePartList consecutivePart : allParts) {
1410+
ParquetFileRange currRange = ranges.get(k++);
1411+
consecutivePart.readFromVectoredRange(currRange, builder);
1412+
}
1413+
} finally {
1414+
// Register all buffers allocated during vectored reads for release.
1415+
// In a finally block so buffers are not leaked on read failures.
1416+
builder.addBuffersToRelease(allocatedBuffers);
13861417
}
13871418
}
13881419

parquet-thrift/src/main/java/org/apache/parquet/thrift/BufferedProtocolReadToWrite.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ private boolean readOneValue(TProtocol in, byte type, List<Action> buffer, Thrif
226226
writeShortAction(buffer, s);
227227
break;
228228
case TType.ENUM: // same as i32 => actually never seen in the protocol layer as enums are written as a i32
229-
// field
229+
// field
230230
case TType.I32:
231231
final int i = in.readI32();
232232
checkEnum(expectedType, i);

parquet-thrift/src/main/java/org/apache/parquet/thrift/ProtocolReadToWrite.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ void readOneValue(TProtocol in, TProtocol out, byte type) throws TException {
7474
out.writeI16(in.readI16());
7575
break;
7676
case TType.ENUM: // same as i32 => actually never seen in the protocol layer as enums are written as a i32
77-
// field
77+
// field
7878
case TType.I32:
7979
out.writeI32(in.readI32());
8080
break;

pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,10 @@
8080
<jackson-annotations.version>2.22</jackson-annotations.version>
8181
<japicmp.version>0.26.1</japicmp.version>
8282
<javax.annotation.version>1.3.2</javax.annotation.version>
83-
<spotless.version>2.46.1</spotless.version>
83+
<spotless.version>3.5.1</spotless.version>
8484
<shade.prefix>shaded.parquet</shade.prefix>
8585
<!-- Guarantees no newer classes/methods/constants are used by parquet. -->
86-
<hadoop.version>3.3.0</hadoop.version>
86+
<hadoop.version>3.4.3</hadoop.version>
8787
<parquet.format.version>2.13.0</parquet.format.version>
8888
<previous.version>1.17.0</previous.version>
8989
<thrift.executable>thrift</thrift.executable>

0 commit comments

Comments
 (0)