[GitHub] [hbase] sunhelly commented on pull request #2699: HBASE-25287 Forgetting to unbuffer streams results in many CLOSE_WAIT…
sunhelly commented on pull request #2699: URL: https://github.com/apache/hbase/pull/2699#issuecomment-741553118 Failed UT is not relevant to this issue. @Apache9 Could you help to review this? I have fixed this issue and deployed on our production clusters, all work well. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #2699: HBASE-25287 Forgetting to unbuffer streams results in many CLOSE_WAIT…
sunhelly commented on pull request #2699: URL: https://github.com/apache/hbase/pull/2699#issuecomment-734230527 TestRSGroupsFallback.testFallback always passes on my local computer. A fixed issue is proposed at `HBASE-25334 TestRSGroupsFallback.testFallback is flaky` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #2699: HBASE-25287 Forgetting to unbuffer streams results in many CLOSE_WAIT…
sunhelly commented on pull request #2699: URL: https://github.com/apache/hbase/pull/2699#issuecomment-734046820 @Apache9 Yes, it's right. `initMetaAndIndex` is also used somewhere else. But I think `initTrailerAndContext` also needs to add a finally. PR has been updated. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #2699: HBASE-25287 Forgetting to unbuffer streams results in many CLOSE_WAIT…
sunhelly commented on pull request #2699: URL: https://github.com/apache/hbase/pull/2699#issuecomment-733530907 > Looks like it just adding a try/finally so we always unbuffer. Thats good. We weren't calling unbuffer ever? Thanks. Otherwise patch looks good to me. Thanks for reviewing, @saintstack . `unbuffer` currently is called in `HFile.createReader`, but when reading blocks by stream reader, new block reader might be created, and `unbuffer` is needed after that. I think `initTrailerAndContext` and `initMetaAndIndex` need to call `unbuffer`, too. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #2699: HBASE-25287 Forgetting to unbuffer streams results in many CLOSE_WAIT…
sunhelly commented on pull request #2699: URL: https://github.com/apache/hbase/pull/2699#issuecomment-733527278 > I see that in HFile.createReader we will call context.getInputStreamWrapper().unbuffer()? It is not enough to solve the problem here? Mind explaining a bit more? Thanks for your question, @Apache9 . I'll try to explain this problem. As discussed in HBASE-9393, `seek + read` (distinguished from `pread`) will result in CLOSE_WAIT sockets, and the `unbuffer()` method could release the socket and file descriptor. But actually, `unbuffer()` closes the latest block reader rather than the whole input stream. When loading hfiles, before `HFile.createReader`, the fileInfo is initialized by reading the trailer block, and `createReader` only uses the fileInfo to construct a HFileReader when it's a HFileStreamReader. (I think it's meaningless here to call `unbuffer`, because no block read here. But this is not important.) After creating reader, `HFileInfo.initMetaAndIndex` is needed, because the data index block and meta index block must be read and added to cache as a step of load-on-open. When reading these blocks, new block reader might be created in `DFSInputStream.blockSeekTo`, it means a new socket. As a result, I think it should add try...finally `unbuffer` to close the socket after reading blocks in `initMetaAndIndex`, and a try includes all the codes after stream created is the safest. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
