storage: Always gossip system config after lease changes hands #17285
Conversation
This file contains 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
Without doing this, it's possible for n1 to gossip the system config, lose its lease, and restart with the source node of the system config gossip still being n1. If this happens, other nodes will not provide the system config gossip back to n1 because its watermark for itself will be higher than the system config's timestamp. I think it's better to add this "last-gossiped-from-here" check to the universal code path for gossiping the system config than to change only the leaseChangingHands code path, since it's possible that one of the other checks or an error preventing gossip could trigger from there and then we wouldn't know the next time through that we should gossip it.
|
This change is |
petermattis
added a commit
to petermattis/cockroach
that referenced
this pull request
Feb 1, 2019
Similar to cockroachdb#17285. We need to always gossip node liveness infos after the node-liveness range lease changes hands due to the way gossip handles propagation of infos which "loopback" to the sender. Due to the highwater stamp mechanism, such infos are never sent back to the sender. If the gossip info is subsequently never re-gossiped, a node in the cluster may never see the info. This is only a problem for gossip keys which can be gossiped from multiple nodes, such as the node liveness keys and the system config key. The other keys which can be gossiped from multiple nodes are the table-stats-added key, and the table-disable-merges key, neither of which appear to have problematic scenarios associated with the failure to propagate them back to the sender after a restart. See cockroachdb#34494 Release note: None
petermattis
added a commit
to petermattis/cockroach
that referenced
this pull request
Feb 5, 2019
Similar to cockroachdb#17285. We need to always gossip node liveness infos after the node-liveness range lease changes hands due to the way gossip handles propagation of infos which "loopback" to the sender. Due to the highwater stamp mechanism, such infos are never sent back to the sender. If the gossip info is subsequently never re-gossiped, a node in the cluster may never see the info. This is only a problem for gossip keys which can be gossiped from multiple nodes, such as the node liveness keys and the system config key. The other keys which can be gossiped from multiple nodes are the table-stats-added key, and the table-disable-merges key, neither of which appear to have problematic scenarios associated with the failure to propagate them back to the sender after a restart. See cockroachdb#34494 Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Without doing this, it's possible for n1 to gossip the system config,
lose its lease, and restart with the source node of the system config
gossip still being n1. If this happens, other nodes will not provide the
system config gossip back to n1 because its watermark for itself will be
higher than the system config's timestamp.
I think it's better to add this "last-gossiped-from-here" check to the
universal code path for gossiping the system config than to change only
the leaseChangingHands code path, since it's possible that one of the
other checks or an error preventing gossip could trigger from there and
then we wouldn't know the next time through that we should gossip it.
This fixes the main problem seen in #17272