[GitHub] [incubator-pinot] siddharthteotia opened a new issue #4417: Potential resource leak in the way we close collection of closeables in SegmentColumnarIndexCreator

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org