Repository navigation
fix: a restart of the VIP's unit renews the lease once before it goes (keel#87) - #145
Open
marcos-mendez wants to merge 1 commit into
Open
marcos-mendez wants to merge 1 commit into
marcos-mendez wants to merge 1 commit into
Conversation
… (keel#87) vip-upgrade / trixie failed on this branch's sibling (#143) and on main: after a try-restart of keel-vip.service on the holder, a sample found no node carrying the VIP, 4.47 s after the restart. The cause: an address kept across a restart has the lifetime of the old controller's last confirmed renewal (RELEASE_AFTER 10 s, less its age and a second of slack), and the next controller extends it only once it renewed the lease itself. On the test's link (250 ms, 2% loss), the last renewal is often seconds old when the restart comes (each renewal is a keep-alive and a linearizable read, a TLS connection each), and the stop, the start, and the first renewal of the next controller took longer than what was left: the kernel removed the address until the new controller carried it again. keel vip tend --stopped (the unit's ExecStopPost) now renews the lease once while the unit restarts, as the controller does, and gives the address the lifetime that renewal leaves, so the next controller has the whole release time. The safety bound is kept: the lifetime comes from a renewal the majority confirmed. With no answer from etcd the address stays as it was, the kernel's to end; a lease gone drops it. Tests: a restart with a 5 s old renewal renews once (keep-alive, read) and gives the address the full lifetime; no answer leaves it; a lease gone drops it; a stop renews nothing; docs/vip.md says so. Version 0.23.18 (#144 holds 0.23.17).
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
1 participant
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.
keel#87 promised that the VIP survives an upgrade.
vip-upgrade / trixiefailed on #143 and on main: after a try-restart of keel-vip.service on the holder, a sample found no node carrying the VIP, 4.47 s after the restart (dropped_s: 4.47,holders_seen: [[], ['A']]).Cause
An address kept across a restart keeps the lifetime of the old controller's last confirmed renewal: RELEASE_AFTER (10 s), less the renewal's age, less 1 s of slack. The next controller extends it only after it has renewed the lease itself.
On the test's link (250 ms ±25 ms, 2% loss), each renewal is a keep-alive plus a linearizable read, each over a new TLS connection. So the last renewal is often seconds old when the restart comes. The stop (1.19 s in the failed run), the start, and the next controller's first renewal took longer than the time left, and the kernel removed the address until the new controller carried it again. This is a product gap, not a test artifact: the sampler already reads each false result a second time (keel#94).
Fix
keel vip tend --stopped(the unit's ExecStopPost,keel/mesh/vipetcd.py:stopped,handed_over) now renews the lease once while the unit restarts, as the controller does. It then gives the address the lifetime that this renewal leaves, so the next controller has the whole release time. The safety bound stays: the lifetime comes from a renewal that the majority confirmed. With no answer from etcd, the address stays as it was and the kernel ends it on time. A lease that is gone drops it. A plain stop renews nothing.docs/vip.mdsays so. Version 0.23.18 (#144 holds 0.23.17).Tests
tests/test_mesh_vipupgrade.py(TestHandOver). A restart with a renewal 5 s old renews once (keep-alive, then read) and gives the address the full lifetime. No answer leaves the lifetime as it is. A lease that is gone drops the address. A stop renews nothing. An address not carried again is said. The default client is this member's etcd. These tests failed before the fix (stopped()had no renewal).Proof on the platform:
vip-upgrade / trixiemust pass 5 times on this head. I record each run below.