Snapshot Stability Fixes#39502
Snapshot Stability Fixes#39502original-brownbear merged 2 commits intoelastic:6.7from original-brownbear:backport-snapshot-fixes-6.7
Conversation
|
Pinging @elastic/es-distributed |
There was a problem hiding this comment.
We will have to mute BwC tests before merging this and then adjust this version to 6.7 in master afterwards, currently we have a 7.0.0 here in master.
There was a problem hiding this comment.
We will have to mute BwC tests before merging this and then adjust this version to 6.7 in master afterwards, currently we have a 7.0.0 here in master.
There was a problem hiding this comment.
passing the settings here (we don't have that argument in master anymore) is the only difference between this PR and master in this file.
There was a problem hiding this comment.
This class should be exactly like it is in master now
|
seems I missed a bwc spot, looking into it now |
original-brownbear
left a comment
There was a problem hiding this comment.
@ywelsch Added some notes and fixed BwC here now, should be good to review :)
| final DiscoveryNode masterNode) { | ||
| void sendSnapshotShardUpdate(Snapshot snapshot, ShardId shardId, ShardSnapshotStatus status, DiscoveryNode masterNode) { | ||
| try { | ||
| if (masterNode.getVersion().onOrAfter(Version.V_6_1_0)) { |
There was a problem hiding this comment.
I simply made it a hard condition here between the old and the new path, the old path doesn't use the de-duplicator.
I figured the optimization isn't really important in the rolling upgrade case and this makes future backports a lot easier than having the conditionals for that in the old code, right?
| logger.debug("[{}] new master thinks the shard [{}] is not completed but the shard is done locally, " + | ||
| "updating status on the master", snapshot.snapshot(), shardId); | ||
| notifySuccessfulSnapshotShard(snapshot.snapshot(), shardId, localNodeId, masterNode); | ||
| notifySuccessfulSnapshotShard(snapshot.snapshot(), shardId, masterNode); |
There was a problem hiding this comment.
All this passing down of masterNode is different from the 7.x/master version since we need the Bwc path in the status update sending below.
|
@ywelsch thanks! |
This reverts commit 4b725e0.
masterto6.7making the snapshot logic in6.7equivalent to that inmasterfunctionally