[ 
https://issues.apache.org/jira/browse/HDFS-17965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105719#comment-18105719
 ] 

ASF GitHub Bot commented on HDFS-17965:
---------------------------------------

eubnara opened a new pull request, #8690:
URL: https://github.com/apache/hadoop/pull/8690

   
   
   <!--
     Thanks for sending a pull request!
       1. If this is your first time, please read our contributor guidelines: 
https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
       2. Make sure your PR title starts with JIRA issue id, e.g., 
'HADOOP-17799. Your PR title ...'.
   -->
   
   ### Description of PR
   
   
   DFSInputStream.close() calls dfsClient.checkOpen() before
   closeCurrentBlockReaders(). When the DFSClient is closed first (e.g.
   FileSystem.closeAllForUGI() during cleanup), checkOpen() throws and the
   block reader is never closed; since the closed flag is already set, the
   socket can never be released afterwards. In a long-lived JVM this leaks the
   connection and leaves the DataNode side stuck in FIN_WAIT1 with a
   non-draining send queue.
   
   We hit this in production through the DefaultContainerExecutor localizer
   path (see YARN-11856): killing a container during localization closes the
   UGI filesystems while a download thread still holds an open DFSInputStream.
   
   This PR wraps the checkOpen()/buffer-warning section in try/finally so
   closeCurrentBlockReaders() and super.close() always run. The existing
   behavior of throwing "Filesystem closed" from close() is preserved.
   
   ### How was this patch tested?
   
   New unit test
   TestDFSInputStream#testCloseReleasesBlockReaderWhenClientAlreadyClosed:
   opens a stream, reads to materialize a block reader, closes the DFSClient
   first, then closes the stream and asserts the block reader was released.
   Fails without the fix (block reader still present), passes with it.
   
   ### For code changes:
   
   - [x] Does the title of this PR start 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? *Note: 
Automated CI
         testing doesn't cover all cases so manual testing with cloud storage 
is still
         required.*
   - [ ] 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:
   
   - [x] The PR includes the phrase "Contains content generated by <tool>"
         where <tool> is the name of the AI tool used.
   Generative AI: Contains content generated by Claude Code (Anthropic
         Claude). The change was human-reviewed and verified on a production
         cluster, and complies with the ASF Generative Tooling Guidance
         (https://www.apache.org/legal/generative-tooling.html).
   - [x] My use of AI contributions follows the ASF legal policy
         https://www.apache.org/legal/generative-tooling.html
   




> DFSInputStream.close() leaks the block reader socket when the DFSClient is 
> already closed
> -----------------------------------------------------------------------------------------
>
>                 Key: HDFS-17965
>                 URL: https://issues.apache.org/jira/browse/HDFS-17965
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: hdfs-client
>         Environment: DFSInputStream.close() calls dfsClient.checkOpen() before
> closeCurrentBlockReaders():
> {code:java}
>   public synchronized void close() throws IOException {
>     try {
>       if (!closed.compareAndSet(false, true)) { ... return; }
>       dfsClient.checkOpen();            // throws if the client is closed
>       ...
>       closeCurrentBlockReaders();       // never reached in that case
>       super.close();
> {code}
> If the DFSClient has already been closed (for example via
> FileSystem.closeAllForUGI() during shutdown/cleanup), checkOpen() throws
> "Filesystem closed" and the current block reader is never closed. Because
> the closed flag has already been CAS-ed(Compare-And-Set) to true, any further 
> close() call
> is a no-op, so the block reader's socket to the DataNode can never be
> released for the lifetime of the client JVM.
> Production impact we observed: with DefaultContainerExecutor, the YARN
> NodeManager runs ContainerLocalizer inside the NM JVM. When a container is
> killed while localizing (YARN-11856), runLocalization()'s finally block
> closes the UGI filesystems while the download thread still holds an open
> DFSInputStream. The subsequent stream close() aborts at checkOpen() and
> leaks the socket inside the long-lived NM process. On the DataNode side the
> connection is stuck in FIN_WAIT1 with a send queue that never drains; on a
> ~600-node cluster this accumulated continuously until DataNodes carried
> large numbers of FIN_WAIT1 connections and NodeManagers leaked fds.
> Fix: release the block reader in a finally block so close() always frees
> the socket, while preserving the existing "Filesystem closed" exception
> behavior.
>            Reporter: YUBI LEE
>            Priority: Major
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to