docs(mq): external NATS durability rests on replicas, not fsync

[EricAndrechek]

Part of #613.

Stacking: the base is feat/coord-nats-kv (#646). This PR's own change is the four commits after 5ba2bc13.

Why

The embedded broker is a single JetStream node, so it fsyncs every event before the 200 (SyncAlways). Under mq.backend: nats, a publish to a stream with 3 or more replicas is acked only once a Raft quorum has stored it. That quorum ack is the durability WaveHouse relies on, so it must not require sync_always on the operator's servers. This PR states that contract and has the topology check report the case where it does not hold.

Measured before the change (at 5ba2bc13)

Nothing in nats mode requires or configures fsync:

• deployments/nats/values.yaml has no sync, sync_interval or sync_always key anywhere. A new unit test now pins this.
• The topology verifier (nats_topology.go) had no sync check. num_replicas < 3 was a recommended finding on the partitions and the lease bucket, but not on the history or dead-letter streams.
• The manifest generator (nats_manifests.go, wavehouse mq manifests) already defaulted to 3 replicas, and it sets no sync or persist option.
• Publish path: ExternalNATS.publish calls js.PublishMsg synchronously with WithExpectStream, so it returns after the stream's PubAck. For R>1 that is the quorum ack.
• nats-server v2.14.6: defaultSyncInterval = 2 * time.Minute (server/filestore.go:333). A stream store and its Raft log inherit the server's SyncAlways/SyncInterval. A stream with persist_mode: async, which is allowed only at R1, forces SyncAlways=false and asynchronous flushing, even when the server sets sync_always (stream.go:995-1003, 1844-1850).
• The only SyncAlways in WaveHouse is mq.EmbeddedSyncAlways (embedded only).

What

1. Verifier (internal/mq/nats_topology.go):
   • num_replicas < 3 is now a recommended finding on the ingest partitions, the history and the DLQ, through one helper. At R1 the text says an ack then rests on one server's disk, and a crash loses what it stored since its last sync (sync_interval). At R2 it recommends 3 across failure domains. Because it is only recommended, a one-server dev cluster still boots.
   • persist_mode: async is a required finding on an ingest partition (the ack precedes the write) and a recommended one on the DLQ. It came up in review: sync_always does not reach such a stream.
2. Manifest generator: the default replica count stays 3. A quorum of 3 survives losing one server. That is the durability the docs now promise, and it is what the verifier recommends. A dev cluster passes --replicas 1 and gets the recommended findings.
3. Maintainer scope item: the generated history stream's maxAge changes from 2h to 15m, matching the settings seed's stream.gap_window_minutes of 15. The generated header and deployment.md say to set it to at least the longest stream.gap_window_minutes among the tenants served. deployments/nats/jetstream.yaml is regenerated.
   • The existing short-history warning is kept. It is fixed so it no longer fires for a window equal to max_age. It compared the sweeper's cutoff with a later time.Now(), so at the new default it would have warned for every tenant on the seed's window. It now allows one second of slack.
4. Docs:
   • deployment.md: a new External NATS → Durability subsection covering (a) embedded fsyncs every event before the 200; (b) nats acks after the stream's quorum stores it, with no fsync required, so durability comes from replicas spread across failure domains; (c) at R1 the server's sync_interval governs, and events since the last sync can be lost on a crash. The Persistent Storage fsync paragraph (~line 178) is now scoped to the embedded broker.
   • durability.md: the intro, the contract table and the "With an external NATS cluster" section are updated to match.
   • configuration.mdx: the Message Queue section no longer says embedded is the only backend.
   • CHANGELOG entries.

Tests

• TestReplicasProblem (unit): R0 and R1 name sync_interval, R2 recommends 3 without it, and R3 and R5 give no finding.
• TestVerifyNATSTopology_Findings: new cases for one replica on a partition, the history and the DLQ, and for persist_mode: async on a partition (required) and the DLQ (recommended). The shipped-manifest finding counts are updated (N+2, and N+3 with the lease bucket).
• TestShippedValues_SetNoSync (unit): walks values.yaml for any key containing sync. The fixture's ServerConfig renders only config.merge, so a server-side assertion alone could not catch a config.jetstream override.
• TestExternalNATS_PublishesWithoutSyncAlways (integration): against a server with SyncAlways off, a publish to an R1 partition is acked and stored, and the verifier reports only recommended findings, including that partition's num_replicas.
• TestExternalNATS_PurgeAckedWarnsOnAShortHistory gains an equal-window tenant that must not warn. Measured: it fails against the previous external.go and passes with the fix over -count=3.

Evidence

• Local make ci (shared queue, GOTOOLCHAIN=go1.26.6) at d563fa9a: exit 0, "All CI checks passed".
  • Go unit: 2479 tests. Integration: 72 tests, 88s. internal/mq integration: 65 tests, 3 skipped.
  • Coverage: unit 93.3%, integration 57.3%, e2e 60.7%, Go total 94.2%, ts-total 82.26%.
• make build-docs: all internal links valid.

Review

The reviewer markers are keyed to the main checkout (#454), so the verdicts are recorded here. No marker was hand-written.

• pre-push-reviewer (opus), four rounds:
  • Round 1: iterate, with 2 [SHOULD]:
    • The SyncAlways assertion could not see the values file. Fixed with TestShippedValues_SetNoSync.
    • persist_mode: async was not covered. Fixed with the verifier findings and docs.
  • Round 2: ship_it.
  • Round 3 (15m default): iterate, with 1 [MUST]. An equal window falsely warned. Fixed and pinned.
  • Round 4: ship_it.
• docs-reviewer (opus), five rounds:
  • Round 1: iterate, with 1 [MAY]. "No event is dropped" read as contradicting the crash-loss bullet; it is now scoped to limits. The same round flagged the stale configuration.mdx line, which is now fixed.
  • Rounds 2 and 3: ship_it.
  • Round 4: 1 [MAY], a CHANGELOG phrase that said the reverse of what happened. Fixed.
  • Round 5: ship_it.

Left to later PRs / not changed

• The lease bucket's own replica finding keeps its wording, because it holds no events.
• No mq.sync_interval knob for the embedded broker (mq: expose JetStream sync_interval as a config knob (throughput vs durability tradeoff) #139 still tracks it).
• The other stacked work (partition claims) is untouched. This PR's code is limited to the verifier, the manifest default, the one PurgeAcked comparison and tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EJr5tY4WQUy2sc4MbW67vL

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 2873b8e8-7a98-4038-be51-ca1af17c6613

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}