From 44ec87871e313a332080087669b164c5bbb50707 Mon Sep 17 00:00:00 2001 From: jx12n Date: Tue, 16 Jun 2026 23:45:46 -0600 Subject: [PATCH] =?UTF-8?q?fix(m12p6):=207th-edge=20=E2=80=94=20correct=20?= =?UTF-8?q?reseed=20seqno=20+=20skip=20suspect=20HNSW=20graph=20on=20resee?= =?UTF-8?q?d-pending=20close=20(rc11)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the rc9 over-correction: forcing baseline for ALL nodes (including caught-up ones) caused needless reseed cascades. Now only divergent nodes (frontier > baseline) use baseline as the reseed seqno; at/below-baseline nodes use frontier+1 so the leader picks cheap catch-up vs snapshot. Also skips the HNSW graph checkpoint on SIGTERM when the shard is reseed- pending: the in-memory index reflects suspect/divergent data the next boot discards, so saving it risks a "Failed to read vectors" failure on the post-reseed open. Durable checkpoints and WAL flush still run. close_shared() gains a save_graphs bool; shutdown_inner_impl() is the shared implementation; node.rs passes !reseed_pending. --- k8s/cluster/statefulset.yaml | 2 +- tidal-server/src/cluster/election_driver.rs | 25 +++++++++----- tidal-server/src/cluster/node.rs | 35 +++++++++++++------ tidal/src/db/lifecycle.rs | 38 ++++++++++++++++----- 4 files changed, 72 insertions(+), 28 deletions(-) diff --git a/k8s/cluster/statefulset.yaml b/k8s/cluster/statefulset.yaml index 70ac61d..87451e9 100644 --- a/k8s/cluster/statefulset.yaml +++ b/k8s/cluster/statefulset.yaml @@ -84,7 +84,7 @@ spec: mountPath: /data containers: - name: tidaldb - image: registry.threesix.ai/tidal/server@sha256:818c923368ecca6e433c42a13672132e9b467c449fa23c035201fe7189f1f52b # m12-rc9 (= rc8 + Bug 6: reseed latch re-baselines from the stream baseline, not frontier+1, so a divergent-rejoin node force-installs the snapshot instead of looping) + image: registry.threesix.ai/tidal/server@sha256:2e1f7a0e2603fc135f6e79a8c376ca265f8ad74724904b1922df310e8e6da306 # m12-rc11 (= rc10 + corrected Bug 6: only force the re-baselining snapshot when frontier > baseline (divergent); at/below baseline keep frontier+1 so a caught-up node does NOT needlessly reseed-cascade) imagePullPolicy: IfNotPresent # The image ENTRYPOINT is the bare binary. We override the command with # a tiny /bin/sh wrapper (the bookworm-slim runtime HAS a shell) so we diff --git a/tidal-server/src/cluster/election_driver.rs b/tidal-server/src/cluster/election_driver.rs index 01fbeab..e3f784d 100644 --- a/tidal-server/src/cluster/election_driver.rs +++ b/tidal-server/src/cluster/election_driver.rs @@ -293,14 +293,23 @@ impl ElectionRuntime { // node is missing in the PREVIOUS stream's numbering // (`own.frontier + 1`); the marker boot re-baselines onto the // leader's stream via the snapshot regardless. - // m12p6: re-baseline from the new term's STREAM BASELINE, not - // `frontier + 1`. When the node's frontier sits at/above the - // baseline (post-baseline old-term data), `frontier + 1` lands - // above the leader's tail → `wal_covers` answers needed=false → the - // reseed install no-ops → re-detect → self-restart loop. A - // `from_seqno <= baseline` forces the snapshot that re-baselines the - // node onto the leader's committed history. - node.latch_reseed_marker(ReseedReason::SnapshotRequired, baseline); + // m12p6: pick the resume seqno so the leader's `wal_covers` makes + // the RIGHT needed-decision. When the node's frontier sits ABOVE the + // baseline it has post-baseline old-term (DIVERGENT) data: `frontier + // + 1` would land above the leader's tail → `wal_covers` answers + // needed=false → the install no-ops → re-detect → self-restart loop. + // Re-baseline from `baseline` (`<= baseline` forces the snapshot that + // discards the divergent suffix). When the frontier is AT/BELOW the + // baseline the node is genuinely BEHIND (or exactly caught up): + // `frontier + 1` is correct and lets the leader's needed-decision + // pick a cheap catch-up vs snapshot — forcing `baseline` here would + // reseed an already-caught-up node and cascade restarts. + let from_seqno = if position.frontier > baseline { + baseline + } else { + position.frontier.saturating_add(1) + }; + node.latch_reseed_marker(ReseedReason::SnapshotRequired, from_seqno); self.joined_term.store(term, Ordering::Release); true } diff --git a/tidal-server/src/cluster/node.rs b/tidal-server/src/cluster/node.rs index 4edfab5..74978e1 100644 --- a/tidal-server/src/cluster/node.rs +++ b/tidal-server/src/cluster/node.rs @@ -1113,7 +1113,15 @@ impl ShardReplica { // because no `{data_dir}/vector` file was written on SIGTERM. `close_shared` // takes `&self` and is idempotent (the `closed` CAS), so the trailing // `drop(db)` and any later `Drop` from a lingering clone are no-ops. - if let Err(e) = db.close_shared() { + // m12p6 7th-edge: when this shard is RESEED-PENDING (a divergent / + // quarantined node about to be re-baselined — its marker latched), skip + // the HNSW-graph save. The in-memory index reflects suspect data the + // next boot DISCARDS via the snapshot install, so saving it is + // pointless and risks committing a graph the next open fails to read + // ("Failed to read vectors" → a slow rebuild). The durable checkpoints + // + WAL flush still run. + let reseed_pending = matches!(self.reseed_marker_store.load(), Ok(Some(_))); + if let Err(e) = db.close_shared(!reseed_pending) { tracing::error!( region = %self.region_name, error = %e, @@ -1124,7 +1132,8 @@ impl ShardReplica { drop(db); tracing::info!( region = %self.region_name, - "region cluster node shutdown: database closed (checkpoint + WAL fsync + HNSW graph)" + reseed_pending, + "region cluster node shutdown: database closed (checkpoint + WAL fsync + HNSW graph unless reseed-pending)" ); } } @@ -2698,15 +2707,19 @@ impl ShardReplica { baseline: u64, ) { self.cluster_metrics.set_divergence_quarantined(true); - // m12p6: the reseed must re-baseline from the new term's STREAM BASELINE, - // NOT `frontier + 1`. A divergent node's frontier sits AT/ABOVE the - // baseline, so `frontier + 1` lands above the leader's WAL tail → the - // leader answers `needed=false` (nothing to ship above its tail) → the - // reseed install no-ops, the divergent suffix is re-detected, and the node - // self-restart-loops. A `from_seqno <= baseline` makes `wal_covers` return - // needed=true → the snapshot installs and re-baselines onto the leader, - // discarding the divergent suffix. - self.latch_reseed_marker(ReseedReason::Quarantine, baseline); + // m12p6: re-baseline a DIVERGENT node (frontier ABOVE the new term's + // baseline) from `baseline` — `frontier + 1` would land above the leader's + // WAL tail → `wal_covers` answers needed=false → the install no-ops and the + // divergent suffix self-restart-loops. A `from_seqno <= baseline` forces the + // snapshot that discards the suffix. (When the frontier is at/below the + // baseline — not the divergent case — `frontier + 1` lets the leader pick a + // cheap catch-up vs snapshot, avoiding a needless reseed cascade.) + let from_seqno = if frontier > baseline { + baseline + } else { + frontier.saturating_add(1) + }; + self.latch_reseed_marker(ReseedReason::Quarantine, from_seqno); tracing::error!( term, tail_term, diff --git a/tidal/src/db/lifecycle.rs b/tidal/src/db/lifecycle.rs index f1c3793..daade06 100644 --- a/tidal/src/db/lifecycle.rs +++ b/tidal/src/db/lifecycle.rs @@ -56,16 +56,31 @@ impl TidalDb { /// Callers MUST have quiesced the write/apply path first (the cluster node /// stops the ship queue and signals the segment receiver before calling this). /// + /// `save_graphs` (m12p6 7th-edge): when `false`, SKIP the HNSW-graph + /// checkpoint. The cluster node passes `false` when the shard is RESEED-PENDING + /// (a divergent/quarantined node about to be re-baselined): its in-memory + /// index reflects suspect/divergent data the next boot will DISCARD via the + /// snapshot install, so persisting it is pointless AND risks committing a + /// graph the next open then fails to read ("Failed to read vectors") — forcing + /// a slow rebuild. Everything else in the close (durable checkpoints, WAL + /// flush/marker) still runs. + /// /// # Errors /// /// Surfaces the first durable-flush failure, exactly like [`close`](Self::close). - pub fn close_shared(&self) -> crate::Result<()> { - self.shutdown_inner() + pub fn close_shared(&self, save_graphs: bool) -> crate::Result<()> { + self.shutdown_inner_impl(save_graphs) } - /// Internal shutdown logic shared by `close()` and `Drop`. - #[allow(clippy::too_many_lines)] + /// Internal shutdown logic shared by `close()` and `Drop` — always saves the + /// HNSW graph (the clean-shutdown path; only a reseed-pending close skips it, + /// via [`close_shared`](Self::close_shared)). pub(crate) fn shutdown_inner(&self) -> crate::Result<()> { + self.shutdown_inner_impl(true) + } + + #[allow(clippy::too_many_lines)] + fn shutdown_inner_impl(&self, save_graphs: bool) -> crate::Result<()> { // CAS: first caller to flip false -> true executes the shutdown body. if self .closed @@ -228,10 +243,17 @@ impl TidalDb { // logged but never fails close()/Drop — a missing graph simply rebuilds // on the next open, which is correct, just slow. The checkpoint thread // is already joined above, so the registry is quiescent here. - crate::db::state_rebuild::checkpoint_embedding_graphs( - &self.embedding_registry, - crate::db::state_rebuild::vector_graph_dir(&self.config).as_deref(), - ); + if save_graphs { + crate::db::state_rebuild::checkpoint_embedding_graphs( + &self.embedding_registry, + crate::db::state_rebuild::vector_graph_dir(&self.config).as_deref(), + ); + } else { + tracing::info!( + "skipping HNSW graph checkpoint on close: reseed-pending shard, \ + its in-memory index is suspect/divergent and will be re-baselined" + ); + } if let Err(e) = storage.flush() { tracing::error!(error = %e, "storage flush failed during shutdown"); first_err.get_or_insert(e);