fix: only the other member of the pair gates a promote; the others are told after (keel#137) #141

Open

marcos-mendez wants to merge 1 commit from fix/promote-announce-in-background into fix/claim-txn-bounded-retry

marcos-mendez commented

2026-10-10 14:49:30 -03:00

(Migrated from github.com)

Fixes #137. Stacked on #136 (both bump debian/changelog); its base is #136's branch.

Problem

In cloud simple, one member outside the pair that was down (web-2) made every keel vip promote wait two 10 s timeouts on it: once for its epoch, once for the announcement. On a real pair a planned promote took about 21 s, and the write gap went up to 11 s. The new primary's follow waits for the VIP on wg0, so it also waited those 10 s; this is the 10 s of #138.

Change

Before etcd (keel/mesh/vippromote.py, promoted):

the promote asks only the other member of the pair for its epoch, asks it to release, and tells it the claim;
each of these is waited for in full: the no-etcd safety rule of #121 stays, because nothing fences a live old primary without etcd;
the VIP is carried only once that member has taken the claim; when it refuses or does not answer, the claim is released and the promote exits non-zero;
the members outside the pair are told after, in a thread that is not a daemon (in_background), and what each said is logged.

A replica that holds the primary's claim asks the release at the next epoch at once, and asks the epoch only when the primary refuses that as stale (then the release again): two round trips with the other member, not three. With three, the first CI run measured 3.44 s.

With --old-primary-gone (promoted_by_all), every peer is asked and told, and the claim is carried on a majority of those that answer, as before. With etcd, the peers are told in the background too.

Also: keel/mesh/vipnode.py (epochs and announce take only), docs/vip.md, debian/changelog 0.23.15.

Tests

tests/test_mesh_vipmove.py:
- a member outside the pair that is down does not hold up the promote (under 0.8 s with the hung peer, its result logged);
- such a member learns the claim after;
- no epoch is asked that is known here, and a stale release asks it, then asks the release again;
the other member refusing the claim stops it;
- a first promote with the other member down is refused, and taken with --old-primary-gone.
tests/vip_helpers.py turns the background off for the older tests, which read the peers right after a promote.
New job vip-simple / trixie (tests/test_vip_simple_netns.py): every spec in cloud simple, C (outside the pair) cut off. A planned promote must take the VIP in less than 3 s at 250 ms and 2 % loss, and tests/vip_overlap.py must prove that no two nodes carry it at one instant.

Measured (`vip-simple / trixie`, `191f4c4`, 250 ms ±25 ms, 2 % loss)

Planned promote with C (outside the pair) cut off: 1.9 s (keel 0.23.13 on real nodes: about 21 s). No two nodes carried the VIP at one instant.

CI record

Green at 191f4c4 at the first run, no rerun. The first head (9988ec4) failed vip-simple / trixie (3.44 s, three round trips, fixed by the change above), tests / coverage (an older test, test_no_peer_answering_is_no_majority, expected the majority message; updated), and the coverage job's test_mesh_netns timed out once.

Test plan

tests / coverage
vip-simple / trixie (the promote's time is printed)
vip, vip-claim, vip-upgrade, etcd, etcd-auth, mariadb, mesh / trixie
package / changelog

Fixes #137. Stacked on #136 (both bump `debian/changelog`); its base is #136's branch. ## Problem In cloud simple, one member outside the pair that was down (web-2) made every `keel vip promote` wait two 10 s timeouts on it: once for its epoch, once for the announcement. On a real pair a planned promote took about 21 s, and the write gap went up to 11 s. The new primary's follow waits for the VIP on `wg0`, so it also waited those 10 s; this is the 10 s of #138. ## Change Before etcd (`keel/mesh/vippromote.py`, `promoted`): - the promote asks only the other member of the pair for its epoch, asks it to release, and tells it the claim; - each of these is waited for in full: the no-etcd safety rule of #121 stays, because nothing fences a live old primary without etcd; - the VIP is carried only once that member has taken the claim; when it refuses or does not answer, the claim is released and the promote exits non-zero; - the members outside the pair are told after, in a thread that is not a daemon (`in_background`), and what each said is logged. A replica that holds the primary's claim asks the release at the next epoch at once, and asks the epoch only when the primary refuses that as stale (then the release again): two round trips with the other member, not three. With three, the first CI run measured 3.44 s. With `--old-primary-gone` (`promoted_by_all`), every peer is asked and told, and the claim is carried on a majority of those that answer, as before. With etcd, the peers are told in the background too. Also: `keel/mesh/vipnode.py` (`epochs` and `announce` take `only`), `docs/vip.md`, `debian/changelog` 0.23.15. ## Tests - `tests/test_mesh_vipmove.py`: - a member outside the pair that is down does not hold up the promote (under 0.8 s with the hung peer, its result logged); - such a member learns the claim after; - no epoch is asked that is known here, and a stale release asks it, then asks the release again; - the other member refusing the claim stops it; - a first promote with the other member down is refused, and taken with `--old-primary-gone`. - `tests/vip_helpers.py` turns the background off for the older tests, which read the peers right after a promote. - New job `vip-simple / trixie` (`tests/test_vip_simple_netns.py`): every spec in cloud simple, C (outside the pair) cut off. A planned promote must take the VIP in less than 3 s at 250 ms and 2 % loss, and `tests/vip_overlap.py` must prove that no two nodes carry it at one instant. ## Measured (`vip-simple / trixie`, 191f4c4, 250 ms ±25 ms, 2 % loss) Planned promote with C (outside the pair) cut off: **1.9 s** (keel 0.23.13 on real nodes: about 21 s). No two nodes carried the VIP at one instant. ## CI record Green at 191f4c4 at the first run, no rerun. The first head (9988ec4) failed `vip-simple / trixie` (3.44 s, three round trips, fixed by the change above), `tests / coverage` (an older test, `test_no_peer_answering_is_no_majority`, expected the majority message; updated), and the coverage job's `test_mesh_netns` timed out once. ## Test plan - [ ] tests / coverage - [ ] vip-simple / trixie (the promote's time is printed) - [ ] vip, vip-claim, vip-upgrade, etcd, etcd-auth, mariadb, mesh / trixie - [ ] package / changelog

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/promote-announce-in-background:fix/promote-announce-in-background

git switch fix/promote-announce-in-background

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch fix/claim-txn-bounded-retry

git merge --no-ff fix/promote-announce-in-background

git switch fix/promote-announce-in-background

git rebase fix/claim-txn-bounded-retry

git switch fix/claim-txn-bounded-retry

git merge --ff-only fix/promote-announce-in-background

git switch fix/promote-announce-in-background

git rebase fix/claim-txn-bounded-retry

git switch fix/claim-txn-bounded-retry

git merge --no-ff fix/promote-announce-in-background

git switch fix/claim-txn-bounded-retry

git merge --squash fix/promote-announce-in-background