QueryableIndexIndexableAdapter.getRows() should return Sequence rather than Iterable

[leventov]

For better or worse currently Sequence interface is used in Druid in situations when we need to "iterate, then close something when done". QueryableIndexIndexableAdapter.getRows() returns Iterable which closes it's resources when hasNext() failed, that is unreliable (somebody might just not try to iterate to the end of the iterable).

[gianm]

Alternate view: Currently we use Sequence mainly (only?) on the query side of things, and QueryableIndexIndexableAdapter is an indexing side thing. Maybe it makes sense to use Sequences there too but currently we largely don't.

[leventov]

Also Sequence is quite heavy-weight and seems to be supposed to be used to iterate/map/filter large objects like segments, subqueries, etc. For Rows maybe another simpler abstraction could be used, something like CloseableIterable extends Iterable, Closeable.

[navis]

I also thought of removing Sequence several times(even made a patch) but in some micro test comparing MergeSequence and MergeIterable, I've seen the former outperformed the latter. So I just let it go.

[stale]

This issue has been marked as stale due to 280 days of inactivity. It will be closed in 2 weeks if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

[stale]

This issue has been closed due to lack of activity. If you think that is incorrect, or the issue requires additional review, you can revive the issue at any time.