Avoid walking the complete list of search contexts on shard creation #123855
Conversation
This I found in the many-shards benchmark during some manual testing. Creating indices slows down measurably when there's concurrent searches going on. Interestingly enough, the bulk of the cost is coming from this hook. This makes sense to some extend because the map can quickly grow to a massive size as it scales as O(shards_searched_on_average * concurrent_searches) and a CHM generally is anything but cheap to iterate over.
original-brownbear
added
>non-issue
:Search Foundations/Search
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
elasticsearchmachine
added
the
Team:Search Foundations
|
The comment in that method: "we prefer to stop searches to restore full availability as fast as possible" makes me think that allowing searches to continue also slows down index creation - did your change actually speed up the index creation as a whole? Maybe I'm misunderstanding the comment... but it reads like: 'freeing the contexts is supposed to speed things up,' and you're saying 'trying to speed things up is slow, let's skip it to go faster.' Another thought on:
I wonder, does it make sense to maintain a different data structure of active readers per shard? That would speed up |
Yea this definitely should just live on
I think it's more like: "freeing contexts is sometimes slow and more importantly slows down under contention, here we can skip it because it does not do anything anyway to remove some contention introduced by index creation". You're 100% right, the current approach is not referencing the contexts from the correct place and dealing with that would be a much stronger fix, I just figured that would have a harder time getting a review in the short term and this change made my benchmarking easier to interpret when I opened it :D (and also still removes some contention/noise from heavily loaded production environments) |
original-brownbear
added
the
auto-backport
|
Thanks Ben! |
💚 Backport successful
|
…lastic#123855) This I found in the many-shards benchmark during some manual testing. Creating indices slows down measurably when there's concurrent searches going on. Interestingly enough, the bulk of the cost is coming from this hook. This makes sense to some extend because the map can quickly grow to a massive size as it scales as O(shards_searched_on_average * concurrent_searches) and a CHM generally is anything but cheap to iterate over.
…123855) (#126798) This I found in the many-shards benchmark during some manual testing. Creating indices slows down measurably when there's concurrent searches going on. Interestingly enough, the bulk of the cost is coming from this hook. This makes sense to some extend because the map can quickly grow to a massive size as it scales as O(shards_searched_on_average * concurrent_searches) and a CHM generally is anything but cheap to iterate over.
This I found in the many-shards benchmark during some manual testing. Creating indices slows down measurably when there's concurrent searches going on. Interestingly enough, the bulk of the cost is coming from this hook. This makes sense to some extend because the map can quickly grow to a massive size as it scales as O(shards_searched_on_average * concurrent_searches) and a CHM generally is anything but cheap to iterate over.
=> no need to do this iteration if we're creating a new shard.