Repository navigation
CRDT document past DEFAULT_MAX_IMPORT_BYTES is unopenable, and unreadable once the ceiling is raised #230
Description
Activity
Thanks — this is an unusually good report, and the three signals you sampled for fault 2 (flat RSS, flat
write_bytes, zeroread_bytes) are what made it diagnosable without a profiler. I traced all of it. Short version: the two faults and the 32 GB you flagged as "probably its own issue" are one root cause, and it should land as one PR rather than three.The 32 GB is not retired segments — it's flush amplification
nodedb-lite/src/nodedb/core/flush.rs:43:let crdt = self.crdt.lock_or_recover(); for (collection, snapshot) in crdt.export_all_snapshots()? { ops.push(WriteOp::Put { ns: Namespace::LoroState, key: snapshot_key_for(&collection), value: wrap(&snapshot), }); }
Every flush unconditionally re-exports and rewrites the entire Loro snapshot for every collection. There is no dirty check — an idle store with zero writes does the identical full-document work on every tick.
Combined with the defaults in
config/types.rs:139:auto_flush_ms= 1000 (on)auto_compact_ms= 0 (off)
A full 77 MB value rewritten once per second for four hours, each rewrite superseding pages into pagedb's deferred-free list, with the only reclaim path disabled by default. 32 GB of file backing 4 KB of live state is the expected outcome, not an anomaly.
auto_compact.rsdocuments this exact failure mode as the reason the knob exists — the knob is simply off by default.Fault 2 is the same code path
You were right that it isn't lazy index construction, and I was looking in the wrong place for a tight loop. It's the flush loop itself.
Once the snapshot grows large enough that a full
export(ExportMode::Snapshot)exceedsauto_flush_ms, the auto-flush task runs back-to-back indefinitely, holding thecrdtmutex for essentially 100% of wall time. Against your measurements:Observation Cause 100% CPU, read_bytes0in-memory Loro serialization, no I/O RSS exactly flat the same ~77 MB buffer allocated and freed each pass non-CRDT route answers in 0.5 ms never acquires the crdtlockevery CRDT read never returns query/document_ops/reads.rs:71takesengine.crdt.lock()and starvesshutdown flush makes no progress, write_bytesfrozenthe final flush queues behind in-flight exports; the export precedes batch_writeAll five, including the two that made it look like an infinite loop. It's metastable: larger document → longer export → larger lock duty cycle → readers never scheduled. That is also why SIGTERM didn't help — the shutdown flush was never going to get the lock.
If you want to confirm on your preserved store before touching anything, it's a two-minute change rather than a profiler: log elapsed around
export_all_snapshots(). Anything overauto_flush_msconfirms it.One root cause
All three faults are the same assumption in different places: a collection's own full snapshot is treated as a small, cheap value — cheap to rewrite (the 32 GB), cheap to re-serialize while holding a reader-visible lock (fault 2), and small enough to fit a peer-message budget (fault 1).
That's why it wants one PR. Fixing them separately means each fix looks locally reasonable while the assumption survives.
On the design question you asked
You framed fault 1 as separate-the-paths versus make-the-limits-configurable. It's both, and the "versus" is what produced the bug — a peer import and a local restore are different operations that happen to share a decoder.
Two things worth adding to your read of it:
The recovery path is self-blocking.
compact_history(nodedb-crdt/src/state/snapshot.rs:92) andcompact_at_version(history.rs:108) both re-admit their own locally-generated shallow snapshot underCrdtImportLimits::default(). Past 64 MiB the store can't open and can't compact — the one operation that would bring it back under the ceiling is gated by the ceiling. Auto-compact would have been failing behindtracing::warn!("auto-compact failed")well before the open failure surfaced; grepping your logs for that will probably date the onset.max_bytesis not the only wall.DEFAULT_MAX_IMPORT_OPS = 1_000_000capsmax_encoded_operations, counted over the whole snapshot's op range incount_range_operations. Raising onlymax_bytes— what you patched — leaves a second ceiling that a larger store hits next with a different error.The underlying invariant:
export_snapshothas no bound at all, while import does. Export is unbounded, import is capped, so durable state a healthy store writes is not guaranteed re-importable by the same binary. Any fix that only moves a number leaves that unenforced one ceiling higher.Suggested shape for the PR
Four commits, in this order:
-
Dirty-tracked flush. Track the last successfully-flushed frontier per collection in
CrdtEngine; inflush.rs, export andPutonly collections whoseoplog_version_vector()has moved. An idle store must do zero snapshot work per tick. This alone removes the 32 GB and the idle spin. -
Snapshot rewrite becomes a checkpoint, not a per-flush operation. Dirty-tracking still means one full rewrite per second under sustained write load, which is your actual workload. The incremental delta path already exists (
crdt:delta:{mutation_id:016x},restore_pending_deltas_incrementalincore/open/restore/crdt.rs) — append deltas per flush and rewrite the base snapshot only when accumulated delta bytes cross a fraction of snapshot size. Per-flush cost goes from O(document) to O(new ops). Largest piece, and the one that makes the fix hold at 10× your volume. -
Bound lock hold time. Even at checkpoint cadence a multi-second export under
self.crdtstarves every reader. Export outside the reader-visible lock, and add a flush-in-flight guard so ticks cannot stack. -
Separate local restore from peer admission. Restore,
compact_historyandcompact_at_versionget a distinct entry point that keeps the structural checks (decode_import_blob_meta,ImportInvalidOperationRangeon regressing per-peer ranges) and drops the byte and op ceilings together. Keep the peer path capped, and threadCrdtImportLimitsout to the embedding caller as you suggested. A distinct entry point rather than anunbounded()constructor, so peer bytes can't reach the exempt path by accident.
Ordering matters for you specifically: commit 4 alone reproduces exactly what you already found by patching the constant — the store opens and then hangs. Commits 1–3 without 4 leave your existing store unopenable. Recovering it needs all four.
Regression coverage to include:
- an idle store performs zero snapshot exports across N flush ticks (counter assertion, not a timing assertion)
- a document exceeding
DEFAULT_MAX_IMPORT_BYTESsurvives a close/reopen round-trip - file size after M writes and flushes stays within a bound of live-state size
Acceptance is your preserved store: it opens, serves reads, and compacts down. Please keep it until this merges.
On
auto_compact_msdefaulting to 0 — I'd leave it off. Turning it on makes every embedder pay repack cost to paper over write amplification that commits 1 and 2 remove at the source. It should get a doc note saying it's a mitigation, not the fix.One thing to have in hand before you start: which route hangs. If commits 1–3 don't clear it, the next suspects are
CrdtState::collection_names()(nodedb-crdt/src/state/core.rs:288— a fullget_deep_value()deep clone of every collection, row and field, purely to read top-level keys, reachable fromquery/catalog.rsduring name resolution) andestimated_memory_bytes()(snapshot.rs:105, a full snapshot export per call). Both are the same "whole document is cheap" assumption and are worth folding into the same PR regardless —collection_namesis a one-line change to key iteration.Happy to review the PR whenever it's ready.
That traces further than I could and reframes it usefully — thank you. Agreed it's one PR; splitting it would have let the "whole document is cheap" assumption survive in whichever place I didn't touch.
Two corrections/data points from this side:
The auto-compact log grep won't date the onset here — compaction was off. This deployment runs
auto_compact_ms = 0explicitly, set weeks ago after measuring multi-second write stalls on the 5-minute tick. So there are zeroauto-compact failedlines: it never attempted. I think that strengthens the diagnosis rather than weakening it — the 32 GB accumulated with the reclaim path fully disabled, so it is flush amplification alone, with no repack masking or partially offsetting it. It also means the self-blocking recovery path you found was never reached, and the first symptom I saw was the open failure.It's a useful accident for your commits 1 and 2: this store is a clean measurement of write amplification with no compaction interaction at all.
I had patched only
max_bytes, soDEFAULT_MAX_IMPORT_OPSis a wall I had not hit yet. Good catch — my patched build opened this particular store, which means it was under 1M ops, but I would have walked straight into that ceiling at the next size and read it as a new bug.The framing I'll carry forward is the invariant you named: export is unbounded, import is capped, so a healthy store can write durable state the same binary cannot re-import. That's the thing worth a regression test independent of any specific number.
The preserved store is kept and will stay until this merges — 32 GB, ~26.4k objects in one collection, 77.4 MB snapshot, and it still reproduces on
da6f18377. Happy to run theexport_all_snapshots()timing patch against it and report the elapsed distribution, and to be the acceptance check for the PR whenever you want it exercised.Status update. Three PRs have landed on
mainagainst this issue. Keeping it open — the store described here still would not open on a build frommaintoday, and the reason is below.Fault 1 — the local restore path bounded by a peer-facing budget
Fixed on the Origin side.
import_local/from_local_snapshotadmit bytes this process produced itself with every structural check (authenticated metadata decode, per-peer range regression, pending dependencies) but without the size ceilings, which only answer a question about untrusted peers. The peer path is unchanged andimport_with_limitsis still the knob for it.- feat(crdt): separate local reload from peer admission #231 — the split, plus the compaction paths, transaction rollback, and the validation-candidate seed
Not yet fixed where you hit it. Your error text comes from
nodedb-lite/src/engine/crdt/engine/lifecycle.rs:151, which still calls the cappedCrdtState::import. That is a one-line change toimport_localinnodedb-lite, gated on a release carrying it. Until that lands, a store past the ceiling still will not open, so this issue stays open.Fault 2 — reopened document pegs a core on every read
We believe this is
estimated_memory_bytes, which answered "how big is this document?" by serialising the whole document, and whichupdate_memory_statscalls at the end of every write path and on the health route.Measured on a 25k-row document (4.1 MB encoded): 26.2 ms per measurement cold, and 101 ms per measurement after a single write. Your figure of 25,120 records in ~4 hours — 0.57 s/record — is consistent with this at your document's size.
One note on the diagnosis in the report: flat RSS does not rule out allocation here. The export allocates a
Vecand drops it per call, so a recycled transient allocation and no allocation look identical at that sampling resolution. 100% CPU with zero I/O fits a full in-memory re-encode as well as it fits a spin.- feat(crdt): separate local reload from peer admission #231 also memoised the estimate, which is what makes repeated reads on an unchanged document cheap (26 ms → 1 µs)
- perf(crdt): calibrate the memory estimate instead of re-exporting per write #232 stopped the re-encode per write: the export now calibrates a bytes-per-operation ratio and the answer comes from an oplog counter. 25k writes each followed by a measurement went from ~42 min to 373 ms, with the estimate landing within 0.5% of the real encoded size
- perf(crdt): make delta apply cost the delta, not the collection #233 addressed the delta apply path, which is what WAL replay runs per record on recovery: 142 ms → 8.8 ms per delta on a 1,500-row collection. Most of that was a UNIQUE probe building and re-resolving a string path per row (129 ms → 445 µs)
What would close this
The offer to run instrumented builds against the preserved 32 GB store is the acceptance case, and we would rather take it than close on extrapolation. Specifically:
nodedb-literestore switched toimport_local— without it the store still will not open, and nothing else can be observed- Then, on that store: does it open, and do reads return? If reads still hang with the estimate no longer re-encoding per operation, fault 2 has a second cause we have not found, and the profile would be worth having
None of the above has been run against a document of your size — the measurements are from 1.5k–25k row fixtures. The shape of the costs is size-independent; the multipliers are not.
Separately, the closing observation in the report — that the 32 GB on disk is not CRDT state, since open reads only 4 KB — does look like its own issue (retired-segment accumulation) and is not addressed by any of the above.
Both faults are fixed on
main. Closing.Fault 1 — local restore bounded by a peer-facing budget
import_localadmits bytes this process produced itself with every structural check — authenticated metadata decode, per-peer ranges that do not regress, pending dependencies — and without the size ceilings, which only answer a question about untrusted peers. The peer path is unchanged andimport_with_limitsremains the knob for it.- Origin: 5ae4a50 (feat(crdt): separate local reload from peer admission #231) — the split, plus compaction, transaction rollback, and the validation-candidate seed
- Lite: 789122b —
import_snapshotnow restores through the local path
That Lite commit is the one that matters for the store in this report. Your error text came from
nodedb-lite/src/engine/crdt/engine/lifecycle.rs, and that line was still calling the capped import until now. Peer snapshots are unaffected — sync routes them throughimport_remote, which stays capped.Fault 2 — reopened document pegs a core on every read
The cause was
estimated_memory_bytes, which answered "how big is this document?" by serialising the whole document, and whichupdate_memory_statscalls at the end of every write path and on the health route. Every operation re-encoded the entire document.Measured on a 25k-row document (4.1 MB encoded): 26.2 ms per measurement cold, 101 ms per measurement after a single write. Your 25,120 records in ~4 hours — 0.57 s/record — is that cost at your document's size.
One correction to the diagnosis in the report: flat RSS does not rule out allocation. The export allocates a
Vecand drops it per call, so a recycled transient allocation is indistinguishable from no allocation at that sampling resolution. 100% CPU with zero I/O fits a full in-memory re-encode as well as it fits a spin.- 5ae4a50 (feat(crdt): separate local reload from peer admission #231) also memoised the estimate — 26 ms to 1 µs for repeated reads on an unchanged document
- aaa98bd (perf(crdt): calibrate the memory estimate instead of re-exporting per write #232) removed the per-write re-encode: the export now calibrates a bytes-per-operation ratio and the answer comes from an oplog counter. 25k writes each followed by a measurement went from ~42 min to 373 ms, with the estimate landing within 0.5% of the real encoded size
- 30fa370 (perf(crdt): make delta apply cost the delta, not the collection #233) fixed the delta apply path, which WAL replay runs per record on recovery: 142 ms to 8.8 ms per delta on a 1,500-row collection. Most of it was a UNIQUE probe building and re-resolving a string path per row, 129 ms to 445 µs
What has not been verified
This is closed on the code, not on your store. Nothing here was run against the preserved 32 GB store, and the measurements come from 1.5k–25k row fixtures — the shape of the costs is size-independent, the multipliers are not. If it opens but reads still hang, fault 2 has a second cause we did not find, and reopening this with that profile would be the fastest way to it. Thank you for the report and for keeping the store.
Also, the closing observation in the report still stands on its own: 32 GB on disk when open reads only 4 KB looks like retired-segment accumulation, and none of the above touches it. Worth a separate issue.
A CRDT document that grows past
DEFAULT_MAX_IMPORT_BYTESbecomes permanently unopenable, and raising the ceiling then exposes a second fault: every read on the reopened document spins at 100% CPU with no I/O.Reproduced on
da6f18377(current main). Both faults are innodedb-crdt; the callers are in nodedb-lite. pagedb is not involved.1. The local restore path is bounded by a peer-facing DoS budget
admit_importrejects an encoded import larger thanDEFAULT_MAX_IMPORT_BYTES(64 MiB):That bound is correct against a hostile peer's encoded import. It is also reached from the snapshot restore path on open, where the bytes are the document's own durable state. There, "too large" is not an attack — it is a document that did its job.
The consequence is that a store crosses the threshold by succeeding at writes, and only finds out at the next open:
There is no escape hatch for an embedder.
CrdtImportLimitsis public, but nothing in nodedb-lite ever constructs one —grep -rn CrdtImportLimits nodedb-lite/srcis empty — so the restore path always usesDefault. An operator holding an unopenable store cannot raise the bound through configuration; only a recompile of nodedb helps.Observed density in our workload is ~2.9 KB of encoded CRDT state per stored object, which puts the practical ceiling near 23k objects in one document.
2. With the ceiling raised, the document opens but cannot be read
Patching the constant to 1 GiB and rebuilding, the same store opens successfully. It is then unusable:
write_bytesflat,read_bytes0No allocation and no I/O while pegging a core is a tight loop, not lazy index construction.
Graceful shutdown does not complete either — after SIGTERM the process held 99.8% CPU for 7.5 min with
write_bytesfrozen at exactly 307200. A shutdown timeout cannot help a flush that makes no progress. (SIGKILL was safe here precisely because nothing had been written since the signal.)Fault 2 is the blocker. Fixing only the ceiling makes such a document openable but still unreadable.
Reproduction
DEFAULT_MAX_IMPORT_BYTES, rebuild, reopen → opens, then fault 2 on the first read.The write path itself was healthy throughout: 25,120 records with zero errors, no AEAD or corruption diagnostics over ~4 hours.
I have a 32 GB store in this state preserved and can run any instrumented build against it. Also worth noting separately: that 32 GB is not CRDT state — open reads only 4 KB — so the on-disk size looks like retired-segment accumulation and is probably its own issue.
Suggested direction
For fault 1, the two paths want different bounds. A peer import is untrusted and should stay capped; a local restore is bounded by what the store already committed, so either exempt it or give it its own limit. Threading
CrdtImportLimitsout to the embedding caller would also let an operator recover an existing store without recompiling.I can send a PR once you indicate which shape you prefer — separating the paths versus making the limits configurable — since either touches the public surface.
For fault 2 I do not have a root cause. Diagnosing the spin needs a profiler with privileges I would rather not enable on this host; if there is an instrumented build or a specific counter you want sampled, I can run it against the preserved store.