Do not renew sync-id if all shards are sealed#29103
Merged
dnhatn merged 2 commits intoelastic:masterfrom Mar 16, 2018
Merged
Conversation
Today the synced-flush always issues a new sync-id even though all shards haven't been changed since the last seal. This causes active shards to have different a sync-id from offline shards even though all were sealed and no writes since then. This commit adjusts not to renew sync-id if all active shards are sealed with the same sync-id. Closes elastic#27838
Collaborator
|
Pinging @elastic/es-distributed |
bleskes
approved these changes
Mar 16, 2018
Contributor
bleskes
left a comment
There was a problem hiding this comment.
LGTM. I like how contained it is (i.e. not trying to cover all edge cases where some of the shards may have another seal id in place or trying to reuse the primary's sync id marker). I left some simple ask for testing. No need for another cycle
| final ShardsSyncedFlushResult secondSeal = SyncedFlushUtil.attemptSyncedFlush(internalCluster(), shardId); | ||
| assertThat(secondSeal.successfulShards(), equalTo(numberOfReplicas + 1)); | ||
| assertThat(secondSeal.syncId(), equalTo(firstSeal.syncId())); | ||
| // Shards were updated, renew synced flush |
Contributor
There was a problem hiding this comment.
can we inject/remove the marker from one of the shards and see that the synced flush is re-applied?
Member
Author
|
Thanks @bleskes for reviewing. |
dnhatn
added a commit
that referenced
this pull request
Mar 17, 2018
Today the synced-flush always issues a new sync-id even though all shards haven't been changed since the last seal. This causes active shards to have different a sync-id from offline shards even though all were sealed and no writes since then. This commit adjusts not to renew sync-id if all active shards are sealed with the same sync-id. Closes #27838
dnhatn
added a commit
that referenced
this pull request
Mar 17, 2018
Today the synced-flush always issues a new sync-id even though all shards haven't been changed since the last seal. This causes active shards to have different a sync-id from offline shards even though all were sealed and no writes since then. This commit adjusts not to renew sync-id if all active shards are sealed with the same sync-id. Closes #27838
dnhatn
added a commit
that referenced
this pull request
Mar 17, 2018
DaveCTurner
added a commit
that referenced
this pull request
Mar 20, 2018
This reverts commit 25b4d9e.
Member
dnhatn
added a commit
that referenced
this pull request
Mar 20, 2018
Let re-enable this commit, then fix the BWC issue.
dnhatn
added a commit
that referenced
this pull request
Mar 20, 2018
I misunderstood how the bwc versions works. If we backport to 5.x, we need to backport to all supported 6.*. This commit corrects the BWC versions for PreSyncedFlushResponse. Relates #29103
dnhatn
added a commit
that referenced
this pull request
Mar 20, 2018
Today the synced-flush always issues a new sync-id even though all shards haven't been changed since the last seal. This causes active shards to have different a sync-id from offline shards even though all were sealed and no writes since then. This commit adjusts not to renew sync-id if all active shards are sealed with the same sync-id. Closes #27838
dnhatn
added a commit
that referenced
this pull request
Mar 20, 2018
I misunderstood how the bwc versions works. If we backport to 5.x, we need to backport to all supported 6.*. This commit corrects the BWC versions for PreSyncedFlushResponse. Relates #29103
dnhatn
added a commit
that referenced
this pull request
Mar 20, 2018
I misunderstood how the bwc versions works. If we backport to 5.x, we need to backport to all supported 6.*. This commit corrects the BWC versions for PreSyncedFlushResponse. Relates #29103
dnhatn
added a commit
that referenced
this pull request
Mar 20, 2018
Today the synced-flush always issues a new sync-id even though all shards haven't been changed since the last seal. This causes active shards to have different a sync-id from offline shards even though all were sealed and no writes since then. This commit adjusts not to renew sync-id if all active shards are sealed with the same sync-id. Closes #27838
dnhatn
added a commit
that referenced
this pull request
Mar 20, 2018
Today the synced-flush always issues a new sync-id even though all shards haven't been changed since the last seal. This causes active shards to have different a sync-id from offline shards even though all were sealed and no writes since then. This commit adjusts not to renew sync-id if all active shards are sealed with the same sync-id. Closes #27838
dnhatn
added a commit
that referenced
this pull request
Mar 21, 2018
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Today the synced-flush always issues a new sync-id even though all
shards haven't been changed since the last seal. This causes active
shards to have different a sync-id from offline shards even though all
were sealed and no writes since then.
This commit adjusts not to renew sync-id if all active shards are sealed
with the same sync-id.
Closes #27838