Skip to content

feat(api): add spec.cluster.privateBackend so flushed parts actually replicate - #17

Draft
tdakkota wants to merge 5 commits into
claude/role-query-groupfrom
claude/16-private-backend
Draft

feat(api): add spec.cluster.privateBackend so flushed parts actually replicate#17
tdakkota wants to merge 5 commits into
claude/role-query-groupfrom
claude/16-private-backend

Conversation

@tdakkota

Copy link
Copy Markdown
Contributor

Closes #16.

Adds spec.cluster.privateBackend, rendered as storage.cluster.private_backend in the generated
oteldb config.

Why this is a durability fix, not a convenience field

storage.cluster.private_backend gates cluster/partsync. The ring replicates only the unflushed
head
; partsync is what replicates flushed parts and what backfills a node that lost its disk.
The operator never rendered the key, and it is unreachable via spec.extraConfig because
storage.cluster is in reservedConfigPaths and a reserved path covers everything beneath it.

Measured on kind (3 nodes, backend: file, replicationFactor: 2, flushInterval: 10s) in #16:

  • Without the flag, every flushed part existed on exactly one node despite RF=2 — 35 files on
    one node, 13 on another, none on the third. A single lost disk was permanent data loss.
  • With it, a wiped node backfills from peers in ~20s via bootstrapGainedTenants → partsync →
    engine load, and parts land on 2 nodes as RF=2 implies.
  • Without it, a wiped node never recovers, and not just slowly: bootstrapShard calls
    syncParts, which no-ops for a supposedly shared backend, so no bucket index appears locally, no
    engine is created, hasAnyEngine stays false, and the next maintenance tick rediscovers the same
    shard. The loop has no exit.

So every clustered OtelDBCluster on the file backend the operator has deployed has been running
without part-level redundancy.

The design decision: derived, not a plain bool

spec.cluster.privateBackend is a *bool. Unset derives from spec.storage.backendtrue
for file, false for s3. An explicit value overrides the derivation.

The reasoning, since this matters more than the code:

  • A plain bool defaulting false makes the safe-looking deployment the non-durable one. The
    operator's default topology is backend: file on per-pod ReadWriteOnce PVCs; there is no
    configuration in which the operator hands several pods one shared filesystem. So file is
    always the private case here, and a false-by-default bool would leave every existing user
    silently broken until they read the field docs.
  • Conversely the operator's s3 backend is one bucket every pod addresses, i.e. always shared, so
    deriving false there is equally exact.
  • The operator knows which backend it configured. oteldb keeps the flag explicit because in
    general
    it is not inferable from the backend type (a file backend on NFS is shared; per-node S3
    buckets exist) — but that generality is about arbitrary oteldb configs, not about the two
    topologies this operator can produce. Deriving is right at this layer and explicit is right at
    oteldb's layer.
  • The escape hatch is kept for the topologies the derivation cannot see: privateBackend: false for
    a file backend on a ReadWriteMany volume, privateBackend: true for per-node S3 buckets. Both
    are covered by tests.

storage.cluster stays reserved from extraConfig — the CRD field is the supported route.

The key is rendered unconditionally (including private_backend: false) rather than omitted when
false, so the ConfigMap states the mode explicitly and the config hash changes on upgrade.

⚠️ Behaviour change for existing clusters on upgrade

This changes the behaviour of already-deployed clusters, deliberately. Any existing
OtelDBCluster on the default file backend that does not set privateBackend will, after the
operator is upgraded, render private_backend: true, get a new config hash, and roll. On the new
pods partsync starts running and flushed parts begin replicating to their RF peers.

That is the correct behaviour and it is strictly additive — it copies data that should already have
been copied
and removes nothing — but it is not a no-op:

  • Expect a one-time backfill of the existing part backlog across the peer network as each shard's
    replicas catch up, and a corresponding rise in disk usage on nodes that were previously missing
    their share (up to roughly RF× the current per-node footprint in the steady state, which is what
    the declared replicationFactor always implied).
  • Anyone who genuinely runs file on a shared ReadWriteMany volume must set
    privateBackend: false before upgrading, or they get pointless self-copying.

Interaction with the oteldb/storage startup diagnostic

oteldb/storage#373 added a startup diagnostic that detects exactly this misconfiguration. It fires
when o.Cluster != nil && !o.Cluster.PrivateBackend && backend.IsNodeLocal(o.Backend) and logs at
Warn:

cluster backend looks node-private but PrivateBackend is false"flushed parts are not
replicated between nodes; after a rebalance handoff or a replica restart this node answers reads
without them, and the caller cannot tell that from real absence"

and surfaces the standing form as ClusterStats.NodeLocalBackendUnshared in Inspect.

The two agree. storage's IsNodeLocal() is true for the file backend and false for S3,
which is precisely the derivation implemented here — so the deployments this PR flips to true are
exactly the ones the diagnostic flags today, and after this PR the warning goes quiet on the
operator's default topology instead of firing on every node. The one asymmetry is harmless: the
diagnostic does not check the inverse (a shared store declared private), so an explicit
privateBackend: true on s3 is not second-guessed by storage either.

Version caveat. Neither side is in a release the operator's pinned image can reach yet:
defaultImage is ghcr.io/oteldb/oteldb:v0.46.0, which vendors storage v0.28.0; oteldb/oteldb#1264
(the config field) is merged to oteldb main but untagged, and storage#373 is on storage main with
no tag containing it. oteldb's config loader is a non-strict yaml.Unmarshal, so the rendered key is
silently ignored by older images rather than fatal — the field is inert until spec.image (or a
future defaultImage bump) points at a build carrying #1264. The README row says so. That also means
the behaviour change above lands when the image moves, not when the operator does, for anyone still
on the default pin.

Tests

  • TestRenderConfigPrivateBackend — table over file/s3/unset backends and both explicit overrides.
  • TestRenderConfigPrivateBackendNotReachableViaExtraConfig — regression pinning that
    storage.cluster stays reserved.
  • TestRenderConfigFileBackendDefaults now asserts private_backend: true.

make lint, make test, go build ./... all clean.

Based on main; #14 and #15 also regenerate CRD manifests, so whichever merges second will need a
mechanical rebase of the generated YAML.

🤖 Generated with Claude Code

portSelfMetric was 8090, which is oteldb's admin API bind (admin.bind defaults to :8090 and the
operator never overrides it), so the Service port named metrics routed to the admin API and UI.

Fixes #12
fix(controller): move self-metrics port off oteldb's admin API bind
Without it cluster/partsync never runs, so only the unflushed head is replicated and every
flushed part exists in exactly one copy regardless of spec.cluster.replicationFactor. A node
that loses its PVC never recovers: bootstrapShard's syncParts no-ops for a supposedly shared
backend, so no engine is ever created.

spec.cluster.privateBackend is a *bool: unset derives from spec.storage.backend, since file is
one PVC per pod (private) and s3 is a bucket every node shares. storage.cluster stays reserved
from extraConfig, so the CRD field is the only route.
…aude/16-private-backend

# Conflicts:
#	README.md
#	internal/controller/config.go
#	internal/controller/naming.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec has no way to set cluster private_backend, so partsync never runs on the file backend

1 participant