GH-1067: Close cached HDFS FileSystem instances - #1141
Conversation
This comment has been minimized.
This comment has been minimized.
|
@xborder I fixed the CI on main. Can you please rebase? |
|
Sorry, probably need to rebase again. |
|
done @lidavidm |
| try { | ||
| super.close(); | ||
| } finally { | ||
| hdfsFileSystems.forEach(FileSystemDatasetFactory::closeHadoopFileSystem); | ||
| } |
There was a problem hiding this comment.
nit: could we use AutoCloseables here?
There was a problem hiding this comment.
I think we could use AutoCloseables by casting the Object obtained through reflection, but it would not simplify or improve the cleanup logic.
Is there any benefit in changing it that I'm not seeing?
There was a problem hiding this comment.
Well, you would assemble a list of lambdas essentially and close it all at once; that way a failure to close one won't affect the others. But this is fine.
| try { | ||
| super.close(); | ||
| } finally { | ||
| hdfsFileSystems.forEach(FileSystemDatasetFactory::closeHadoopFileSystem); | ||
| } |
There was a problem hiding this comment.
Well, you would assemble a list of lambdas essentially and close it all at once; that way a failure to close one won't affect the others. But this is fine.
3 participants
What's Changed
Closes #1067 .