fix(m12p6): 7th-edge — correct reseed seqno + skip suspect HNSW graph on reseed-pending close (rc11)
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.
This commit is contained in:
parent
727fbfcb6b
commit
44ec87871e
@ -84,7 +84,7 @@ spec:
|
|||||||
mountPath: /data
|
mountPath: /data
|
||||||
containers:
|
containers:
|
||||||
- name: tidaldb
|
- 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
|
imagePullPolicy: IfNotPresent
|
||||||
# The image ENTRYPOINT is the bare binary. We override the command with
|
# 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
|
# a tiny /bin/sh wrapper (the bookworm-slim runtime HAS a shell) so we
|
||||||
|
|||||||
@ -293,14 +293,23 @@ impl ElectionRuntime {
|
|||||||
// node is missing in the PREVIOUS stream's numbering
|
// node is missing in the PREVIOUS stream's numbering
|
||||||
// (`own.frontier + 1`); the marker boot re-baselines onto the
|
// (`own.frontier + 1`); the marker boot re-baselines onto the
|
||||||
// leader's stream via the snapshot regardless.
|
// leader's stream via the snapshot regardless.
|
||||||
// m12p6: re-baseline from the new term's STREAM BASELINE, not
|
// m12p6: pick the resume seqno so the leader's `wal_covers` makes
|
||||||
// `frontier + 1`. When the node's frontier sits at/above the
|
// the RIGHT needed-decision. When the node's frontier sits ABOVE the
|
||||||
// baseline (post-baseline old-term data), `frontier + 1` lands
|
// baseline it has post-baseline old-term (DIVERGENT) data: `frontier
|
||||||
// above the leader's tail → `wal_covers` answers needed=false → the
|
// + 1` would land above the leader's tail → `wal_covers` answers
|
||||||
// reseed install no-ops → re-detect → self-restart loop. A
|
// needed=false → the install no-ops → re-detect → self-restart loop.
|
||||||
// `from_seqno <= baseline` forces the snapshot that re-baselines the
|
// Re-baseline from `baseline` (`<= baseline` forces the snapshot that
|
||||||
// node onto the leader's committed history.
|
// discards the divergent suffix). When the frontier is AT/BELOW the
|
||||||
node.latch_reseed_marker(ReseedReason::SnapshotRequired, baseline);
|
// 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);
|
self.joined_term.store(term, Ordering::Release);
|
||||||
true
|
true
|
||||||
}
|
}
|
||||||
|
|||||||
@ -1113,7 +1113,15 @@ impl ShardReplica {
|
|||||||
// because no `{data_dir}/vector` file was written on SIGTERM. `close_shared`
|
// because no `{data_dir}/vector` file was written on SIGTERM. `close_shared`
|
||||||
// takes `&self` and is idempotent (the `closed` CAS), so the trailing
|
// takes `&self` and is idempotent (the `closed` CAS), so the trailing
|
||||||
// `drop(db)` and any later `Drop` from a lingering clone are no-ops.
|
// `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!(
|
tracing::error!(
|
||||||
region = %self.region_name,
|
region = %self.region_name,
|
||||||
error = %e,
|
error = %e,
|
||||||
@ -1124,7 +1132,8 @@ impl ShardReplica {
|
|||||||
drop(db);
|
drop(db);
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
region = %self.region_name,
|
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,
|
baseline: u64,
|
||||||
) {
|
) {
|
||||||
self.cluster_metrics.set_divergence_quarantined(true);
|
self.cluster_metrics.set_divergence_quarantined(true);
|
||||||
// m12p6: the reseed must re-baseline from the new term's STREAM BASELINE,
|
// m12p6: re-baseline a DIVERGENT node (frontier ABOVE the new term's
|
||||||
// NOT `frontier + 1`. A divergent node's frontier sits AT/ABOVE the
|
// baseline) from `baseline` — `frontier + 1` would land above the leader's
|
||||||
// baseline, so `frontier + 1` lands above the leader's WAL tail → the
|
// WAL tail → `wal_covers` answers needed=false → the install no-ops and the
|
||||||
// leader answers `needed=false` (nothing to ship above its tail) → the
|
// divergent suffix self-restart-loops. A `from_seqno <= baseline` forces the
|
||||||
// reseed install no-ops, the divergent suffix is re-detected, and the node
|
// snapshot that discards the suffix. (When the frontier is at/below the
|
||||||
// self-restart-loops. A `from_seqno <= baseline` makes `wal_covers` return
|
// baseline — not the divergent case — `frontier + 1` lets the leader pick a
|
||||||
// needed=true → the snapshot installs and re-baselines onto the leader,
|
// cheap catch-up vs snapshot, avoiding a needless reseed cascade.)
|
||||||
// discarding the divergent suffix.
|
let from_seqno = if frontier > baseline {
|
||||||
self.latch_reseed_marker(ReseedReason::Quarantine, baseline);
|
baseline
|
||||||
|
} else {
|
||||||
|
frontier.saturating_add(1)
|
||||||
|
};
|
||||||
|
self.latch_reseed_marker(ReseedReason::Quarantine, from_seqno);
|
||||||
tracing::error!(
|
tracing::error!(
|
||||||
term,
|
term,
|
||||||
tail_term,
|
tail_term,
|
||||||
|
|||||||
@ -56,16 +56,31 @@ impl TidalDb {
|
|||||||
/// Callers MUST have quiesced the write/apply path first (the cluster node
|
/// Callers MUST have quiesced the write/apply path first (the cluster node
|
||||||
/// stops the ship queue and signals the segment receiver before calling this).
|
/// 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
|
/// # Errors
|
||||||
///
|
///
|
||||||
/// Surfaces the first durable-flush failure, exactly like [`close`](Self::close).
|
/// Surfaces the first durable-flush failure, exactly like [`close`](Self::close).
|
||||||
pub fn close_shared(&self) -> crate::Result<()> {
|
pub fn close_shared(&self, save_graphs: bool) -> crate::Result<()> {
|
||||||
self.shutdown_inner()
|
self.shutdown_inner_impl(save_graphs)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Internal shutdown logic shared by `close()` and `Drop`.
|
/// Internal shutdown logic shared by `close()` and `Drop` — always saves the
|
||||||
#[allow(clippy::too_many_lines)]
|
/// 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<()> {
|
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.
|
// CAS: first caller to flip false -> true executes the shutdown body.
|
||||||
if self
|
if self
|
||||||
.closed
|
.closed
|
||||||
@ -228,10 +243,17 @@ impl TidalDb {
|
|||||||
// logged but never fails close()/Drop — a missing graph simply rebuilds
|
// logged but never fails close()/Drop — a missing graph simply rebuilds
|
||||||
// on the next open, which is correct, just slow. The checkpoint thread
|
// on the next open, which is correct, just slow. The checkpoint thread
|
||||||
// is already joined above, so the registry is quiescent here.
|
// is already joined above, so the registry is quiescent here.
|
||||||
|
if save_graphs {
|
||||||
crate::db::state_rebuild::checkpoint_embedding_graphs(
|
crate::db::state_rebuild::checkpoint_embedding_graphs(
|
||||||
&self.embedding_registry,
|
&self.embedding_registry,
|
||||||
crate::db::state_rebuild::vector_graph_dir(&self.config).as_deref(),
|
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() {
|
if let Err(e) = storage.flush() {
|
||||||
tracing::error!(error = %e, "storage flush failed during shutdown");
|
tracing::error!(error = %e, "storage flush failed during shutdown");
|
||||||
first_err.get_or_insert(e);
|
first_err.get_or_insert(e);
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user