sentinel: fix check for same reported vs spec sync standbys#494
Merged
sgotti merged 1 commit intoMay 29, 2018
Merged
Conversation
nh2
reviewed
May 28, 2018
| {[]string{"a", "b", "c"}, []string{"a", "b", "c"}, true}, | ||
| {[]string{"a", "b", "c"}, []string{"b", "c", "a"}, true}, | ||
| {[]string{"a", "b", "c", "a"}, []string{"a", "c", "b", "b"}, false}, | ||
| {[]string{"a", "b", "c", "a"}, []string{"a", "c", "b", "b"}, false}, |
Contributor
There was a problem hiding this comment.
May want to add some cases where the two slices have different length to get coverage of the if len(a) != len(b) { return false } bit (which is what makes it correct e.g. for {abc, abcd, false}).
Member
Author
There was a problem hiding this comment.
It's covered by the second test (the one with empty strings) but I'll add a more explicit test with non empty strings to make it clear.
Contributor
|
That's great, thanks! |
When electing a new master we are swapping the new master uid with the old master uid in the syncstandbys slice. This could end with an unordered slice in the spec that will make the sentinel fail the check that the reported standbys are the same of the spec one blocking any future syncstandby update. Since the reported order is not a problem just check that the syncstandbys are the same regardless of their order. We'll keep the sorting to avoid unneeded updates to synchronous_standby_names by the keeper.
sgotti
force-pushed
the
fix_reported_vs_spec_syncstandbys_check
branch
from
b3992d0 to
be61e2c
Compare
sgotti
added a commit
that referenced
this pull request
May 29, 2018
…_check sentinel: fix check for same reported vs spec sync standbys
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
2 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.
When electing a new master we are swapping the new master uid with the old
master uid in the syncstandbys slice. This could end with an unordered slice in
the spec that will make the sentinel fail the check that the reported standbys
are the same of the spec one blocking any future syncstandby update.
Since the reported order is not a problem just check that the syncstandbys are
the same regardless of their order.
We'll keep the sorting to avoid unneeded updates to synchronous_standby_names by
the keeper.