binarybrain-009 commented on PR #18589: URL: https://github.com/apache/iceberg/pull/18589#issuecomment-6421003632
Thanks for taking a look. A count-based bound can't tell a factory that nobody uses from one that a live reader is reading through. That's how the 101st `S3FileIO` ends up closing a factory another FileIO is still using, which is the hang in #17485. FileIOs that are never closed already leak their S3 clients today (#15898), and without the bound the factory has the same lifetime as those clients. If we want a safety net for that case, one option is the `FileIOTracker` pattern. We would key the cache weakly by the `S3FileIOProperties` instance, which each `PrefixedS3Client` creates for itself and which uses identity equality. A factory would then be closed when its FileIO is closed or becomes unreachable, but never because of how many FileIOs exist. Open streams would hold that key, so a factory can't be collected mid-read. I'm happy to switch to that if it sounds better to you and @RussellSpitzer. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
