siddharthteotia opened a new issue #4417: Potential resource leak in the way we close collection of closeables in SegmentColumnarIndexCreator URL: https://github.com/apache/incubator-pinot/issues/4417 It looks like there is a potential resource leak in in the way we are closing collection/array of closeable resources. While working on https://github.com/apache/incubator-pinot/pull/4368, I saw this pattern in [SegmentColumnarIndexCreator](https://github.com/apache/incubator-pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreator.java#L535) If there is an exception in the middle of close() calls, then subsequent resources won't be cleaned. The close() on SegmentDictionaryCreator is NO-OP. However, close() for ForwardIndexCreator and InvertedIndexCreator can throw exceptions. I believe we should we should write a custom utility class for closing a collection (or multiple collections) of closeables. This utility class can have a method to take a Iterable<?extends Closeable> and for each closeable, it invokes close(), catches the exception if any and maintains a top level exception along with a set of suppressed exceptions. If the caller has multiple collection of closeable resources, it can concat them and then invoke the close() method on utility class. Further looking into what each close() does, I observed a couple of other things: (1) The above problem is also there in [OffHeapBitmapInvertedIndexCreator.close()](https://github.com/apache/incubator-pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/inv/OffHeapBitmapInvertedIndexCreator.java#L214) (2) Also, it's [destroyBuffer](https://github.com/apache/incubator-pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/inv/OffHeapBitmapInvertedIndexCreator.java#L241) method doesn't seem to be idempotent. (3) The [close() ](https://github.com/apache/incubator-pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/fwd/SingleValueSortedForwardIndexCreator.java#L56)on SingleValueSortedForwardIndexCreator will leak the writer if any of the preceding data set operations throw exceptions. (4) The [close()](https://github.com/apache/incubator-pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/io/writer/impl/FixedByteSingleValueMultiColWriter.java#L102) on FixedByteSingleValueMultiColWriter is not idempotent.
---------------------------------------------------------------- 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]
