You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "steveloughran (via GitHub)" <gi...@apache.org> on 2026/07/09 17:20:53 UTC
[PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
steveloughran opened a new pull request, #8594:
URL: https://github.com/apache/hadoop/pull/8594
### How was this patch tested?
New tests
### For code changes:
- [ ] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
### AI Tooling
If an AI tool was used:
- [ ] The PR includes the phrase "Contains content generated by <tool>"
where <tool> is the name of the AI tool used.
- [ ] My use of AI contributions follows the ASF legal policy
https://www.apache.org/legal/generative-tooling.html
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8594:
URL: https://github.com/apache/hadoop/pull/8594#issuecomment-4929580446
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 37s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 46m 26s | | trunk passed |
| +1 :green_heart: | compile | 18m 8s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | compile | 19m 16s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | checkstyle | 1m 22s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 58s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 29s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 1m 23s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 32m 53s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 19s | | the patch passed |
| +1 :green_heart: | compile | 16m 31s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 16m 31s | | the patch passed |
| +1 :green_heart: | compile | 17m 0s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 17m 0s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 1m 30s | [/results-checkstyle-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/1/artifact/out/results-checkstyle-hadoop-common-project_hadoop-common.txt) | hadoop-common-project/hadoop-common: The patch generated 13 new + 236 unchanged - 7 fixed = 249 total (was 243) |
| +1 :green_heart: | mvnsite | 2m 1s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 26s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 1m 23s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 27s | | the patch passed |
| +1 :green_heart: | shadedclient | 32m 9s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 24m 10s | | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 1m 17s | | The patch does not generate ASF License warnings. |
| | | 230m 15s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/8594 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 88bd1fdf7958 5.15.0-177-generic #187-Ubuntu SMP Sat Apr 11 22:54:33 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 8afd68618d887b0fb2f99922251376ab7f118f18 |
| Default Java | Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.11+10-1-24.04.2-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/1/testReport/ |
| Max. process+thread count | 1257 (vs. ulimit of 10000) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/1/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #8594:
URL: https://github.com/apache/hadoop/pull/8594#issuecomment-4935712164
updated. Cut the public TFileMeta/1 ctors as the class is actually package private, nothing was using it.
If anyone feels that is overkill and suspects that there was some reflection based access, I could revert.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
Posted by "ajfabbri (via GitHub)" <gi...@apache.org>.
ajfabbri commented on code in PR #8594:
URL: https://github.com/apache/hadoop/pull/8594#discussion_r3634084602
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparators.java:
##########
@@ -73,45 +73,28 @@ public void tearDown() throws IOException {
// bad comparator format
@Test
- public void testFailureBadComparatorNames() throws IOException {
- try {
- writer = new Writer(out, BLOCK_SIZE, compression, "badcmp", conf);
- fail("Failed to catch unsupported comparator names");
- }
- catch (Exception e) {
- // noop, expecting exceptions
- e.printStackTrace();
- }
+ public void testFailureBadComparatorNames() throws Exception {
+ intercept(IllegalArgumentException.class, "Unsupported comparator", () ->
+ new Writer(out, BLOCK_SIZE, compression, "badcmp", conf));
}
Review Comment:
`out` *is* closed in `tearDown()`, resolving
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8594:
URL: https://github.com/apache/hadoop/pull/8594#issuecomment-4929668120
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 52s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 47m 57s | | trunk passed |
| +1 :green_heart: | compile | 18m 25s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | compile | 18m 36s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | checkstyle | 1m 27s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 59s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 23s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 1m 21s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 15s | | trunk passed |
| +1 :green_heart: | shadedclient | 36m 47s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 19s | | the patch passed |
| +1 :green_heart: | compile | 17m 15s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 17m 15s | | the patch passed |
| +1 :green_heart: | compile | 18m 32s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 18m 32s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 1m 26s | [/results-checkstyle-hadoop-common-project_hadoop-common.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/2/artifact/out/results-checkstyle-hadoop-common-project_hadoop-common.txt) | hadoop-common-project/hadoop-common: The patch generated 13 new + 237 unchanged - 7 fixed = 250 total (was 244) |
| +1 :green_heart: | mvnsite | 1m 59s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 19s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 1m 20s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 23s | | the patch passed |
| +1 :green_heart: | shadedclient | 36m 31s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 23m 43s | | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 1m 12s | | The patch does not generate ASF License warnings. |
| | | 241m 3s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/8594 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 0a96fcf42716 5.15.0-185-generic #195-Ubuntu SMP Fri Jun 19 17:11:50 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 8afd68618d887b0fb2f99922251376ab7f118f18 |
| Default Java | Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.11+10-1-24.04.2-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/2/testReport/ |
| Max. process+thread count | 3107 (vs. ulimit of 10000) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/2/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran merged PR #8594:
URL: https://github.com/apache/hadoop/pull/8594
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8594:
URL: https://github.com/apache/hadoop/pull/8594#discussion_r3559230233
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/Utils.java:
##########
@@ -274,8 +275,36 @@ public static void writeString(DataOutput out, String s) throws IOException {
* @throws IOException raised on errors performing I/O.
*/
public static String readString(DataInput in) throws IOException {
+ return readString(in, -1);
+ }
+
+ /**
+ * Read a String as a VInt n, followed by n Bytes in Text format, rejecting
+ * lengths that are -2 or less. or larger than the supplied bound before any
+ * buffer is allocated.
Review Comment:
Javadoc has a sentence break/typo ("-2 or less. or larger") which makes the description harder to read. Consider rephrasing to a single sentence with proper punctuation.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileLoading.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.io.file.tfile;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.EOFException;
+import java.io.IOException;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.io.LongWritable;
+import org.apache.hadoop.io.file.tfile.TFile.Reader;
+import org.apache.hadoop.io.file.tfile.TFile.Writer;
+import org.apache.hadoop.test.GenericTestUtils;
+
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Test TFile loading resilience.
+ */
+public class TestTFileLoading {
+
+ private static final String ROOT =
+ GenericTestUtils.getTestDir().getAbsolutePath();
+
+ private static final int BLOCK_SIZE = 512;
+
+ private static final String COMPRESSION = Compression.Algorithm.NONE.getName();
+
+ private static final String JCLASS_COMPARATOR =
+ TFile.COMPARATOR_JCLASS + LongWritable.Comparator.class.getName();
+
+ private FileSystem fs;
+
+ private Path path;
+
+ @BeforeEach
+ public void setUp() throws IOException {
+ Configuration conf = new Configuration();
+ path = new Path(ROOT, "TestTFileLoading");
+ fs = FileSystem.getLocal(conf);
+ }
+
+ @AfterEach
+ public void tearDown() throws IOException {
+ fs.delete(path, true);
+ }
+
+ private Configuration enableJclass() {
+ Configuration conf = new Configuration();
+ conf.setBoolean(TFile.TFILE_COMPARATOR_JCLASS_ENABLED, true);
+ return conf;
+ }
+
+ @Test
+ public void testJClassComparatorRejectedByDefault() throws Exception {
+ try (FSDataOutputStream out = fs.create(path)) {
+ intercept(IllegalArgumentException.class, () ->
+ new Writer(out, BLOCK_SIZE, COMPRESSION, JCLASS_COMPARATOR, new Configuration()));
+ }
+ }
+
+ @Test
+ public void testJClassComparatorMustBeRawComparator() throws Exception {
+ Configuration conf = enableJclass();
+ String notAComparator =
+ TFile.COMPARATOR_JCLASS + Chunk.class.getName();
+ try (FSDataOutputStream out = fs.create(path)) {
+ intercept(IllegalArgumentException.class, () ->
+ new Writer(out, BLOCK_SIZE, COMPRESSION, notAComparator, conf));
+ }
+ }
+
+ /**
+ * A file that declares a jclass comparator (written with the feature
+ * enabled) is not honored by a reader that has not enabled it; the named
+ * class is only resolved and constructed when the feature is turned on.
+ */
+ @Test
+ public void testReaderIgnoresJClassWhenDisabled() throws Exception {
Review Comment:
The test name says the reader "ignores" `jclass:` comparators when disabled, but the test asserts the reader rejects the file. Renaming the test improves clarity and matches the behavior being verified.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/TFile.java:
##########
@@ -2048,27 +2063,32 @@ static final class TFileMeta {
private final BytesComparator comparator;
// ctor for writes
- public TFileMeta(String comparator) {
+ TFileMeta(String comparator, Configuration conf) {
// set fileVersion to API version when we create it.
version = TFile.API_VERSION;
recordCount = 0;
strComparator = (comparator == null) ? "" : comparator;
- this.comparator = makeComparator(strComparator);
+ this.comparator = makeComparator(strComparator, conf);
}
// ctor for reads
- public TFileMeta(DataInput in) throws IOException {
+ TFileMeta(DataInput in, Configuration conf) throws IOException {
version = new Version(in);
if (!version.compatibleWith(TFile.API_VERSION)) {
throw new RuntimeException("Incompatible TFile fileVersion.");
}
recordCount = Utils.readVLong(in);
- strComparator = Utils.readString(in);
- comparator = makeComparator(strComparator);
+ strComparator = Utils.readString(in, MAX_COMPARATOR_LENGTH);
+ comparator = makeComparator(strComparator, conf);
}
- @SuppressWarnings("unchecked")
static BytesComparator makeComparator(String comparator) {
+ return makeComparator(comparator, new Configuration());
+ }
Review Comment:
`TFileMeta.makeComparator(String)` creates a full default `Configuration`, which can load default resources and is relatively expensive for a pure parsing helper. Using `new Configuration(false)` avoids that overhead while keeping behavior the same (and callers that need settings should use the `(String, Configuration)` overload).
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/TFile.java:
##########
@@ -166,6 +166,21 @@ static int getFSOutputBufferSize(Configuration conf) {
/** comparator prefix: java class */
public static final String COMPARATOR_JCLASS = "jclass:";
+ /**
+ * {@value}.
+ */
+ public static final String TFILE_COMPARATOR_JCLASS_ENABLED =
+ "tfile.comparator.jclass.enabled";
+ public static final boolean TFILE_COMPARATOR_JCLASS_ENABLED_DEFAULT = false;
+
Review Comment:
With `TFILE_COMPARATOR_JCLASS_ENABLED` now gating `jclass:` comparator resolution, the public `TFile.makeComparator(String)` helper has no way to accept a `Configuration` with that flag enabled. Adding an overload which takes a `Configuration` lets callers opt in programmatically (and keeps the default behavior locked down).
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparators.java:
##########
@@ -73,45 +73,28 @@ public void tearDown() throws IOException {
// bad comparator format
@Test
- public void testFailureBadComparatorNames() throws IOException {
- try {
- writer = new Writer(out, BLOCK_SIZE, compression, "badcmp", conf);
- fail("Failed to catch unsupported comparator names");
- }
- catch (Exception e) {
- // noop, expecting exceptions
- e.printStackTrace();
- }
+ public void testFailureBadComparatorNames() throws Exception {
+ intercept(IllegalArgumentException.class, "Unsupported comparator", () ->
+ new Writer(out, BLOCK_SIZE, compression, "badcmp", conf));
}
Review Comment:
`setUp()` creates and leaves `out` open, and these tests intentionally trigger exceptions during `Writer` construction. Without closing `out`, this can leak file descriptors and may cause intermittent failures on some platforms. Close `out` in `tearDown()` (you already have `closeOutput()` for this).
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8594:
URL: https://github.com/apache/hadoop/pull/8594#issuecomment-4937597188
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 34s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 41m 7s | | trunk passed |
| +1 :green_heart: | compile | 16m 23s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | compile | 16m 29s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | checkstyle | 1m 33s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 9s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 26s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 1m 24s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 34m 5s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 12s | | the patch passed |
| +1 :green_heart: | compile | 15m 26s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 15m 26s | | the patch passed |
| +1 :green_heart: | compile | 16m 41s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 16m 41s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 1m 29s | | hadoop-common-project/hadoop-common: The patch generated 0 new + 234 unchanged - 9 fixed = 234 total (was 243) |
| +1 :green_heart: | mvnsite | 2m 0s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 25s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 1m 27s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 18s | | the patch passed |
| +1 :green_heart: | shadedclient | 31m 11s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 23m 44s | | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 1m 13s | | The patch does not generate ASF License warnings. |
| | | 218m 52s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/8594 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 9ff8c78061bf 5.15.0-177-generic #187-Ubuntu SMP Sat Apr 11 22:54:33 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 3fbb1a4ee7e557bcb180d43e3b1f0aadadd47e4b |
| Default Java | Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.11+10-1-24.04.2-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/3/testReport/ |
| Max. process+thread count | 3150 (vs. ulimit of 10000) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/3/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19936. TFile parsing is too brittle [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8594:
URL: https://github.com/apache/hadoop/pull/8594#issuecomment-4996741975
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 35s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 41m 52s | | trunk passed |
| +1 :green_heart: | compile | 16m 12s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | compile | 16m 47s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | checkstyle | 1m 25s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 1s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 27s | | trunk passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 1m 30s | | trunk passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 12s | | trunk passed |
| +1 :green_heart: | shadedclient | 31m 4s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 13s | | the patch passed |
| +1 :green_heart: | compile | 15m 33s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 15m 33s | | the patch passed |
| +1 :green_heart: | compile | 16m 35s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javac | 16m 35s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 1m 32s | | hadoop-common-project/hadoop-common: The patch generated 0 new + 233 unchanged - 10 fixed = 233 total (was 243) |
| +1 :green_heart: | mvnsite | 2m 2s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 28s | | the patch passed with JDK Ubuntu-21.0.11+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | javadoc | 1m 26s | | the patch passed with JDK Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| +1 :green_heart: | spotbugs | 3m 17s | | the patch passed |
| +1 :green_heart: | shadedclient | 30m 44s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 23m 48s | | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 1m 14s | | The patch does not generate ASF License warnings. |
| | | 216m 25s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/8594 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux f4e48f5a9d68 5.15.0-177-generic #187-Ubuntu SMP Sat Apr 11 22:54:33 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / a457744cca371e1bfeb0de6c4338679fa90c7ea5 |
| Default Java | Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.11+10-1-24.04.2-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.19+10-1-24.04.2-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/4/testReport/ |
| Max. process+thread count | 1305 (vs. ulimit of 10000) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8594/4/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org