kvserver: allow term and generation increments #105044
Conversation
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
There was a problem hiding this comment.
TFTR - good catch on the comments. Snapshots are "loosely coupled" with other replica and lease movements, so it needs to be more tolerant of forward changes to these values but not backward values.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @pavelkalinnikov)
pkg/kv/kvserver/replica_command.go line 2917 at r1 (raw file):
Previously, pavelkalinnikov (Pavel Kalinnikov) wrote…
Could you update the
validateSnapshotDelegationRequestmethod comment accordingly? It currently speaks in terms of term/gen equality.Is the comment above up-to-date? Not sure what "if there are merges or splits, it is safer to reject" means, but don't we enable this "unsafe" scenario with this change?
Updated the comment. Forward changes to term and generation are OK, but backwards changes are not.
pkg/kv/kvserver/replica_command.go line 2968 at r1 (raw file):
Previously, pavelkalinnikov (Pavel Kalinnikov) wrote…
Is the comment above up-to-date?
Update the comment here also.
Previously delegated snapshots had a check where it would not allow delegation if the term or generation were different between the sender and the recipient. This was incorrect as on the coordinator it was possible that the term and generation could increment. This change allows the sending of changes as long as the sender (leaseholder or delegate) has an equal or greater term or generation. Epic: none Fixes: cockroachdb#104477 Release note: None
andrewbaptist
force-pushed
the
2023-06-15-fix-delegate-check
branch
from
d21647b
to
fe3f6c2
Compare
|
bors r=pavelkalinnikov |
|
Build failed: |
|
bors retry |
|
Build succeeded: |
|
blathers backport 23.1 |
Previously delegated snapshots had a check where it would not allow delegation if the term or generation were different between the sender and the recipient. This was incorrect as on the coordinator it was possible that the term and generation could increment. This change allows the sending of changes as long as the sender (leaseholder or delegate) has an equal or greater term or generation.
Epic: none
Fixes: #104477
Release note: None