cassandra icon indicating copy to clipboard operation
cassandra copied to clipboard

Fix RangeIntersectionIterator resource leak/superfluous wrapping of input iterator

Open jkni opened this issue 9 months ago • 2 comments

These were regressions introduced several months ago in the implementation of improved query planning. The build method originally took a ranges argument that was called with rangeIterators, so these correctly referred to the argument. When these arguments were removed, ranges started referring to a field of the parent RangeIterator.Builder class.

jkni avatar May 06 '24 15:05 jkni

@michaeljmarshall @pkolaczk Git blame suggests you both as reviewers here but feel free to point me elsewhere if you don't have bandwidth.

jkni avatar May 06 '24 18:05 jkni

Michael's comments inspired me to realize that Plan.Intersection and Plan.Union would call ranges() if an exception is thrown during execute, returning the empty RangeIterator ranges to be closed and leaking iterators. Since no classes extending RangeIterator.Builder ever actually use the ranges field and they all override add methods to use their own collections, I removed this from RangeIterator.Builder entirely, preventing us from making this mistake.

jkni avatar May 06 '24 19:05 jkni