Peer liveness ping targets /health (constant 200), so a mid-life DB outage doesn't drain peer traffic #176

Activity

1. beardthelion

   added

   sev:mediumDegraded but workaround existsDegraded but workaround exists

   crate:nodegitlawb-node — the serving node and REST APIgitlawb-node — the serving node and REST API

   kind:bugDefect fix — wrong or unsafe behaviorDefect fix — wrong or unsafe behavior

   subsystem:peersPeer announce, discovery, and registryPeer announce, discovery, and registry

   on Jul 10, 2026
2. beardthelion commented on Jul 27, 2026

   

   beardthelion

   on Jul 27, 2026

   CollaboratorAuthor

   More actions

   Adding a second gap here rather than opening a separate issue, because it changes what "fixed" means for this one.

   The direction above is to point the peer liveness ping at /ready. That is right, but /ready has the same shallowness problem one layer down: it probes only state.db.ping(), a single SELECT 1. It answers "is the pool alive", not "can this node serve".

   That distinction did not matter much when the database was either up or down. PR #261 makes it matter. It adds a spent-signature ledger that every mutation route charges before the handler runs, and that layer fails closed: any error touching consumed_signatures returns 503. The failure modes are ones the pool does not share, a statement timeout confined to the new table, lock contention on it, index bloat, so the pool answers SELECT 1 happily while every write is refused.

   Composing the two: a node in that state returns 503 on every mutation and reports {"status":"ready"}. To be precise about what I checked, those two halves are each verified (the /ready path, and the fail-closed 503 proven by reverting the guard and watching the test go red) but I did not stand up that exact combined state and probe both endpoints, so the conjunction is deduction rather than an observation.

   The consequence for this issue: if the peer ping moves to /ready as proposed, the mesh will drain a node whose database is gone, but not one whose ledger is sick. The fix would inherit the gap instead of closing it.

   Why deepening /ready looks safe

   The obvious objection is blast radius. If the production nodes shared one Postgres, a fault in consumed_signatures would fail every node's readiness at once and pull the whole fleet, turning "writes 503, reads fine" into a total outage including reads. That would be worse than the gap.

   Reasoning rather than measurement, since I could not check the deployment: they do not share one. The sync worker mirrors repos from peers over HTTP, polling sync_queue, resolving the origin's URL from the peers table, then git clone --mirror or git fetch --prune, and registering itself as a replica afterwards. That protocol is incoherent against shared state: the nodes would already see each other's rows, would race the same queue items, and worst of all the rows describe repos on a per-node volume, so shared metadata over separate disks means serving metadata for bytes a node does not have. Each node also loads its own identity keypair and announces its own URL.

   So the topology the code is written for is one database per node, which makes a ledger fault node-local and makes draining exactly that node the correct behavior. I have not confirmed this against the live deployment (DATABASE_URL is a Fly secret), so treat it as the design's intent rather than a measured fact. A mismatch would not be silent, though: it would show up as repos 404ing that the database claims exist.

   Suggested shape

   When this issue is picked up, have /ready probe the ledger table alongside the pool rather than only SELECT 1, so "ready" means "can serve writes" and the peer ping inherits that. Keeping /health as pure liveness, per the direction above, still holds.
3. kevincodex1

   closed this as completedin #248on Aug 10, 2026