[8.14] [Bug] During heavy indexing load it's possible for lazy rollover to trigger multiple rollovers (#109636) #110031
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rigger multiple rollovers (elastic#109636) Let’s say we have `my-metrics` data stream which is receiving a lot of indexing requests. The following scenario can result in multiple unnecessary rollovers: 1. We update the mapping and mark it to be lazy rolled over 2. We receive 5 bulk index requests that all contain a write request for this data stream. 3. Each of these requests are being picked up “at the same time”, they see that the data stream needs to be rolled over and they issue a lazy rollover request. 4. Currently, data stream my-metrics has 5 tasks executing an unconditional rollover. 5. The data stream gets rolled over 5 times instead of one. This scenario is captured in the `LazyRolloverDuringDisruptionIT`. We have witnessed this also in the wild, where a data stream was rolled over 281 times extra resulting in 281 empty indices. This PR proposes: - To create a new task queue with a more efficient executor that further batches/deduplicates the requests. - We add two safe guards, the first to ensure we will not enqueue the rollover task if we see that a rollover has occurred already. The second safe guard is during task execution, if we see that the data stream does not have the `rolloverOnWrite` flag set to `true` we skip the rollover. - When we skip the rollover we return the following response: ``` { "acknowledged": true, "shards_acknowledged": true, "old_index": ".ds-my-data-stream-2099.05.07-000002", "new_index": ".ds-my-data-stream-2099.05.07-000002", "rolled_over": false, "dry_run": false, "lazy": false, } ```
gmarouli
added
:Data Management/Data streams
nielsbauman
approved these changes
Jun 21, 2024
| @@ -374,7 +374,7 @@ static Condition.Stats buildStats(@Nullable final IndexMetadata metadata, @Nulla | |||
| .flatMap(Arrays::stream) | |||
| .filter(shard -> shard.getShardRouting().primary()) | |||
| .map(ShardStats::getStats) | |||
| .mapToLong(shard -> shard.docs.getCount()) | |||
| .mapToLong(shard -> shard.docs == null ? 0L : shard.docs.getCount()) | |||
There was a problem hiding this comment.
Are these changes (this one and the one a couple of lines higher) intentional? I see they're also on main, so I think they're ok, but they're not part of your original PR.
There was a problem hiding this comment.
That is true, I intentionally chose to leave them because it seems to me like a bug fix than a feature.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport
This will backport the following commits from
mainto8.14:Questions ?
Please refer to the Backport tool documentation