vrozov commented on issue #272: Refactor AvroIterable and prevent multiple iterators URL: https://github.com/apache/incubator-iceberg/pull/272#issuecomment-511143702 @rdblue Closing resource in an iterator when it (iterator is exhausted) is not a "better" solution, it is the mitigation for the resource leak/shortage issue. Otherwise, it would not be necessary for `AvroIterable` to be `CloseableIterable`. Such a solution still leaves a possibility for the resource shortage. It will also require a wrapper around `DataFileReader` as `DataFileReader` iterator does not close the underlying resource when it is exhausted. So, all `CloseableIterable` still needs to be closed either explicitly or by using a try-with-resources block. Note that a common expectation for `Closeable` or `AutoCloseable` used in a try-with-resources block is that there is a single resource acquired and that is guaranteed to be closed, not multiple resources as in the case of `CloseableGroup`. Additionally `CloseableGroup' is a subject of resource leak: should `close()` for one of `Closeable` in `closeables` throw an `IOException` or `RuntimeException`, all other `Closeable` won't be closed. It sounds that the major disagreement is whether it is OK to keep resources a little bit longer and allow multiple iterators or it is better to ensure that a resource is released asap but fail multiple iterators request. After spending a significant effort in isolating iceberg resource leak/shortage in our environment, my strong preference and recommendation is to fail fast when resources are not released asap. In an environment where resources are limited, the first behavior will still lead to run-time errors.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
