Track Shard Snapshot Generation in CS #46864
Conversation
Adds communication of new shard generations from datanodes to master and tracking of those generations in the CS. This is a preliminary to elastic#46250
original-brownbear
added
>non-issue
:Distributed Coordination/Snapshot/Restore
|
Pinging @elastic/es-distributed |
| return nodeId; | ||
| } | ||
|
|
||
| public String generation() { |
There was a problem hiding this comment.
Tracking the generation here as well as via the String callback to have it available for retries of shard status updates on e.g. master failover in SnapshotShardsService.
| if (primary == null || !primary.assignedToNode()) { | ||
| builder.put(shardId, | ||
| new SnapshotsInProgress.ShardSnapshotStatus(null, ShardState.MISSING, "primary shard is not allocated")); | ||
| new SnapshotsInProgress.ShardSnapshotStatus(null, ShardState.MISSING, "primary shard is not allocated", |
There was a problem hiding this comment.
Passing null for the generations for now since we don't know them when starting a shard snapshot yet. #46250 will make use of the ability to pass the shard-generation via the CS by determining the initial shard gen values from the repositories' RepositoryData here.
There was a problem hiding this comment.
Ok - Maybe add a comment that will be later removed?
…-via-network-for-46250
…-via-network-for-46250
|
@tlrx let me know what you think about this for an increment here :) Obviously it's a little artificial to track the shards' generations here without using them though on the bright side it makes debugging things a little easier in isolation sine we get an additional bit of synchronization between the master and the data nodes. |
tlrx
left a comment
tlrx
left a comment
There was a problem hiding this comment.
LGTM! I had to run the code a bit to validate my understanding because I found this change surprisingly simple :) I left minor comments.
@tlrx let me know what you think about this for an increment here :)
That's good for me. Thanks for splitting things as it makes things much more easier to review.
I would only merge it to
master/8.0for now and then do the complete backport of #46250 in one go later to reduce the noise from this change.
+1 too. Don't forget the backport pending label.
server/src/main/java/org/elasticsearch/index/snapshots/IndexShardSnapshotStatus.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java
Outdated
Show resolved
Hide resolved
| if (primary == null || !primary.assignedToNode()) { | ||
| builder.put(shardId, | ||
| new SnapshotsInProgress.ShardSnapshotStatus(null, ShardState.MISSING, "primary shard is not allocated")); | ||
| new SnapshotsInProgress.ShardSnapshotStatus(null, ShardState.MISSING, "primary shard is not allocated", |
There was a problem hiding this comment.
Ok - Maybe add a comment that will be later removed?
test/framework/src/main/java/org/elasticsearch/index/shard/IndexShardTestCase.java
Show resolved
Hide resolved
…-via-network-for-46250
|
Jenkins run elasticsearch-ci/1 (dep. download fail) |
|
Thanks Tanguy
Yea the CS + network/serialization code is a lot cleaner than the code around |
original-brownbear
deleted the
send-new-shard-gen-via-network-for-46250
branch
* Track Shard Snapshot Generation in CS Adds communication of new shard generations from datanodes to master and tracking of those generations in the CS. This is a preliminary to elastic#46250
4 participants
Adds communication of new shard generations from datanodes to master
and tracking of those generations in the CS.
This is a preliminary to #46250 extracted to a separate PR for simpler review => I would only merge it to
master/8.0for now and then do the complete backport of #46250 in one go later to reduce the noise from this change.