You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "voonhous (via GitHub)" <gi...@apache.org> on 2026/09/13 09:02:31 UTC
[PR] fix: close the filesystem's stream on rewrap [hudi]
voonhous opened a new pull request, #19935:
URL: https://github.com/apache/hudi/pull/19935
### Describe the issue this Pull Request addresses
`HadoopFSUtils.getFSDataInputStream` takes the `FSDataInputStream` a filesystem returns, pulls out its inner `FSInputStream`, buffers that and hands the buffered stream to callers. Closing what Hudi returns closes the file, but never the object the filesystem created. Filesystems that track the streams they hand out, such as Spark's test `DebugFilesystem`, therefore report every Hudi log file read as leaked although the OS stream is closed.
Found by TestStreamingSource on #19932: once the two `StreamTest` suites ran in a JVM where `DebugFilesystem` was the cached local filesystem, the suite aborted with "There are 26 possibly leaked file streams", 23 of them from this path. On master the check never sees Hudi's reads, because the SQL suites cache a plain LocalFileSystem first.
### Summary and Changelog
- The stream returned for the `FSInputStream` branch now closes the original `FSDataInputStream` after the buffered wrapper. The inner stream's second close is a no-op by the `Closeable` contract.
- Unit test with a `LocalFileSystem` subclass that counts the closes of the streams it hands out.
### Impact
Callers see the same stream; only its `close` reaches one more object.
### Risk Level
low. The extra close targets a wrapper whose inner stream is already closed.
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: close the filesystem's stream on rewrap [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19935:
URL: https://github.com/apache/hudi/pull/19935#issuecomment-5652454626
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "c9d1770d391602fac3a61adca98b5ebe10a9bff7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "c9d1770d391602fac3a61adca98b5ebe10a9bff7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* c9d1770d391602fac3a61adca98b5ebe10a9bff7 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: close the filesystem's stream on rewrap [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on code in PR #19935:
URL: https://github.com/apache/hudi/pull/19935#discussion_r4002090417
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/hadoop/fs/HadoopFSUtils.java:
##########
@@ -231,8 +231,21 @@ public static FSDataInputStream getFSDataInputStream(FileSystem fs,
}
if (fsDataInputStream.getWrappedStream() instanceof FSInputStream) {
+ // The buffered stream wraps the inner FSInputStream, not the FSDataInputStream the filesystem
+ // returned. Filesystems that track the streams they hand out (Spark's DebugFilesystem in tests)
+ // see a leak unless that outer object is closed too; the inner stream's close is idempotent.
+ final FSDataInputStream original = fsDataInputStream;
return new TimedFSDataInputStream(convertToHadoopPath(filePath), new FSDataInputStream(
- new BufferedFSInputStream((FSInputStream) fsDataInputStream.getWrappedStream(), bufferSize)));
+ new BufferedFSInputStream((FSInputStream) original.getWrappedStream(), bufferSize)) {
+ @Override
+ public void close() throws IOException {
+ try {
+ super.close();
+ } finally {
+ original.close();
Review Comment:
Intentionally out of scope. The GCS branch drops plain FSDataInputStream wrappers; the connector's own FSInputStream, which owns the connection, is what gets closed, so nothing leaks there. The only thing that observes an unclosed outer wrapper is a stream-tracking filesystem like Spark's test DebugFilesystem, which only fronts file:. This PR fixes the branch that had an observed failure and a test; I'd leave GCS alone rather than change an I/O path no test exercises.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: close the filesystem's stream on rewrap [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on code in PR #19935:
URL: https://github.com/apache/hudi/pull/19935#discussion_r4002090417
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/hadoop/fs/HadoopFSUtils.java:
##########
@@ -231,8 +231,21 @@ public static FSDataInputStream getFSDataInputStream(FileSystem fs,
}
if (fsDataInputStream.getWrappedStream() instanceof FSInputStream) {
+ // The buffered stream wraps the inner FSInputStream, not the FSDataInputStream the filesystem
+ // returned. Filesystems that track the streams they hand out (Spark's DebugFilesystem in tests)
+ // see a leak unless that outer object is closed too; the inner stream's close is idempotent.
+ final FSDataInputStream original = fsDataInputStream;
return new TimedFSDataInputStream(convertToHadoopPath(filePath), new FSDataInputStream(
- new BufferedFSInputStream((FSInputStream) fsDataInputStream.getWrappedStream(), bufferSize)));
+ new BufferedFSInputStream((FSInputStream) original.getWrappedStream(), bufferSize)) {
+ @Override
+ public void close() throws IOException {
+ try {
+ super.close();
+ } finally {
+ original.close();
Review Comment:
Intentionally out of scope. The GCS branch drops plain `FSDataInputStream` wrappers; the connector's own `FSInputStream`, which owns the connection, is what gets closed, so nothing leaks there.
The only thing that observes an unclosed outer wrapper is a stream-tracking filesystem like Spark's test `DebugFilesystem`, which only fronts file:.
This PR fixes the branch that had an observed failure and a test; I'd leave GCS alone rather than change an I/O path no test exercises.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: close the filesystem's stream on rewrap [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous merged PR #19935:
URL: https://github.com/apache/hudi/pull/19935
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: close the filesystem's stream on rewrap [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19935:
URL: https://github.com/apache/hudi/pull/19935#discussion_r3999277020
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/hadoop/fs/HadoopFSUtils.java:
##########
@@ -231,8 +231,21 @@ public static FSDataInputStream getFSDataInputStream(FileSystem fs,
}
if (fsDataInputStream.getWrappedStream() instanceof FSInputStream) {
+ // The buffered stream wraps the inner FSInputStream, not the FSDataInputStream the filesystem
+ // returned. Filesystems that track the streams they hand out (Spark's DebugFilesystem in tests)
+ // see a leak unless that outer object is closed too; the inner stream's close is idempotent.
+ final FSDataInputStream original = fsDataInputStream;
return new TimedFSDataInputStream(convertToHadoopPath(filePath), new FSDataInputStream(
- new BufferedFSInputStream((FSInputStream) fsDataInputStream.getWrappedStream(), bufferSize)));
+ new BufferedFSInputStream((FSInputStream) original.getWrappedStream(), bufferSize)) {
+ @Override
+ public void close() throws IOException {
+ try {
+ super.close();
+ } finally {
+ original.close();
Review Comment:
π€ The GCS branch in `getFSDataInputStreamForGCS` (a few lines below) has the same shape β it pulls the inner `FSInputStream` out of two levels of `FSDataInputStream` and drops both outer objects. Not reachable by `DebugFilesystem`, but would it make sense to apply the same close-through there for consistency, or is that intentionally out of scope?
<sub><i>β οΈ AI-generated; verify before applying. React π/π to flag quality.</i></sub>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: close the filesystem's stream on rewrap [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19935:
URL: https://github.com/apache/hudi/pull/19935#issuecomment-5652408575
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/19935?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 80.23%. Comparing base ([`1f5a4fe`](https://app.codecov.io/gh/apache/hudi/commit/1f5a4fe53aaadd69d91a208a833ace990ae06122?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`c9d1770`](https://app.codecov.io/gh/apache/hudi/commit/c9d1770d391602fac3a61adca98b5ebe10a9bff7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19935 +/- ##
============================================
- Coverage 80.23% 80.23% -0.01%
+ Complexity 34747 34746 -1
============================================
Files 2546 2546
Lines 142608 142612 +4
Branches 17362 17362
============================================
+ Hits 114419 114421 +2
- Misses 20277 20278 +1
- Partials 7912 7913 +1
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.84% <ΓΈ> (-0.01%)` | :arrow_down: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.33% <ΓΈ> (ΓΈ)` | |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.57% <ΓΈ> (ΓΈ)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.25% <ΓΈ> (ΓΈ)` | |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.19% <ΓΈ> (ΓΈ)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.99% <ΓΈ> (ΓΈ)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.90% <100.00%> (+0.01%)` | :arrow_up: |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.02% <ΓΈ> (ΓΈ)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.61% <ΓΈ> (ΓΈ)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.34% <ΓΈ> (ΓΈ)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.99% <ΓΈ> (ΓΈ)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19935/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.96% <ΓΈ> (ΓΈ)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/19935/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/19935/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.89% <100.00%> (-0.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/19935?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [.../java/org/apache/hudi/hadoop/fs/HadoopFSUtils.java](https://app.codecov.io/gh/apache/hudi/pull/19935?src=pr&el=tree&filepath=hudi-hadoop-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fhadoop%2Ffs%2FHadoopFSUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1oYWRvb3AtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2hhZG9vcC9mcy9IYWRvb3BGU1V0aWxzLmphdmE=) | `84.57% <100.00%> (+0.33%)` | :arrow_up: |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/19935/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: close the filesystem's stream on rewrap [hudi]
Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous commented on code in PR #19935:
URL: https://github.com/apache/hudi/pull/19935#discussion_r4002090417
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/hadoop/fs/HadoopFSUtils.java:
##########
@@ -231,8 +231,21 @@ public static FSDataInputStream getFSDataInputStream(FileSystem fs,
}
if (fsDataInputStream.getWrappedStream() instanceof FSInputStream) {
+ // The buffered stream wraps the inner FSInputStream, not the FSDataInputStream the filesystem
+ // returned. Filesystems that track the streams they hand out (Spark's DebugFilesystem in tests)
+ // see a leak unless that outer object is closed too; the inner stream's close is idempotent.
+ final FSDataInputStream original = fsDataInputStream;
return new TimedFSDataInputStream(convertToHadoopPath(filePath), new FSDataInputStream(
- new BufferedFSInputStream((FSInputStream) fsDataInputStream.getWrappedStream(), bufferSize)));
+ new BufferedFSInputStream((FSInputStream) original.getWrappedStream(), bufferSize)) {
+ @Override
+ public void close() throws IOException {
+ try {
+ super.close();
+ } finally {
+ original.close();
Review Comment:
Intentionally out of scope. The GCS branch drops plain `FSDataInputStream` wrappers; the connector's own FSInputStream, which owns the connection, is what gets closed, so nothing leaks there. The only thing that observes an unclosed outer wrapper is a stream-tracking filesystem like Spark's test DebugFilesystem, which only fronts file:. This PR fixes the branch that had an observed failure and a test; I'd leave GCS alone rather than change an I/O path no test exercises.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org