Skip to content

Remove GroupShardsIterator and replace it with plain List (#116891)#122253

Merged
original-brownbear merged 1 commit intoelastic:8.xfrom
original-brownbear:116891-8.x
Feb 11, 2025
Merged

Remove GroupShardsIterator and replace it with plain List (#116891)#122253
original-brownbear merged 1 commit intoelastic:8.xfrom
original-brownbear:116891-8.x

Conversation

@original-brownbear
Copy link
Contributor

@original-brownbear original-brownbear commented Feb 11, 2025

There is no point in having GroupShardsIterator, it's mostly an unnecessary layer of indirection as it has no state and a single field only. It's only value could be seen in it hiding the ability to mutate the list it wraps, but that hardly justifies the overhead on the search path and extra code complexity. Moreover, the list it references is not copied/immutable in any way, so the value of hiding is limited also.
back port of #116891

There is no point in having `GroupShardsIterator`, it's mostly an
unnecessary layer of indirection as it has no state and a single field
only. It's only value could be seen in it hiding the ability to mutate
the list it wraps, but that hardly justifies the overhead on the search
path and extra code complexity. Moreover, the list it references is not
copied/immutable in any way, so the value of hiding is limited also.
@original-brownbear original-brownbear added backport :Search Foundations/Search Catch all for Search Foundations labels Feb 11, 2025
@original-brownbear original-brownbear merged commit 697f6a1 into elastic:8.x Feb 11, 2025
15 checks passed
@original-brownbear original-brownbear deleted the 116891-8.x branch February 11, 2025 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments