-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add split recovery source #124834
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
Add split recovery source #124834
Conversation
|
Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing) |
| case PEER -> PeerRecoverySource.INSTANCE; | ||
| case SNAPSHOT -> new SnapshotRecoverySource(in); | ||
| case LOCAL_SHARDS -> LocalShardsRecoverySource.INSTANCE; | ||
| case SPLIT -> SplitRecoverySource.INSTANCE; |
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.
Can we add this recovery type to the top of this class where the other ones are also listed.
| } | ||
|
|
||
| /** | ||
| * peer recovery from a primary shard |
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.
Nit: peer -> "reshard split" perhaps ? Can we add a little more description here like this is for reshard when we are increasing number of shards and it is different from the existing split functionality we have ?
| return maybeUpdatedState; | ||
| } | ||
|
|
||
| private static boolean invalidShardSplit(StartedShardEntry startedShardEntry, ProjectId projectId, ClusterState clusterState) { |
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.
nit, do we have a custom of invalid functions that return false if the state is valid? It seems like a double negative and using valid would be more natural to me, unless that is against custom.
| IndexReshardingMetadata reshardingMetadata = indexMetadata.getReshardingMetadata(); | ||
| assert reshardingMetadata != null; | ||
| IndexReshardingState.Split split = reshardingMetadata.getSplit(); | ||
| int sourceShardId = startedShardEntry.shardId.getId() % split.shardCountBefore(); |
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.
should we add a function to IndexReshardingState.Split to return the source shard number for a given target? I know it's only modulo but it might be nice to have this kind of logic in one place.
| assert indexMetadata != null; | ||
| IndexReshardingMetadata reshardingMetadata = indexMetadata.getReshardingMetadata(); | ||
| assert reshardingMetadata != null; | ||
| IndexReshardingState.Split split = reshardingMetadata.getSplit(); |
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.
I suppose we could have getSplit return null if reshardingMetadata is null to avoid having to do this two step check.
Add a new split recovery source. This recovery type will always be
allocated with a primary term + 1 of the source shard. And will validate
the source primary term has not changed when starting the shard.