Просмотр исходного кода

docs(replica-eviction): document why the simple fix (v1.7.2 draft) echoes infinitely

Tried the one-line fix option 1 — add persistence_->logInsert in
applyReplicatedEntry. It caused infinite replication amplification:
follower's WAL leaks back to leader via GetEntriesSince, leader
re-applies (thinks they're follower's writes), re-logs to own WAL,
restreams back. 5000 inserts → 216k+ WAL entries + 1.9 GB follower
RSS against a 16 MB cap.

Root cause at database_grpc_impl.cpp:1561 — GetEntriesSince sets the
response node_id to nodeState.nodeId (its own) instead of preserving
the WalEntry's original origin. WalEntry doesn't even carry the
origin — it's an on-disk format limitation that predates replication's
current state.

Proper fix is a v1.8.0-class change: WAL format version bump,
persistence_->log* APIs accept explicit originNodeId, sync protocol
filters by peer != originNodeId on the send side.

v1.7.x remains at v1.7.1. Known limitation: followers that evict
replicated docs lose them. Size the follower to hold the working set
until the proper fix ships."
fszontagh 3 месяцев назад
Родитель
Сommit
f35a28e9a0
1 измененных файлов с 23 добавлено и 3 удалено
  1. 23 3
      tests/load_test/README.md

+ 23 - 3
tests/load_test/README.md

@@ -405,10 +405,30 @@ When eviction then fires on the follower, the doc is dropped to an evicted-stub.
 
 Fix options (not made in this changeset — the test documents the bug):
 
-1. In `applyReplicatedEntry`, after `store_->loadDocument(...)`, append to the local WAL (e.g. `persistence_->appendInsert(collection, doc)` / `appendUpsert` depending on op), so evicted-on-follower docs have a durable backing store. Need to make sure this does not trigger re-replication — either a separate WAL-only path or a flag on `loadDocument` that writes to WAL but suppresses the replication-broadcast callback.
-2. Alternatively, teach the eviction callback to fall back to pulling from a peer when the local WAL has no entry — more complex, adds a network hop on cold read.
+1. In `applyReplicatedEntry`, after `store_->loadDocument(...)`, append to the local WAL (e.g. `persistence_->logInsert/logUpdate` depending on op), so evicted-on-follower docs have a durable backing store.
+2. Teach the eviction callback to fall back to pulling from a peer when the local WAL has no entry — more complex, adds a network hop on cold read.
 
-Both leader's broadcast *and* follower's `loadDocument` side were intentionally separated to avoid a replication loop; option (1) is the less invasive of the two.
+### Attempted fix (option 1) and why it failed
+
+A v1.7.2 attempt added `persistence_->logInsert(collection, doc)` directly inside `applyReplicatedEntry` after the `loadDocument` call. The fix compiled cleanly but **caused infinite replication echo amplification** when run:
+
+```
+[23:30:14] Memory check: 1954 MB estimated (100%, pressure=emergency),
+           408121 docs, 30747 evicted
+[23:30:14] Pulled 5000 entries from peer replica-leader-128
+           (seq: 211866 -> 216867)
+```
+
+The leader's WAL reached 216867+ entries after inserting only 5000. Root cause is at `service/src/database_grpc_impl.cpp:1561` — the `GetEntriesSince` server handler hard-codes the response's `node_id` to its OWN `nodeState.nodeId`, not the entry's original origin. So when follower's WAL is streamed back to the leader, every entry claims to have originated on the follower. The leader then can't recognize its own original writes and re-applies them, re-logs to its WAL, re-streams them to the follower, and the cycle continues.
+
+The proper fix requires:
+
+1. Adding a `nodeId` field to `struct WalEntry` (`service/src/persistence/wal.hpp:37`) — **WAL on-disk format change, breaks backward compat without a version bump**
+2. Plumbing the origin nodeId through `persistence_->logInsert/logUpdate/logDelete`
+3. Using `walEntry.nodeId` (not `nodeState.nodeId`) in `GetEntriesSince`
+4. The sync protocol on the sending side skipping entries whose `node_id` equals the peer being sent to
+
+This is a v1.8.0-class change that warrants its own design + format migration path. For v1.7.x the documented limitation stands: **replicated docs on a follower that get evicted are lost. Size the follower's `max_memory_mb` to hold the working set until a proper replication overhaul ships.**
 
 ### Test output the bug produces