emmanuel099 commented on issue #13297: URL: https://github.com/apache/iceberg/issues/13297#issuecomment-2975566460
Hi, my team and I had a similar problem while querying and optimizing tables with a huge number of data and delete files. Spark always ran OOM, because resources (Parquet readers, ...) weren't freed properly. I did some debugging and identified `CloseableIterable` as the culprit. Unfortunately, I never had time to upstream it properly. `CloseableIterable` has the following two problems: 1. When turning a `CloseableIterable` into an iterator, the returned `CloseableIterator` doesn't properly close the iterable, thereby causing a resource leak. _By returning a custom instance of `CloseableIterator` which keeps track of the closable iterable, we properly close the iterable when the iterator gets closed._ 2. When combining an iterable with a closeable using `CloseableIterable.combine` we have a resource leak if the iterable implements `CloseableIterable` because we never call close on the iterable. _By overloading `combine` for `CloseableIterable` we can ensure that we close the iterable in addition to the closeable._ With the following patch applied, the queries and table optimizations ran without OOM and much lower memory limits. ```diff From 6e2cfd71c2dcecf432f2db2119ac530674af5042 Mon Sep 17 00:00:00 2001 From: Emmanuel Pescosta <[email protected]> Date: Thu, 16 Jan 2025 12:19:46 +0100 Subject: [PATCH] fix(iceberg.io): Properly close CloseableIterable when turning it into an iterator A CloseableIterator created from a CloseableIterable didn't close the iterable when the iterator is closed. This may cause a resource leak. --- .../apache/iceberg/io/CloseableIterable.java | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/io/CloseableIterable.java b/api/src/main/java/org/apache/iceberg/io/CloseableIterable.java index 06323612a..240c1ef8f 100644 --- a/api/src/main/java/org/apache/iceberg/io/CloseableIterable.java +++ b/api/src/main/java/org/apache/iceberg/io/CloseableIterable.java @@ -51,7 +51,7 @@ public interface CloseableIterable<T> extends Iterable<T>, Closeable { @Override public CloseableIterator<E> iterator() { - return CloseableIterator.withClose(iterable.iterator()); + return closeableIteratorOf(iterable, this); } }; } @@ -69,7 +69,43 @@ public interface CloseableIterable<T> extends Iterable<T>, Closeable { @Override public CloseableIterator<E> iterator() { - return CloseableIterator.withClose(iterable.iterator()); + return closeableIteratorOf(iterable, this); + } + }; + } + + static <E> CloseableIterable<E> combine(CloseableIterable<E> iterable, Closeable closeable) { + return new CloseableIterable<E>() { + @Override + public void close() throws IOException { + closeable.close(); + iterable.close(); + } + + @Override + public CloseableIterator<E> iterator() { + return closeableIteratorOf(iterable, this); + } + }; + } + + private static <E> CloseableIterator<E> closeableIteratorOf( + Iterable<E> iterable, Closeable closeable) { + Iterator<E> iterator = iterable.iterator(); + return new CloseableIterator<E>() { + @Override + public void close() throws IOException { + closeable.close(); + } + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public E next() { + return iterator.next(); } }; } -- 2.49.0 ``` @jkolash Can you please check if this would fix your problem? -- 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]
