Skip to content

Commit ac845fc

Browse files
authored
Merge pull request #2 from base-14/nitin/replication-health-metrics
fix: parse SHOW REPLICAS data_info correctly; add replication health + drift metrics
2 parents 83964ef + 7d549ed commit ac845fc

9 files changed

Lines changed: 897 additions & 87 deletions

File tree

‎FIXES-REQUIRED.md‎

Lines changed: 183 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,17 @@
44

55
During debugging of KB pod crashes (2025-12-05), we identified several issues in the memgraph-operator that cause high CPU usage on Memgraph MAIN instances and incorrect health reporting.
66

7+
**Update (2026-06-04):** While debugging intermittent empty query results in a production cluster, we traced a silent, 104-day replication outage to three additional operator defects (Issues 5–7 below). The affected replica was registered and heartbeating but had **zero data** (`data_info: {}`), and the `-read` service kept serving it to clients. This incident also showed that Issue 1's premise is partially wrong — see the correction note under Issue 1.
8+
79
## Issue Summary
810

911
1. **SHOW REPLICAS output format changed in Memgraph v2.21.0** - Parser is incompatible
1012
2. **Status updates trigger immediate re-reconciliation** - Backoff is ineffective
1113
3. **Multiple queries per reconciliation** - Overwhelms Memgraph under frequent reconciliation
14+
4. **Validation test writes to database** - Invasive health checks
15+
5. **`clear-replication-state` init container deletes `epoch_id`** - Breaks replica lineage on every restart (root cause of the production outage)
16+
6. **No remediation for invalid/diverged replicas** - Operator logs warnings forever, never re-seeds
17+
7. **`-read` service selects ALL pods regardless of replication health** - Broken replicas serve client traffic
1218

1319
---
1420

@@ -46,6 +52,18 @@ Operator log:
4652
- Triggers frequent reconciliation attempts
4753
- Emits spurious warning events
4854

55+
### ⚠️ Correction (2026-06-04, production incident)
56+
57+
The premise "replicas are incorrectly marked unhealthy even when replication is working" is **not safe to assume**. On a healthy replica, Memgraph (verified on v3.7.2) reports:
58+
59+
```
60+
data_info: {memgraph: {behind: 0, status: "ready", ts: <non-zero>}}
61+
```
62+
63+
An **empty** `data_info` (`{}`) is NOT a healthy replica misparsed — it means the MAIN has no confirmed replication state for that replica, i.e. data streaming never engaged. In the production incident the replica showed `{}` for 104+ days and genuinely contained zero data while the main had 3151 `DEPENDS_ON` relationships. The original "evidence" above (oteldemo1 showing `{}`) was very likely a real desync too, misdiagnosed as a parser artifact.
64+
65+
**Consequence for the fix:** when implementing the parser, classify `data_info: {}` (or missing per-database entries) as **unhealthy/invalid**, not healthy. The parser bug and real desyncs are independent failure modes that currently produce the same log line; after the fix they must be distinguishable.
66+
4967
### Required Changes
5068

5169
**File:** `internal/memgraph/client.go`
@@ -248,14 +266,156 @@ The `testReplicationLag` function in `internal/controller/validation.go:105-172`
248266
- Skip validation if previous N attempts failed
249267
- Reduce frequency when cluster is under load
250268
269+
### Note (2026-06-04)
270+
271+
`testReplicationLag` (write to MAIN → read from each REPLICA) is exactly the check that **would have caught the 104-day production divergence** — the test node would never have appeared on the replica. Before making validation read-only, investigate why it didn't surface this incident (not running? failing silently? result not acted upon?). Whatever replaces it must still detect a replica that silently stops receiving data. A read-only equivalent: compare `SHOW STORAGE INFO` vertex/edge counts between MAIN and replicas.
272+
273+
---
274+
275+
## Issue 5: `clear-replication-state` Init Container Deletes `epoch_id` (HIGH PRIORITY — root cause of the production outage)
276+
277+
### Problem
278+
279+
The `clear-replication-state` init container in `internal/controller/statefulset.go:222-250` runs on **every pod restart** and executes:
280+
281+
```sh
282+
rm -rf $DATA/.internal/replication
283+
rm -f $DATA/.internal/replication_*
284+
rm -f $DATA/.internal/epoch_id # ← the harmful line
285+
```
286+
287+
The comment claims *"Graph data is preserved - only replication state is cleared."* That combination is precisely the problem: in Memgraph, `epoch_id` is the proof that two instances share commit history. Deleting it while preserving graph data produces an instance that **has data under an unprovable lineage**. When such a pod becomes a REPLICA, the MAIN sees a diverged instance (the documented "replica at one point acted as main / branched history" condition): it accepts registration, keeps a heartbeat TCP session alive, but **never streams data**. `SHOW REPLICAS` shows `data_info: {}` indefinitely.
288+
289+
A fresh, empty replica (ts=0, no epoch) always recovers — MAIN sends a snapshot. A data-bearing replica with wiped epoch **never** recovers automatically.
290+
291+
### Evidence (production incident, 2026-06-04)
292+
293+
- The affected replica pod: role `REPLICA`, registered, MAIN holds an ESTABLISHED TCP session to its `:10000` — yet `MATCH ()-[r:DEPENDS_ON]->() RETURN count(...)` = **0** vs **3151** on MAIN.
294+
- `SHOW REPLICAS` on MAIN: `data_info: {}` (0 properties), `system_info: null`.
295+
- Operator logged `unhealthy replica detected ... status:"{}"` every reconcile for the pods' entire 104-day uptime.
296+
- User impact: a client proxy (`BACKEND_ADDR=<cluster>-graph-read:7687`) intermittently returned empty results to downstream consumers whenever a connection landed on the empty replica (see Issue 7).
297+
- Manual recovery attempts (`DROP REPLICA` + `REGISTER REPLICA`) fail or are futile: registration is not the broken layer, and the reconcile loop (Issue 2) re-registers within seconds anyway.
298+
299+
### Impact
300+
301+
- Any pod restart can silently break replication **permanently** for that replica
302+
- Failure mode is invisible to clients of "registered" status — only `data_info` reveals it
303+
- Combined with Issue 7, broken replicas serve empty results to production traffic
304+
305+
### Required Changes
306+
307+
**File:** `internal/controller/statefulset.go`
308+
309+
1. **Stop deleting `epoch_id`.** The original motivation (MAIN failing at startup trying to reconnect to since-removed replicas) is addressed by clearing the replication *registration* state only:
310+
```sh
311+
rm -rf $DATA/.internal/replication
312+
rm -f $DATA/.internal/replication_*
313+
# epoch_id is NOT deleted — it is required for replica lineage
314+
```
315+
2. Verify against the deployed Memgraph version (v3.7.2 in production — see Version Compatibility) which on-disk paths hold (a) replica registrations on MAIN, (b) role state, (c) epoch/lineage. Delete only (a), and only if startup-reconnect failures are still reproducible on that version.
316+
3. Divergence recovery (data wipe + reseed) belongs in the controller as explicit remediation (Issue 6), not in a blanket init container.
317+
318+
### Testing
319+
320+
- Restart a replica pod with data → it must rejoin and continue replicating (`data_info.status == "ready"`, `behind == 0`)
321+
- Restart the MAIN pod → it must come back as MAIN without startup failures and re-establish streaming to replicas
322+
- Restart both pods together (the incident scenario) → replication must re-engage without manual intervention
323+
324+
---
325+
326+
## Issue 6: No Remediation for Invalid/Diverged Replicas (HIGH PRIORITY)
327+
328+
### Problem
329+
330+
The operator can *detect* a broken replica but never *fixes* it:
331+
332+
- `ConfigureReplication` (`internal/controller/replication.go:96`) registers a replica only `if !currentReplicaNames[replicaName]` — an existing-but-dead registration is skipped forever.
333+
- `CheckReplicationHealth` (`internal/controller/replication.go:222-233`) classifies anything not `ready`/`replicating` as unhealthy, logs a warning, emits an event — and does nothing else.
334+
335+
Result: a replica in `invalid`/diverged/never-seeded state stays broken indefinitely while the operator logs the same warning every reconcile (in the incident: every ~1-30s for 104 days).
336+
337+
### Impact
338+
339+
- Replication outages require manual diagnosis and manual PVC surgery to resolve
340+
- Manual `DROP REPLICA`/`REGISTER REPLICA` races the reconcile loop (Issue 2), making operator-fighting the only manual path
341+
- "Self-healing" is the operator's core value proposition and is absent for the most common replication failure
342+
343+
### Required Changes
344+
345+
**File:** `internal/controller/replication.go` (+ controller plumbing)
346+
347+
1. Track consecutive unhealthy observations per replica (in status or in-memory with backoff).
348+
2. After N consecutive cycles (e.g., 5) of `data_info` empty/`invalid`:
349+
a. `DROP REPLICA <name>` on MAIN
350+
b. Trigger a clean re-seed of the replica pod: delete the pod **and** wipe its data (annotation consumed by an init container, or PVC delete + recreate — design decision)
351+
c. On pod Ready: set role, re-register → MAIN seeds a fresh replica via snapshot transfer
352+
3. Emit distinct events for each phase (`ReplicaReseedStarted`, `ReplicaReseedCompleted`, `ReplicaReseedFailed`) so the remediation is observable.
353+
4. Cap retries with exponential backoff; surface a terminal `ReplicationDegraded` condition on the CR if reseeding fails repeatedly.
354+
5. Depends on Issue 1: remediation must key off correctly parsed `data_info`, and `{}` must classify as unhealthy (see Issue 1 correction note).
355+
356+
### Testing
357+
358+
- Inject divergence (wipe `epoch_id` on a data-bearing replica, restart) → operator detects, reseeds, replica converges to MAIN's counts
359+
- Verify no remediation loop on transient `recovery` status (replica catching up must not be reaped)
360+
- Verify backoff: unreachable replica does not cause hot reseed loops
361+
362+
---
363+
364+
## Issue 7: `-read` Service Selects All Pods Regardless of Replication Health (HIGH PRIORITY)
365+
366+
### Problem
367+
368+
`buildReadService` (`internal/controller/service.go:109-115`) ships with an acknowledged placeholder:
369+
370+
```go
371+
// Initially selects all pods; we'll use an annotation or label to mark read pods
372+
// For now, this selects all pods - reads should work on any pod
373+
selector := labelsForCluster(cluster)
374+
```
375+
376+
"Reads should work on any pod" is only true when every replica is in sync. The selector includes the MAIN **and** every replica, healthy or not. There is no mechanism to evict a desynced replica from read rotation.
377+
378+
### Evidence (production incident, 2026-06-04)
379+
380+
A client proxy points at `<cluster>-graph-read:7687`. With one healthy MAIN and one empty replica behind that service, ~50% of new proxy backend connections returned empty query results to downstream consumers. Confirmed by running `SHOW REPLICATION ROLE` + a count query repeatedly through the proxy: role flipped `main`/`replica` in lockstep with count `3151`/`0`.
381+
382+
### Impact
383+
384+
- Replication failures become **user-facing data corruption** (silently empty/stale results) instead of reduced capacity
385+
- Intermittent symptom (connection-dependent) is expensive to diagnose; in the incident it was initially misattributed to the proxy
386+
- Port-forward verification is misleading here: `kubectl port-forward svc/...` pins to one pod, so the read service "looks fine" when tested that way
387+
388+
### Required Changes
389+
390+
**File:** `internal/controller/service.go`, `internal/controller/labels.go`, reconcile loop
391+
392+
1. Maintain a per-pod replication-health label from reconcile results (Issue 1's parsed `data_info`), e.g.:
393+
```
394+
memgraph.base14.io/role: main|replica
395+
memgraph.base14.io/replication-healthy: "true"|"false"
396+
```
397+
2. Change the `-read` selector to `replication-healthy=true` (replicas in `ready`/`replicating`, plus the MAIN if reads-from-main is desired for capacity).
398+
3. **Fallback rule:** if zero healthy replicas exist, the selector must fall back to the MAIN (mirroring `buildWriteService`'s pod-name selector) — never serve a known-broken replica, and never empty the service.
399+
4. Label updates must be prompt on health transitions (tie into Issue 6's remediation phases so a reseeding replica is out of rotation until converged).
400+
401+
### Testing
402+
403+
- Healthy cluster: `-read` endpoints = all healthy pods
404+
- Break one replica (Issue 6 injection): endpoints drop the broken replica within one reconcile; client queries never return empty
405+
- Zero healthy replicas: endpoints = MAIN only
406+
- Replica reseeds: re-enters endpoints only after `ready`/`behind: 0`
407+
251408
---
252409

253410
## Implementation Order
254411

255-
1. **Issue 1** (SHOW REPLICAS parser) - Must fix first, causes all other issues to be worse
256-
2. **Issue 2** (Status update triggering reconciliation) - Significant impact reduction
257-
3. **Issue 3** (Query reduction) - Optimization
258-
4. **Issue 4** (Validation) - Nice to have
412+
1. **Issue 1** (SHOW REPLICAS parser) - Must fix first: correct health *detection* is a prerequisite for Issues 6 and 7. Apply the correction note (`{}` = unhealthy).
413+
2. **Issue 5** (`epoch_id` wipe) - Stops *creating* new diverged replicas on every restart
414+
3. **Issue 6** (auto-remediation) - Fixes existing/future diverged replicas without manual PVC surgery
415+
4. **Issue 7** (read service health gating) - Stops broken replicas from serving client traffic (closes the user-facing symptom class)
416+
5. **Issue 2** (Status update triggering reconciliation) - Significant impact reduction; also removes the reconcile-race that blocks manual recovery
417+
6. **Issue 3** (Query reduction) - Optimization
418+
7. **Issue 4** (Validation) - Revisit only after understanding why it missed the production divergence
259419

260420
## Estimated Effort
261421

@@ -265,12 +425,30 @@ The `testReplicationLag` function in `internal/controller/validation.go:105-172`
265425
| Issue 2 | 1-2 hours | memgraphcluster_controller.go |
266426
| Issue 3 | 2-3 hours | memgraphcluster_controller.go, replication.go |
267427
| Issue 4 | 1-2 hours | validation.go |
428+
| Issue 5 | 2-3 hours | statefulset.go, statefulset_test.go |
429+
| Issue 6 | 4-6 hours | replication.go, replication_test.go, memgraphcluster_controller.go, api types (status/conditions) |
430+
| Issue 7 | 3-4 hours | service.go, service_test.go, labels.go, memgraphcluster_controller.go |
268431

269-
**Total: ~6-10 hours**
432+
**Total: ~15-23 hours**
270433

271434
## Version Compatibility
272435

273436
After fixing Issue 1, consider:
274437
- Testing with Memgraph v2.18, v2.19, v2.20, v2.21
275438
- The SHOW REPLICAS format may have changed between versions
276439
- May need version detection and format-specific parsing
440+
441+
**Note (2026-06-04):** The production cluster runs **Memgraph v3.7.2**, while the chart's values claim `memgraph/memgraph:2.21.0` — config drift worth auditing separately. All fixes must be validated against v3.7.2 first; its healthy `SHOW REPLICAS` output is:
442+
443+
```
444+
| name | socket_address | sync_mode | system_info | data_info |
445+
| "x" | "host:10000" | "async" | Null | {memgraph: {behind: 0, status: "ready", ts: N}} |
446+
```
447+
448+
## Incident Reference
449+
450+
The production investigation (2026-06-04) that produced Issues 5-7 and the Issue 1 correction:
451+
intermittent empty proxy results → traced through proxy → `-read` service → empty replica →
452+
operator init container `epoch_id` wipe. Key reproduction technique: run `SHOW REPLICATION ROLE` plus a
453+
count query repeatedly over fresh connections through the service; lockstep flips identify per-pod
454+
divergence that `kubectl port-forward svc/...` (which pins to one pod) cannot reveal.

‎VERSION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
0.1.8
1+
0.2.0

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ require (
4646
github.com/inconshreveable/mousetrap v1.1.0 // indirect
4747
github.com/josharian/intern v1.0.0 // indirect
4848
github.com/json-iterator/go v1.1.12 // indirect
49+
github.com/kylelemons/godebug v1.1.0 // indirect
4950
github.com/mailru/easyjson v0.7.7 // indirect
5051
github.com/moby/spdystream v0.5.0 // indirect
5152
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect

0 commit comments

Comments
 (0)