-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Prevent auto-sharding for data streams in LOOKUP index mode #131429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent auto-sharding for data streams in LOOKUP index mode #131429
Conversation
Hi @lukewhiting, I've created a changelog YAML for you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR prevents auto-sharding functionality for data streams that use LOOKUP index mode, as lookup mappers don't support scaling beyond 1 replica. The implementation adds an early return in the auto-sharding calculation logic when the index mode is LOOKUP.
- Adds a check in the auto-sharding service to return NOT_APPLICABLE_RESULT for LOOKUP index mode data streams
- Includes comprehensive test coverage for both scenarios with and without index statistics
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
DataStreamAutoShardingService.java | Adds LOOKUP index mode check to prevent auto-sharding calculation |
DataStreamAutoShardingServiceTests.java | Adds test cases to verify auto-sharding is disabled for LOOKUP index mode |
Comments suppressed due to low confidence (2)
server/src/test/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingServiceTests.java:1347
- [nitpick] The test method name could be more descriptive. Consider renaming to 'testCalculateReturnsNotApplicableForLookupIndexModeWithStats' to better distinguish it from the null stats test case.
public void testCalculateReturnsNotApplicableForLookupIndexMode() {
...va/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingServiceTests.java
Show resolved
Hide resolved
Pinging @elastic/es-data-management (Team:Data Management) |
return NOT_APPLICABLE_RESULT; | ||
} | ||
|
||
if (dataStream.getIndexMode() == IndexMode.LOOKUP) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change lgtm!
if you wouldn't mind me asking a few (hopefully not dumb) Qs just to learn, just if you know them off the top of your head, i can investigate anything you might not know
- terminology: it seems in the code and slack discussion regarding the ticket, it seems we use the term
shard
to only meanwrite shard
, but we also havereplica/read shards
right? just wondering whether whenever i see the termshard
should i basically assume this meanswrite shard/primary
- so LOOKUP is a type of index, that only only ever has one primary shard, but can it have more than 1 replica/read shard? assuming it can, do we have any replica auto-sharding in place in data streams if the read load gets too heavy?
- why does LOOKUP only ever allow one primary shard? it's always possible it has heavy writes (hence the error for scaleup i'm assuming), is it basically just a mode where you're assuming up-front write volume is low and it's -- as in the name -- just a lookup
- for this auto-sharding result logic, if we scale up, do we roll-over to a new index with more primaries or split? and if rollover the new index has more primaries right
- for the ticket, did the lookup index just completely fail to rollover and kept accumulating data, or it just threw the validation exceptions as this kept being called to scale-up but eventually was rolled over due to size/time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@szybia We can chat about this in our 1:1 tomorrow if you like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please! might take the load off luke...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM also
💔 Backport failed
You can use sqren/backport to manually backport by running |
…131429) * Prevent auto-sharding for data streams in LOOKUP index mode * Update docs/changelog/131429.yaml * Reduce test duplication
…131429) * Prevent auto-sharding for data streams in LOOKUP index mode * Update docs/changelog/131429.yaml * Reduce test duplication (cherry picked from commit ea22dff) # Conflicts: # server/src/main/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingService.java # server/src/test/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingServiceTests.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…131429) * Prevent auto-sharding for data streams in LOOKUP index mode * Update docs/changelog/131429.yaml * Reduce test duplication (cherry picked from commit ea22dff) # Conflicts: # server/src/main/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingService.java # server/src/test/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingServiceTests.java
…#132079) * Prevent auto-sharding for data streams in LOOKUP index mode * Update docs/changelog/131429.yaml * Reduce test duplication (cherry picked from commit ea22dff) # Conflicts: # server/src/main/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingService.java # server/src/test/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingServiceTests.java
…#132082) * Prevent auto-sharding for data streams in LOOKUP index mode * Update docs/changelog/131429.yaml * Reduce test duplication (cherry picked from commit ea22dff) # Conflicts: # server/src/main/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingService.java # server/src/test/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingServiceTests.java
…#132080) * Prevent auto-sharding for data streams in LOOKUP index mode * Update docs/changelog/131429.yaml * Reduce test duplication (cherry picked from commit ea22dff) # Conflicts: # server/src/main/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingService.java # server/src/test/java/org/elasticsearch/action/datastreams/autosharding/DataStreamAutoShardingServiceTests.java
This PR disabled the datastream autosharding for LOOKUP indices to prevent them scaling above 1 replica which is unsupported by the lookup mappers.
Fixes ES-12330