fix: the VIP's lock no longer starts keel-database-follow.path (keel#164) #173

Merged

navigator merged 1 commit from fix/vip-lock-no-path-trigger-164 into main 2026-10-15 12:44:44 -03:00

Conversation 0 Commits 1 Files changed 8 +151 -20

navigator commented 2026-10-15 09:56:35 -03:00

Owner

Copy link

Fixes #164. Stacked on #172.

With etcd, keel-database-follow.path (PathChanged=/var/lib/keel/vip) started keel database follow every 1 to 2 s, and on the replica it failed with unit-start-limit-hit.

Cause

vipstate.locked() opened /var/lib/keel/vip/lock for writing. The VIP controller takes that lock at each turn: take() every 1 s on each node, and carry_held() at each confirmed renewal, every 2 s on the holder. The close of a file opened for writing is IN_CLOSE_WRITE, an event of PathChanged=, even when nothing was written. The numbers match the evidence: a run every 2.2 s on the holder, 5 runs in 7 s on the replica.

Change

• The lock file is opened read only. flock needs no write access, and the close is then IN_CLOSE_NOWRITE. The path does not change, so a helper of the older version shares the lock during the upgrade.
• tests/pathwatch.py watches a directory with systemd's PathChanged mask (ctypes inotify).

Tests

• RED first: the controllers of a steady pair run for 10 s with no event in the watched directory (it failed with [(8, 'lock')] on each node). A move of the VIP still makes an event.
• The netns harness now follows on these inotify events, as systemd does, not on the mtime of the files: the mtime did not change, so CI missed the loop. The netns test checks that a steady pair starts at most 1 follow in 20 s.

Not in this PR: keel diff does not report a failed unit of an enabled overlay as drift. With the cause fixed, the path does not fail.

Fixes #164. Stacked on #172. With etcd, `keel-database-follow.path` (`PathChanged=/var/lib/keel/vip`) started `keel database follow` every 1 to 2 s, and on the replica it failed with `unit-start-limit-hit`. ## Cause `vipstate.locked()` opened `/var/lib/keel/vip/lock` for writing. The VIP controller takes that lock at each turn: `take()` every 1 s on each node, and `carry_held()` at each confirmed renewal, every 2 s on the holder. The close of a file opened for writing is `IN_CLOSE_WRITE`, an event of `PathChanged=`, even when nothing was written. The numbers match the evidence: a run every 2.2 s on the holder, 5 runs in 7 s on the replica. ## Change - The lock file is opened read only. `flock` needs no write access, and the close is then `IN_CLOSE_NOWRITE`. The path does not change, so a helper of the older version shares the lock during the upgrade. - `tests/pathwatch.py` watches a directory with systemd's `PathChanged` mask (ctypes inotify). ## Tests - RED first: the controllers of a steady pair run for 10 s with no event in the watched directory (it failed with `[(8, 'lock')]` on each node). A move of the VIP still makes an event. - The netns harness now follows on these inotify events, as systemd does, not on the mtime of the files: the mtime did not change, so CI missed the loop. The netns test checks that a steady pair starts at most 1 follow in 20 s. Not in this PR: `keel diff` does not report a failed unit of an enabled overlay as drift. With the cause fixed, the path does not fail.

navigator added 4 commits 2026-10-15 09:56:35 -03:00

feat: with etcd, an expired lease states the old PostgreSQL primary gone (keel#165) ... 312e2db482

The maintainer decided keel#165 on 2026-10-15 (handbook 0054, amendment
of 2026-10-15). With etcd, an automatic failover of the PostgreSQL pair
moved the VIP, but the new primary took no write until the operator ran
keel database promote --old-primary-gone.

- The VIP controller claims after the lease of the newest claim is gone.
  When that lease expired at its due time, and the controller saw it
  alive before, the claim states that the old primary is gone. The root
  half writes /var/lib/keel/database/old-primary-gone, with the epoch of
  the claim, before it records the claim. So the follow that the claim
  starts promotes the standby and commits alone.
- A revoked lease, or a lease that the controller never saw alive (after
  a restart of the controller), states nothing. The holder that claims
  again after a revoke states nothing.
- keel database follow takes a statement with no epoch (the operator's)
  or one for the epoch of this node's newest claim. It removes a
  statement of another epoch unused.
- The root half cannot ask etcd again. It trusts the word of its own
  helper for this statement only, and it checks the claim: verified,
  this node's own, above the epoch it held, the previous holder another
  node.
- The netns case (c) now runs with no operator promote: B's controller
  claims when A's lease expired, and B commits alone.

fix: keel database promote makes the database writable before it joins the late asks (keel#169) ... b4a593d1bd

On a real pair with etcd, keel database promote --old-primary-gone made
the database writable 11.9 s and 12.9 s after the VIP moved to it. The
bound of keel#148 is 5 s.

keel vip promote bounds each ask of the gone old primary to 1.5 s, but
it joins the late asks before it returns. A dead node costs the members'
channel's 10 s connect timeout, and keel database promote observed and
followed only after that join. CI did not see it: the harness follows
the VIP's state on its own, as keel-database-follow.path does, and that
path had failed on the real nodes (keel#164).

- vippromote.promote takes `then`, a step that runs once the promote
  succeeded, before the join. keel database promote passes its database
  step there. The promote still joins the asks before it exits.
- A refused promote runs no database step. The operator's statement is
  removed only when the database step did not run.

fix: a PostgreSQL standby caught up after the reseed may take the VIP again (keel#166) ... 10ea05501f

After an automatic failover, the old holder is fenced in its VIP state.
A fenced node never claims by itself (0049). The confirmed reseed did
not clear that fence, so on real nodes the second automatic failover
did not come: the VIP stayed on no node until the operator ran keel
database promote --old-primary-gone.

- keel database follow clears the fence of a PostgreSQL standby that
  streams from the holder and received all that the holder flushed.
  The holder's flushed position comes first, from IDENTIFY_SYSTEM, then
  the standby's received position. Caught up, not only streaming: a
  commit that the holder acknowledged alone is then on this node too.
- The follow does this after a seed or a reseed, and on each run that
  finds the node a standby of the holder.
- keel database watch runs follow on a fenced PostgreSQL standby that
  streams. A node that is fenced now is cleared at its next watch run.
- A node fenced on its own claim stays fenced.
- The netns test checks that the reseeded old primary is not fenced
  within 3 watch runs.

fix: the VIP's lock no longer starts keel-database-follow.path (keel#164) ... 36c447fd76

With etcd, keel-database-follow.path (PathChanged=/var/lib/keel/vip)
started keel database follow every 1 to 2 s on both nodes of the pair.
On the replica it failed with unit-start-limit-hit in a few seconds,
and after that only the watch, every 30 s, saw a VIP move.

The cause: vipstate.locked() opened /var/lib/keel/vip/lock for writing.
The VIP controller takes that lock at each turn: take() every 1 s on
each node, and carry_held() at each confirmed renewal, every 2 s on the
holder. The close of a file opened for writing is IN_CLOSE_WRITE, an
event of PathChanged=, even when nothing was written. The numbers match
the evidence: a run every 2.2 s on the holder, 5 runs in 7 s on the
replica.

- The lock file is opened read only. flock needs no write access, and
  the close is then IN_CLOSE_NOWRITE. The path does not change, so a
  helper of the older version shares the lock during the upgrade.
- tests/pathwatch.py watches a directory with systemd's PathChanged
  mask. A unit test runs the controllers of a steady pair for 10 s and
  sees no event; a move of the VIP still makes one.
- The netns harness follows on these events instead of the mtime of the
  files, as systemd does. The mtime did not change, so CI missed the
  loop. The netns test checks that a steady pair starts at most 1
  follow in 20 s.

navigator referenced this pull request 2026-10-15 09:56:47 -03:00

fix: the VIP rides out an etcd re-election, and the watch starts a stopped holder (keel#167, keel#168) #174

navigator force-pushed fix/vip-lock-no-path-trigger-164 from 36c447fd76 to 197cae4758 2026-10-15 10:30:12 -03:00 Compare

navigator force-pushed fix/vip-lock-no-path-trigger-164 from 36c447fd76 to 857d10b11c 2026-10-15 11:17:08 -03:00 Compare

navigator force-pushed fix/vip-lock-no-path-trigger-164 from 857d10b11c to 2661e8962a 2026-10-15 12:03:23 -03:00 Compare

navigator merged commit 76101f7594 into main 2026-10-15 12:44:44 -03:00

navigator referenced this pull request from a commit 2026-10-15 12:44:46 -03:00

Merge pull request 'fix: the VIP's lock no longer starts keel-database-follow.path (keel#164)' (#173) from fix/vip-lock-no-path-trigger-164 into main

navigator referenced this pull request 2026-10-15 14:32:15 -03:00

ci: flaky tests cost about 1 h per PR (mesh channel TLS EOF, vip carry wait, etcd deadline) #177

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

accessibility

Barrier affecting people with disabilities

  

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

wontfix

This will not be worked on

No labels

accessibility

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Keel-Linux/keel!173

No description provided.