kvserver: (partially) deflake transfer-leases/drain-other-node#88205
kvserver: (partially) deflake transfer-leases/drain-other-node#88205craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
116c469 to
50df133
Compare
nvb
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @irfansharif)
In cockroachdb#85629 we changed our lease transfer protocol to only ever transfer expiration-based leases, and have recipients later upgrade them to the more efficient epoch based ones. This was done to limit the effects of ill-advised lease transfers since the incoming leaseholder would need to recognize itself as such within a few seconds -- so we wanted this upgrade happen after having received the lease. In cockroachdb#83261 however we noticed that the upgrade was not immediate -- we were waiting until the current lease's expiration was within its renewal duration -- 4.5s. When the lease was eventually renewed the upgrade did happen, but it was not immediate. We fix this here and remove the manual clock advancing the supporting test had that masked this issue. It now demonstrates that we're no longer relying on upgrades happen as part of the (slow) renewal process. Release note: None
50df133 to
991b134
Compare
irfansharif
left a comment
There was a problem hiding this comment.
Had to update some other tests to explicitly wait for lease upgrades to happen, lest they run into errors around proposals being issues under different leases. Will wait on green CI to bors.
Reviewable status:
|
CI failure repros in the commit earlier. bors r+ |
nvb
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status:
|
CI failure is #88051. bors r+ |
|
Already running a review |
|
Build failed (retrying...): |
|
Build succeeded: |
In #85629 we changed our lease transfer protocol to only ever transfer expiration-based leases, and have recipients later upgrade them to the more efficient epoch based ones. This was done to limit the effects of ill-advised lease transfers since the incoming leaseholder would need to recognize itself as such within a few seconds -- so we wanted this upgrade happen after having received the lease.
In #83261 however we noticed that the upgrade was not immediate -- we were waiting until the current lease's expiration was within its renewal duration -- 4.5s. When the lease was eventually renewed the upgrade did happen, but it was not immediate. We fix this here and remove the manual clock advancing the supporting test had that masked this issue. It now demonstrates that we're no longer relying on upgrades happen as part of the (slow) renewal process.
Release note: None