From ac8a5e07a4eb24967a0c9830a1cb6cee90bb2988 Mon Sep 17 00:00:00 2001 From: Atanas Oreshkov Date: Mon, 10 Aug 2026 21:27:21 +0300 Subject: [PATCH] Drain a buffer, copy a store while it is being written, and bound a hostile expression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier 1: one item per remaining gap, in the order the readiness note sequences them. Four use cases were each blocked on something specific, and none of the seven items needed a format change. Bound what an untrusted JSONPath expression may cost. The module's chosen use case is expressions you did not write, and until now nothing bounded what a *small, valid* query cost against a *large* document: `$..*..nope` is fourteen characters, is quadratic in the document's node count, and returns the empty nodelist — so nothing measured on the answer can see it coming. That is why the fixture names a field the document does not have, and why the bound has to be on the work. The bound refuses; it never truncates. `rabosh-jsonpath`'s own conventions said the walk carries no budget, because one that stopped early and returned what it had would be a wrong answer with nothing to say so. That rule is unchanged and still governs. `JsonPathLimitExceededException` is the opposite mechanism: the caller gets no nodelist rather than a short one. `Evaluation.stop()` therefore throws rather than returning a `Boolean`, deliberately, because reusing the sink's own "stop" would look like a simplification while reintroducing exactly the truncation the rule forbids. Counted in steps and never on a clock, for the reason the I-Regexp bound is. All 703 compliance cases and the module's 20 000-deep and 5 000-wide fixtures pass under the shipped defaults, which are a backstop rather than a policy. Copy a store while it is being written. The recipe `checkpoint` replaces was *stop writing and copy the directory*, which a desktop application cannot do because it is the writer. Flush, pin a snapshot, link what that snapshot sees. The ordering rule — log, then memtable, then segment, then manifest, then delete — governs the target as much as the source, so every data file is durable before the manifest naming it exists and `CURRENT` is written last. Core copies every file *numbered after* a live segment rather than a list of suffixes, so it needs no knowledge of what a `.cat` or `.pst` is and a sidecar kind added later travels for free. `INDEXES` is named rather than numbered and cannot travel that way, so `IndexCatalog.copyRegistryTo` carries it — losing it would lose an instruction rather than derived data, and leave the posting files as orphans. The sidecars are read by the copy and not rebuilt, verified by opening the checkpoint with backfilling off and by breaking it. The fault suite fails the copy at four steps and asserts the source is unharmed at each. The step it arms is worth knowing: the segments are hard-linked, so no byte is written for one and a `WRITE` fault never fires. `FORCE` is what happens either way, and it is the step the ordering rule is actually about. Retire a key range. `deleteRange` is the loop a caller would otherwise write, and writing it correctly means knowing four invariants of the layer below. Point deletes in bounded batches, deliberately: no new operation id, no format change, no change to what a merge emits or what `EntryCursor` collapses, and above all no change to the tombstone-drop rule, which fails by returning a deleted document to a reader. A real range tombstone is the other design and needs a measurement first. Say when a predicate cannot match the data's types. In a third-party archive a field arriving as `"500"` in some payloads and `500` in others is the normal state of the world, and the symptom is a query returning fewer rows with nothing to say why. A diagnostic, never a coercion: type bracketing is unchanged and `ColumnPredicate.matches` is still the only definition. The family travels on `Normal.Leaf` from the lowering because `ColumnPredicate.kind` is internal to `rabosh-index`, and re-deriving it in the query layer would be that second definition arriving by the back door. Reported over every leaf, not only indexed ones — a path with no index is where a caller has no other signal at all. Get bytes out for a lakehouse, with no Parquet dependency taken. A document read from a segment carries *that segment's* shared dictionary, so handing `(metadata, value)` to something expecting a self-contained Variant is a trap that sometimes works — the test pins that, using document 50 rather than document 0, because document 0's two names happen to occupy the same dictionary ids in both and read back perfectly. `detached()` rebuilds with a dictionary of its own. `shreddingAdvice` renders what the catalog already computes for a shredding schema, including the decision a hand-written one gets wrong: whether `variant_value` can be dropped. Make the second-instance case legible. Two instances of a desktop application on one data directory is not an error, it is Tuesday. `StoreLockedException` now carries the directory and a `LockHolder`. The start time is not decoration: operating systems reuse pids, so `isRunning` requires a live process with this id *and* this start instant — reporting a pid on the strength of the number alone is how a user ends up killing a stranger's process. The lock moved to byte zero with the record after it, because a Windows file lock is mandatory and a second process could not otherwise read the record at the one moment it wants to; the two regions overlap, so a build using either scheme still excludes one using the other. No lock stealing, no timeout, no force-open. And the drain sample, which is the acceptance test for the two items above in the only way that matters — a caller's program. Snapshot, scan from the watermark, ship, *then* record the watermark, retire, compact. Every mistake in that order is silent. Writing it found the one thing the inclusive bounds cannot say, so `Key.successor()` is now public: the sample accepted by failing first. Phase 23 published a deprecation cycle for the stable core and this is its first live test. `StoreLockedException` needed a new constructor; the old two-argument one is still there, `@Deprecated(WARNING)` with a `ReplaceWith`, which is why `directory` is `Path?` rather than `Path`. The ABI dumps grew by 107 lines and shrank by none. Co-Authored-By: Claude Opus 5 --- .claude/rules/index-and-query.md | 13 + .claude/rules/storage-durability.md | 45 +++ CHANGELOG.md | 53 ++++ CLAUDE.md | 5 + COMPATIBILITY.md | 9 +- INTEGRATION.md | 91 +++++- README.md | 14 +- STABILITY.md | 30 +- rabosh-api/api/rabosh-api.api | 5 + .../kotlin/app/oreshkov/rabosh/api/Rabosh.kt | 64 +++++ .../rabosh/api/LakehouseHandoffTest.kt | 207 ++++++++++++++ .../rabosh/api/RaboshCheckpointTest.kt | 142 ++++++++++ rabosh-catalog/api/rabosh-catalog.api | 19 ++ .../rabosh/catalog/ShreddingAdvice.kt | 162 +++++++++++ rabosh-core/api/rabosh-core.api | 29 ++ .../app/oreshkov/rabosh/core/Checkpoint.kt | 192 +++++++++++++ .../app/oreshkov/rabosh/core/DocumentStore.kt | 141 ++++++++++ .../kotlin/app/oreshkov/rabosh/core/Key.kt | 30 ++ .../oreshkov/rabosh/core/StoreDirectory.kt | 104 ++++++- .../oreshkov/rabosh/core/StoreException.kt | 79 +++++- .../oreshkov/rabosh/core/CheckpointTest.kt | 178 ++++++++++++ .../oreshkov/rabosh/core/DeleteRangeTest.kt | 183 ++++++++++++ .../oreshkov/rabosh/core/LockHolderMain.kt | 38 +++ .../app/oreshkov/rabosh/core/StoreLockTest.kt | 123 ++++++++ rabosh-index/api/rabosh-index.api | 1 + .../app/oreshkov/rabosh/index/IndexCatalog.kt | 26 ++ rabosh-jsonpath/CLAUDE.md | 41 ++- rabosh-jsonpath/api/rabosh-jsonpath.api | 42 +++ .../rabosh/jsonpath/JsonPathEvaluator.kt | 221 +++++++++++---- .../rabosh/jsonpath/JsonPathLimits.kt | 146 ++++++++++ .../oreshkov/rabosh/jsonpath/JsonPathQuery.kt | 68 ++++- .../rabosh/jsonpath/JsonPathLimitsTest.kt | 266 ++++++++++++++++++ rabosh-query/api/rabosh-query.api | 10 + .../oreshkov/rabosh/query/DocumentMatcher.kt | 5 +- .../app/oreshkov/rabosh/query/Explain.kt | 107 +++++++ .../app/oreshkov/rabosh/query/Normal.kt | 69 ++++- .../rabosh/query/ExplainTypeNoteTest.kt | 185 ++++++++++++ rabosh-samples/build.gradle.kts | 8 + .../app/oreshkov/rabosh/samples/DrainMain.kt | 214 ++++++++++++++ .../oreshkov/rabosh/samples/SamplesTest.kt | 41 +++ .../oreshkov/rabosh/testkit/crash/ChildJvm.kt | 9 + rabosh-variant/api/rabosh-variant.api | 1 + .../app/oreshkov/rabosh/variant/Variant.kt | 31 ++ 43 files changed, 3330 insertions(+), 117 deletions(-) create mode 100644 rabosh-api/src/test/kotlin/app/oreshkov/rabosh/api/LakehouseHandoffTest.kt create mode 100644 rabosh-api/src/test/kotlin/app/oreshkov/rabosh/api/RaboshCheckpointTest.kt create mode 100644 rabosh-catalog/src/main/kotlin/app/oreshkov/rabosh/catalog/ShreddingAdvice.kt create mode 100644 rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/Checkpoint.kt create mode 100644 rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/CheckpointTest.kt create mode 100644 rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/DeleteRangeTest.kt create mode 100644 rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/LockHolderMain.kt create mode 100644 rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/StoreLockTest.kt create mode 100644 rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathLimits.kt create mode 100644 rabosh-jsonpath/src/test/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathLimitsTest.kt create mode 100644 rabosh-query/src/test/kotlin/app/oreshkov/rabosh/query/ExplainTypeNoteTest.kt create mode 100644 rabosh-samples/src/main/kotlin/app/oreshkov/rabosh/samples/DrainMain.kt diff --git a/.claude/rules/index-and-query.md b/.claude/rules/index-and-query.md index 6adb417..2c5602f 100644 --- a/.claude/rules/index-and-query.md +++ b/.claude/rules/index-and-query.md @@ -139,6 +139,19 @@ differential suites that hold every claim here in `.claude/rules/testing.md`. per element. An element ordinal space was refused against it; the numbers are in the open-work index's Tier 2 record. +- **`explain` may report a type mismatch and may never repair one.** `ExplainTypeNote` says when a + leaf's family disagrees with the types the catalog observed at its path — a numeric comparison where + a fifth of the values are strings — because in a third-party archive that is the normal state of the + world and the symptom is a query returning fewer rows with nothing to say why. It changes no answer, + no plan and no bound, which is exactly why `explain` is the right place for it: nothing there can be + got wrong in a way that costs a document. **Anything that made a numeric predicate match a string + would be a second definition of `ColumnPredicate.matches` and would break skipping.** The family + travels on `Normal.Leaf` from the lowering, because `ColumnPredicate.kind` is `internal` to + `rabosh-index` and re-deriving it in the query layer would be that second definition arriving by the + back door. A leaf that brackets to nothing — `EXISTS`, `IS NULL`, a mixed `IN` — reports nothing, + since there is no family for the data to disagree with, and the note is over **every** leaf rather + than only indexed ones, because a path with no index is where a caller has no other signal at all. + - **A negated leaf is never a flipped operator.** `not($.a >= 10)` holds for a document whose `a` is a string and for one with no `a`; `$.a < 10` holds for neither. The normaliser keeps the negation on the leaf and applies it to the *document's* answer. The rewrite is the most natural-looking diff --git a/.claude/rules/storage-durability.md b/.claude/rules/storage-durability.md index 83614e7..e9bf3fe 100644 --- a/.claude/rules/storage-durability.md +++ b/.claude/rules/storage-durability.md @@ -44,3 +44,48 @@ Cross-cutting rules are indexed in the root `CLAUDE.md`; module conventions in - **`DocumentStore.write` publishes the read bound last.** A batch is atomic to one view because `visibleSequence` moves only once every operation is in the memtable. The unit is the view, not the call. + +- **A checkpoint obeys the ordering rule in the *target* directory, not only in the source.** *Log, + then memtable, then segment, then manifest, then delete* governs `writeCheckpoint` exactly as it + governs a flush: every data file is linked or copied **and forced** before the manifest naming it is + written, and `CURRENT` is written last of all. A checkpoint that forced its manifest first is the + same bug in a new place and fails the same way — a directory that opens and then cannot find a + segment. Two consequences that are decisions rather than details. The **snapshot is held open across + the copy**, which is what stops a compaction reclaiming a segment out from under it; that is why the + snapshot is in the design rather than being a way to pick a sequence. And **no log is copied**, + which is correct only because `checkpoint` flushes first — a copied log would replay into sequence + numbers the checkpoint's own manifest has already issued. + + What core copies is *every file numbered after a live segment*, never a list of suffixes: + `rabosh-core` does not know what a `.cat`, `.idx`, `.pst` or `.col` is and must not learn, so a + sidecar kind added later travels with no change here. The one file that is **named** rather than + numbered is `INDEXES`, which is `IndexCatalog.copyRegistryTo`'s job and is why `Rabosh.checkpoint` + exists rather than the facade delegating and stopping. Losing it would lose an *instruction* rather + than derived data — the inversion `index-and-query.md` states — and would leave the posting files as + orphans for the next sweep. + + A failed checkpoint is not unwound, deliberately: the target is not a store until `CURRENT` names + its manifest, so a partial one is a directory to throw away rather than a state to repair. What is + asserted instead is that the **source is unharmed**, at every step, by the fault-injecting + filesystem. Note which step the fault is armed on: the segments are *hard-linked*, so no byte is + written for one and a `WRITE` fault never fires — `FORCE` is the step that happens either way, and + it is the one the ordering rule is about. + +- **`deleteRange` is point deletes, and staying that way is the decision.** One snapshot scopes the + whole loop, keys are collected a batch at a time rather than all at once, and the next scan resumes + at `Key.successor()` of the last key handled — an inclusive bound restarted at that key would rescan + a range whose head is now a tombstone. No new operation id, no format change, no change to what a + merge emits or what `EntryCursor` collapses, and above all no change to the tombstone-drop rule, + which is on the short list of invariants that fail by returning a deleted document to a reader. A + real range tombstone is the other design and the format has room for it; it needs the + bytes-written-per-byte-retired measurement first, and that question belongs in the open-work index. + +- **The `LOCK` file's byte zero is the lock and everything after it is a diagnostic.** `tryLock()` + with no arguments takes `[0, Long.MAX_VALUE)`, and a Windows file lock is *mandatory* — so a second + process could not read a holder record written inside it, which is exactly when it wants to. Locking + one byte and writing `pid=… startedAt=…` after it leaves the record readable on every platform, and + the two regions overlap at byte zero so a build using either scheme still excludes one using the + other. The record is **not** forced and is not part of any ordering rule: losing it costs a better + error message and never a document. An empty record reads as *holder unknown*, which is what a store + last opened by an older release looks like, and `LockHolder.isRunning` checks the start time as well + as the pid because pids are reused. diff --git a/CHANGELOG.md b/CHANGELOG.md index 61515b4..c4c98a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,59 @@ else may change in any release. That claim lives in [STABILITY.md](STABILITY.md) ### Added +- **`Rabosh.checkpoint(target)` — a consistent copy, taken while you are writing.** The recipe it + replaces was *stop writing and copy the directory*, which a desktop application cannot do because it + is the writer. The database is flushed, a snapshot is pinned, and the copy is of what that snapshot + sees, so it holds exactly the acknowledged prefix as of `CheckpointInfo.sequence`. Segments are + hard-linked where the filesystem allows it, so it costs a directory entry per file rather than its + bytes — which also means it is a consistent *view* rather than an off-site backup. + + Sidecars travel with their segments and are **read** by the copy rather than rebuilt, verified by + opening the checkpoint with backfilling off. The `INDEXES` registry travels too, because an index + definition is an instruction somebody gave rather than derived data. The fault-injecting filesystem + fails the copy at four steps and the **source is unharmed in every case**. + +- **`Rabosh.deleteRange(from, to)` — retention by key range**, both bounds inclusive and both + optional. The loop a caller would otherwise write, which to write correctly means knowing four + invariants of the storage layer. Deliberately point deletes rather than an LSM range tombstone: no + new operation id, no format change, no change to compaction. Follow it with `compact()`, which is + what turns tombstones into reclaimed space. + +- **`JsonPathLimits` — bounded evaluation for untrusted JSONPath.** `rabosh-jsonpath`'s chosen use + case is expressions you did not write, and until now nothing bounded what a *small, valid* query + cost against a *large* document: `$..*..nope` is fourteen characters, is quadratic in the document's + node count, and returns the empty nodelist — so nothing measured on the answer can see it coming. + + **The bound refuses; it never truncates.** Exceeding it raises `JsonPathLimitExceededException` and + delivers nothing, because a short nodelist cannot be told from a small document. Counted in steps + and never on a clock, for the reason the I-Regexp bound is. All 703 compliance cases and the + module's 20 000-deep and 5 000-wide fixtures pass under the shipped defaults, which are a backstop + rather than a policy — a deployment serving hostile expressions should set its own, far lower. + +- **`explain()` says when a predicate cannot match the data's types.** A numeric comparison against a + path where a third of the values arrive as strings now carries a note on the plan. A *diagnostic, + never a coercion*: type bracketing is unchanged, `ColumnPredicate.matches` is still the only + definition, and nothing here makes a numeric predicate match a string. Reported for leaves with no + index too, which is where a caller has no other signal at all. + +- **`Variant.detached()` and `InferredSchema.shreddingAdvice()` — the lakehouse hand-off**, with no + Parquet dependency taken. A document read from a segment carries *that segment's* shared dictionary, + so handing `(metadata, value)` to something expecting a self-contained Variant is a trap that + sometimes works — `detached()` rebuilds it with a dictionary of its own. The advice renders what the + catalog already computes for a Parquet **shredding schema**, including the decision a hand-written + one gets wrong: whether `variant_value` can be dropped. + +- **`StoreLockedException` says who holds the directory.** It carries `directory` and a `LockHolder` + with the pid and start time the `LOCK` file records, so a desktop application's second launch can + focus the existing window instead of matching on a message. **The start time is not decoration** — + operating systems reuse pids, and `isRunning` checks both, so a user is never told to kill a + stranger's process. No lock stealing, no timeout, no force-open. + +- **`:rabosh-samples:runDrain`** — a staging buffer drained: snapshot, ship, record the watermark, + retire, compact, in that order. It composes the two items above, which makes it their acceptance + test in the only way that matters, a caller's program. Also **`Key.successor()`**, promoted to + public because writing that loop found it was the one thing the inclusive bounds cannot say. + - **[`INTEGRATION.md`](INTEGRATION.md) — the runtime contract, in public.** The rules an embedding application has to obey were discoverable only by reading KDoc on classes a caller may never open, and three of them fail *silently*: a row is valid only until the next `next()`, a leaked `Snapshot` diff --git a/CLAUDE.md b/CLAUDE.md index 67253db..b97e9da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -102,6 +102,7 @@ it fails **silently** — a document missing from a result, a file that stops me - A null slot holds the type's zero, and that zero must never reach a bound. - Type bracketing is part of the query contract, because skipping depends on it. - A negated leaf is never a flipped operator. +- `explain` may report a type mismatch and may never repair one. - A plan's candidates are a superset and its certainties a subset, and the gap is what gets read. - A composite index needs every declared field fixed by equality and does not care what else the query asks: **more** conjuncts are dropped and cost it only its certainty, **fewer** are unsound. @@ -115,6 +116,10 @@ it fails **silently** — a document missing from a result, a file that stops me ### The write path — `.claude/rules/storage-durability.md` +- A checkpoint obeys the ordering rule in the target directory, not only in the source. +- `deleteRange` is point deletes, and staying that way is the decision. +- The `LOCK` file's byte zero is the lock and everything after it is a diagnostic. + - The log is appended before the memtable is touched, always. - A torn tail may be dropped; anything that would lose an acknowledged commit is reported. - A tombstone may only be dropped at the bottom-most level that can contain the key, and only below diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index 4048185..b8168c1 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -38,8 +38,13 @@ are embedded in a larger file and carry a version without one. | Bitmap block | *embedded* | — | 1 | | Variant metadata | *embedded* | — | 1 | -`CURRENT` and `LOCK` carry no version. `CURRENT` holds one manifest name and nothing else; `LOCK` holds -nothing at all. +`CURRENT` and `LOCK` carry no version. `CURRENT` holds one manifest name and nothing else. `LOCK` held +nothing at all until 0.3.0 and now carries one line of ASCII naming the process that holds it — +`pid=… startedAt=…` — which is a **diagnostic and not a format**: nothing reads it except a process +that has just failed to take the lock, an empty one reads as "holder unknown", and no guarantee in +this document covers it. The lock itself is on byte zero and the record begins after it, so a holder +can be read while it is being held; a build that locks the whole file and a build that locks byte +zero still exclude each other, which is what keeps releases mixable on one directory. The magics are spelled `JKDB-` because the project was called `jsonkdb` when the format was written. A magic is a discriminator saying which kind of file this is, never branding, so the prefix is diff --git a/INTEGRATION.md b/INTEGRATION.md index d4734d7..5da8bcd 100644 --- a/INTEGRATION.md +++ b/INTEGRATION.md @@ -63,14 +63,37 @@ space. A second attempt raises `StoreLockedException`, distinguishably in each direction — `… is already open in this process` when it is your own code, `… is locked by another process` when it is not. Held -by `DocumentStoreTest`'s *a second store cannot open the same directory*. +by `DocumentStoreTest`'s *a second store cannot open the same directory* and by `StoreLockTest`, +which takes the lock from a second **JVM** because that is the only way to reach the branch a real +second launch takes. **For a desktop or CLI application this is the normal second-launch case, not a fault.** Catch `StoreLockedException` specifically — it is a distinct subtype of the sealed `StoreException` — and -focus the existing window, or exit with a message. Do not catch `Exception` and treat it as -corruption, and do not delete the lock file: it is a real advisory lock held by an open channel, and -removing it produces the two-writer state it exists to prevent. There is no force-open and there will -not be one. +read the details off the exception rather than out of its message: + +```kotlin +val db = try { + Rabosh.open(directory) +} catch (locked: StoreLockedException) { + val who = locked.holder + when { + who != null && who.isRunning -> focusExistingWindow(who.pid) + else -> reportAlreadyOpen(locked.directory) // holder unknown, or the record is stale + } + return +} +``` + +`holder` is `null` when nothing could be read — a store last opened by a release before 0.3.0 wrote +no record — and `LockHolder.isRunning` is false when the recorded process is gone, which means the +record is stale rather than that the lock is free. **`isRunning` checks the start time as well as the +pid**, deliberately: operating systems reuse process ids, and a user who is told to kill pid 4242 on +the strength of a pid alone can kill a stranger's process. + +Do not catch `Exception` and treat it as corruption, and **do not delete the lock file**: it is a +real advisory lock held by an open channel, and removing it produces the two-writer state it exists +to prevent. There is no force-open, no timeout and no lock stealing, and there will not be — each of +those converts a clear failure into a corrupt store. **Within the process, one writing thread.** Any number of threads may read concurrently — snapshots, scans and queries are all safe — and `Rabosh` guards the cached planner statistics behind `query` @@ -141,19 +164,57 @@ asked for the load. Do not report success before the `sync()`. ## Taking a copy of a store -**There is no `checkpoint` yet.** Until there is, the only defined way to copy a store is: +```kotlin +val info = db.checkpoint(Path.of("backup", "2026-08-10")) +// info.sequence — every commit at or below it is in the copy, nothing above it is +Rabosh.open(info.directory).use { copy -> /* a database, indexes and model included */ } +``` + +**Safe to call while you are writing**, which is the point: an application that is itself the writer +cannot stop to be copied. The database is flushed, a snapshot is pinned, and the copy is of what that +snapshot sees. Commits that land during the call are above `info.sequence` and are simply not in it. + +Everything travels — segments, their `.cat`, `.idx`, `.pst` and `.col` sidecars, and the `INDEXES` +registry — and the sidecars are **read** by the copy rather than rebuilt, so an index the original +paid a scan for is not paid for twice. + +Three things to know before you rely on it: + +- **The target must be empty or absent.** A checkpoint is never merged into a directory that already + holds a store; that would open, and be wrong. +- **It is a consistent view, not an off-site backup.** Files are hard-linked where the filesystem + allows it (`info.hardLinked` says), so the copy costs a directory entry per file rather than its + bytes — and shares blocks with the original. Moving it somewhere else is what makes it a backup, + and that step is yours. +- **A failure leaves a partial target and never touches the source.** There is no unwind, because the + directory is not a store until `CURRENT` names its manifest; throw it away and take another. + +`DocumentStore.checkpoint` is the same thing one layer down, and carries everything except the index +registry — which is `IndexCatalog`'s file, and is why `Rabosh.checkpoint` exists rather than the +facade just delegating. + +## Retention + +```kotlin +val retired = db.deleteRange(to = Key.of("event:2026-07-31")) +db.compact() +``` + +Both bounds are inclusive and both are optional. This is the loop you would otherwise write, and +writing it correctly means knowing four things that are not on any signature — that the scan must be +scoped by a snapshot, that the deletes belong in a batch, that a tombstone is reclaimed by compaction +rather than by the delete, and that a tombstone may only be dropped at the bottom-most level below the +oldest live snapshot. -1. Stop writing. Not "pause the ingest thread" — no `put`, `delete` or `write` may be in flight. -2. `db.flush()`, which returns when the memtable is on the platter and the manifest names it. -3. Copy the whole directory, including `CURRENT`, `MANIFEST-*`, every `.wal`, `.seg`, `.cat`, `.idx`, - `.pst` and `.col`. Not a subset: the manifest names the files it expects. -4. Resume writing. +**`deleteRange` writes one tombstone per key**, deliberately: it is point deletes rather than an LSM +range tombstone, so it costs nothing in the format and its cost is proportional to the number of keys +deleted. **Follow it with `compact()`** when the space matters — the tombstones make the keys +disappear, and compaction makes the tombstones disappear. -**A directory copied while a writer is running is not defined to be recoverable**, and a copy that -skips the log or the manifest is not a store. Neither failure is loud — the copy usually opens and is -usually missing something. +It is atomic per batch rather than overall, and the range is emptied as of the moment you called it, +so keys written during the call survive and a repeated call converges instead of racing a writer. -`LOCK` may be copied or not; it holds nothing. +`:rabosh-samples:runDrain` is this and `checkpoint` in the order a staging buffer actually uses them. ## Version pinning diff --git a/README.md b/README.md index 59d290d..f4858fd 100644 --- a/README.md +++ b/README.md @@ -577,12 +577,13 @@ front end to the planner by accident, because the build would have to acquire th ## Samples -Two runnable programs, in `rabosh-samples`. Neither is published and neither depends on anything +Three runnable programs, in `rabosh-samples`. None is published and none depends on anything but `rabosh-api`. ```sh ./gradlew :rabosh-samples:runThreeSteps # write blind -> model later -> index later, narrated ./gradlew :rabosh-samples:runIndexLater # a background build, queried while it is half finished +./gradlew :rabosh-samples:runDrain # a staging buffer drained, checkpointed and retired ``` `runThreeSteps` is the README's opening snippet with the evidence attached: it runs one query before @@ -599,6 +600,16 @@ timing, and queries from there: some segments answered from sidecars, the rest s because a cancelled build and a running one leave the same thing behind — and the second pass builds exactly the segments the first did not. +`runDrain` is the one that is pure integration, and it exists because every mistake in it is silent. +A staging buffer holds events until something downstream has taken them, and the loop that hands them +over is five calls in one order: pin a snapshot, scan from the watermark, ship, *then* record the +watermark, then `deleteRange` what was shipped and `compact`. A watermark advanced before the ship +succeeds loses data; a scan without a snapshot can see a compaction land underneath it; a drain that +never compacts grows for ever while reporting that it deleted everything. It also takes a +`checkpoint` **while still writing**, opens the copy, and shows that it holds the prefix as of its +sequence and nothing after it. Deliberately not a `DrainCursor` — the value is the order, which a +wrapper would hide. + Both are executed by `SamplesTest` on every `./gradlew build`, and what it asserts is their *output*: a sample that ran to completion and printed `0 rows` has failed at the only job it has. @@ -645,6 +656,7 @@ documentation that nothing executes rots: ```sh ./gradlew :rabosh-samples:runThreeSteps # the three steps, narrated, with the counters ./gradlew :rabosh-samples:runIndexLater # a background index build, stopped and resumed +./gradlew :rabosh-samples:runDrain # drain, checkpoint and retention, in the order they go ``` A benchmark task **fails if it produced no results** — JMH can decline to start and still exit zero, diff --git a/STABILITY.md b/STABILITY.md index cb33dba..f3aedf4 100644 --- a/STABILITY.md +++ b/STABILITY.md @@ -20,19 +20,21 @@ wrap all of the API or none of it. Two tiers cost nothing and say what the evide Small on purpose. It is the surface the README's examples call, the surface the two samples call, and it has not moved in two releases. -**`rabosh-api`** — `Rabosh` (`open`, `close`, `put`, `get`, `delete`, `write`, `scan`, `snapshot`, -`query`, `keys`, `explain`, `createIndex`, `createIndexInBackground`, `buildIndexesInBackground`, -`dropIndex`, `indexes`, `schema`, `indexCandidates`, `attach`, `flush`, `sync`, `rotate`, `compact`, -`stats`, `directory`, `options`) and `RaboshOptions`. +**`rabosh-api`** — `Rabosh` (`open`, `close`, `put`, `get`, `delete`, `deleteRange`, `write`, `scan`, +`snapshot`, `query`, `keys`, `explain`, `createIndex`, `createIndexInBackground`, +`buildIndexesInBackground`, `dropIndex`, `indexes`, `schema`, `indexCandidates`, `attach`, +`checkpoint`, `flush`, `sync`, `rotate`, `compact`, `stats`, `directory`, `options`) and +`RaboshOptions`. -**`rabosh-core`** — `Key`, `WriteBatch`, `Durability`, `Snapshot`, `DocumentCursor`, `StoreOptions`, -`StoreStats`, `LogRecoveryMode`, `SegmentObserver`, `SegmentObservation`, `SegmentSummary`, and the -whole `StoreException` hierarchy. +**`rabosh-core`** — `Key` (including `successor`), `WriteBatch`, `Durability`, `Snapshot`, +`DocumentCursor`, `StoreOptions`, `StoreStats`, `LogRecoveryMode`, `SegmentObserver`, +`SegmentObservation`, `SegmentSummary`, `CheckpointInfo`, `LockHolder`, and the whole +`StoreException` hierarchy. **`rabosh-query`** — `Query`, `Predicate` and its cases, the predicate DSL (`path`, `and`, `or`, `not`, `eq`, `anyOf`, `exists`, `isNull`, `elemMatch`, the comparison operators), `PathRef`, `Comparison`, `QueryValue`, `Projection`, `Row`, `QueryCursor`, `QueryStats`, `Explain`, -`ExplainSource`, `IndexUse`. +`ExplainSource`, `ExplainTypeNote`, `IndexUse`. **`rabosh-index`** — `IndexDefinition`, `IndexHandle`, `IndexBuild`, `IndexBuildProgress`, `IndexBuildState`, `IndexCoverage`, `IndexOptions`, `DamagedIndexPolicy`, `CompositeSegmentObserver`, @@ -40,15 +42,19 @@ and the whole `IndexException` hierarchy. **`rabosh-catalog`** — `CatalogPath`, `CatalogStep` and the node walk, `InferredSchema`, `InferredField` (except its `sketch`), `CatalogCoverage`, `IndexCandidate`, `IndexCandidateOptions`, -`IndexKind`, `ValueBounds`, `NumericRange`, `TextRange`, `CatalogOptions`, `DamagedSketchPolicy`, and -the whole `CatalogException` hierarchy. +`IndexKind`, `ValueBounds`, `NumericRange`, `TextRange`, `CatalogOptions`, `DamagedSketchPolicy`, +`ShreddingAdvice` and `InferredSchema.shreddingAdvice`, and the whole `CatalogException` hierarchy. -**`rabosh-variant`** — `Variant` and its readers, `VariantNode`, `VariantPath`, `VariantPathStep`, +**`rabosh-variant`** — `Variant` and its readers including `detached`, `VariantNode`, `VariantPath`, +`VariantPathStep`, `VariantKind`, `VariantBasicType`, `VariantPrimitiveType`, `VariantBuilder`, `VariantMetadata`, `DuplicateFieldPolicy`, `toJsonString` / `toJsonSummaryString`, and the whole `VariantException` hierarchy. -**`rabosh-jsonpath`** — `JsonPathQuery`. +**`rabosh-jsonpath`** — `JsonPathQuery`, `JsonPathLimits`, `JsonPathLimit`, +`JsonPathLimitExceededException`. The limits are stable core rather than experimental because the +module's chosen use case is evaluating expressions you did not write, and a bound a caller cannot +rely on is not a bound. ### Two entries that are in the list for a reason worth knowing diff --git a/rabosh-api/api/rabosh-api.api b/rabosh-api/api/rabosh-api.api index 2a6e975..a9fc6f4 100644 --- a/rabosh-api/api/rabosh-api.api +++ b/rabosh-api/api/rabosh-api.api @@ -2,11 +2,16 @@ public final class app/oreshkov/rabosh/api/Rabosh : java/lang/AutoCloseable { public static final field Companion Lapp/oreshkov/rabosh/api/Rabosh$Companion; public final fun attach ()V public final fun buildIndexesInBackground ()Lapp/oreshkov/rabosh/index/IndexBuild; + public final fun checkpoint (Ljava/nio/file/Path;)Lapp/oreshkov/rabosh/core/CheckpointInfo; public fun close ()V public final fun compact ()V public final fun createIndex (Lapp/oreshkov/rabosh/index/IndexDefinition;)Lapp/oreshkov/rabosh/index/IndexHandle; public final fun createIndexInBackground (Lapp/oreshkov/rabosh/index/IndexDefinition;)Lapp/oreshkov/rabosh/index/IndexBuild; public final fun delete (Lapp/oreshkov/rabosh/core/Key;)V + public final fun deleteRange ()J + public final fun deleteRange (Lapp/oreshkov/rabosh/core/Key;)J + public final fun deleteRange (Lapp/oreshkov/rabosh/core/Key;Lapp/oreshkov/rabosh/core/Key;)J + public static synthetic fun deleteRange$default (Lapp/oreshkov/rabosh/api/Rabosh;Lapp/oreshkov/rabosh/core/Key;Lapp/oreshkov/rabosh/core/Key;ILjava/lang/Object;)J public final fun dropIndex (Lapp/oreshkov/rabosh/index/IndexHandle;)V public final fun explain (Lapp/oreshkov/rabosh/query/Query;)Lapp/oreshkov/rabosh/query/Explain; public final fun explain (Lapp/oreshkov/rabosh/query/Query;Lapp/oreshkov/rabosh/core/Snapshot;)Lapp/oreshkov/rabosh/query/Explain; diff --git a/rabosh-api/src/main/kotlin/app/oreshkov/rabosh/api/Rabosh.kt b/rabosh-api/src/main/kotlin/app/oreshkov/rabosh/api/Rabosh.kt index 1336bde..ba692d6 100644 --- a/rabosh-api/src/main/kotlin/app/oreshkov/rabosh/api/Rabosh.kt +++ b/rabosh-api/src/main/kotlin/app/oreshkov/rabosh/api/Rabosh.kt @@ -5,6 +5,7 @@ import app.oreshkov.rabosh.catalog.IndexCandidate import app.oreshkov.rabosh.catalog.IndexCandidateOptions import app.oreshkov.rabosh.catalog.InferredSchema import app.oreshkov.rabosh.catalog.SchemaCatalog +import app.oreshkov.rabosh.core.CheckpointInfo import app.oreshkov.rabosh.core.DocumentCursor import app.oreshkov.rabosh.core.DocumentStore import app.oreshkov.rabosh.core.Key @@ -147,6 +148,28 @@ public class Rabosh private constructor( /** Commits a deletion of [key]. Deleting an absent key is legal and writes a tombstone. */ public fun delete(key: Key): Unit = store.delete(key) + /** + * Deletes every key in `[from, to]`, both bounds inclusive and both optional, and returns how + * many. + * + * ```kotlin + * val retired = db.deleteRange(to = Key.of("event:2026-07-31")) + * db.compact() // tombstones are reclaimed by compaction, not by this call + * ``` + * + * Retention by key range, for a store that keys by time — which is what a staging buffer and a + * payload archive both do, and the whole of their retention policy. It is the loop a caller would + * otherwise write, and writing it correctly means knowing four invariants of the layer below; + * see [DocumentStore.deleteRange], which this delegates to unchanged. + * + * **Deliberately point deletes rather than a range tombstone**, so it costs one tombstone per key + * and nothing at all in the format. Follow it with [compact] when the space matters: the + * tombstones are what make the keys disappear, and compaction is what makes the tombstones + * disappear. + */ + @JvmOverloads + public fun deleteRange(from: Key? = null, to: Key? = null): Long = store.deleteRange(from, to) + /** * Commits [batch] as one record, atomically and as one view. * @@ -381,6 +404,47 @@ public class Rabosh private constructor( invalidateStatistics() } + /** + * Writes a consistent copy of this database into [target], which must be empty or absent. + * + * ```kotlin + * val info = db.checkpoint(Path.of("backup", "2026-08-10")) + * Rabosh.open(info.directory).use { copy -> /* every commit up to info.sequence, indexes and all */ } + * ``` + * + * **Safe to call while the application is writing**, which is the whole reason it exists: the + * recipe it replaces is *stop writing and copy the directory*, and a desktop application cannot + * stop writing because it is the writer. The database is flushed, a snapshot is pinned, and the + * copy is of what that snapshot sees — so the result holds exactly the acknowledged prefix as of + * [CheckpointInfo.sequence]. Anything committed during the call is above that sequence and is + * simply not in the copy. + * + * **Everything travels, and the sidecars are read rather than rebuilt.** Segments carry their + * `.cat`, `.idx`, `.pst` and `.col` files with them, and this method adds the one file + * [DocumentStore.checkpoint] cannot know about — `INDEXES`, the index registry, which is a + * *definition* an operator gave rather than derived data. Losing it would leave the copy silently + * without an index somebody created, so it is copied here and the layer that owns it does the + * writing. + * + * **A checkpoint, not a backup tool.** No scheduling, no retention, no incremental mode, no + * compression. And the files are hard-linked where the filesystem allows it — see + * [CheckpointInfo.hardLinked] — so the result shares its blocks with the source: taking it + * somewhere else is what turns a consistent view into a backup, and that step is yours. + * + * @throws java.nio.file.FileAlreadyExistsException if [target] exists and is not an empty + * directory. + * @throws IllegalStateException if this database is closed. + */ + public fun checkpoint(target: Path): CheckpointInfo { + checkOpen() + val info = store.checkpoint(target) + // After the store's own copy, because the registry names indexes whose posting files have to + // be there already — the ordering rule applied to the layer above it, for the same reason a + // manifest is forced after the segments it names. + indexCatalog?.copyRegistryTo(target) + return info + } + /** Flushes, then compacts until no level is over its budget. Returns when the tree is in shape. */ public fun compact() { store.compact() diff --git a/rabosh-api/src/test/kotlin/app/oreshkov/rabosh/api/LakehouseHandoffTest.kt b/rabosh-api/src/test/kotlin/app/oreshkov/rabosh/api/LakehouseHandoffTest.kt new file mode 100644 index 0000000..b3e18d9 --- /dev/null +++ b/rabosh-api/src/test/kotlin/app/oreshkov/rabosh/api/LakehouseHandoffTest.kt @@ -0,0 +1,207 @@ +package app.oreshkov.rabosh.api + +import app.oreshkov.rabosh.catalog.shreddingAdvice +import app.oreshkov.rabosh.query.Projection +import app.oreshkov.rabosh.query.Query +import app.oreshkov.rabosh.query.path +import app.oreshkov.rabosh.variant.Variant +import app.oreshkov.rabosh.variant.VariantMetadata +import app.oreshkov.rabosh.variant.toJsonString +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals +import kotlin.test.assertTrue +import org.junit.jupiter.api.io.TempDir + +/** + * Getting bytes out, for a caller writing them into a lakehouse. + * + * **The claim under test is that a detached document's `(metadata, value)` pair stands alone**, and + * the way it is checked is what makes it worth anything: the pair is taken apart and read back + * through an *independent decode* — `VariantMetadata` and `Variant` reconstructed from the two byte + * arrays with nothing of the store behind them — rather than by asking the object that produced it + * whether it is happy. A round trip through the same reader would pass for a document still pointing + * at a segment's dictionary, which is precisely the trap. + * + * No Parquet writer is involved and none is a dependency. What this pins is that the bytes are + * self-contained and that the field names survive, which is the part the engine owes a caller. + */ +class LakehouseHandoffTest { + + @TempDir + lateinit var root: Path + + private fun options() = RaboshOptions(store = apiStoreOptions()) + + /** + * The trap, demonstrated before the fix: a document read from a segment carries **that + * segment's** dictionary. + * + * Asserted as an inequality rather than described, because it is the reason `detached` exists and + * a reader meeting only the fixed version would reasonably wonder why. + */ + @Test + fun `a document read from a segment carries the segment's shared dictionary`() { + val directory = scratch(root, "shared") + Rabosh.open(directory, options()).use { db -> + // Each document names a field of its own, so the segment's dictionary grows with the + // corpus while no single document's does. The main corpus cannot show this — every + // document there carries the same six names, which is exactly the *homogeneous* shape + // one dictionary per segment is optimised for and therefore the shape where the trap is + // invisible. + for (index in 0 until 100) { + db.put(keyFor(index), """{"common":$index,"only_in_$index":true}""") + } + db.flush() + + // Document 50, not document 0, and the reason is the whole hazard in miniature. A + // dictionary is name-ordered, so document 0's two names — `common`, `only_in_0` — happen + // to take ids 0 and 1 in the *shared* dictionary as well as in its own, and pairing its + // value with the wrong dictionary reads back perfectly. The trap is not that a mismatched + // pair always fails; it is that it sometimes succeeds. + val attached = db.get(keyFor(50))!! + val detached = attached.detached() + + assertEquals(2, detached.metadata.size, "the document names two fields") + assertTrue( + attached.metadata.size > 50, + "the segment's dictionary names every document's fields: ${attached.metadata.size}", + ) + assertNotEquals( + attached.metadata.toByteArray().size, + detached.metadata.toByteArray().size, + "if these matched, the fixture would not be demonstrating anything", + ) + assertEquals(attached.toJsonString(), detached.toJsonString(), "and the value is unchanged") + + // The failure this prevents, made concrete: the value's field ids index into whichever + // dictionary it is paired with, so pairing the detached value with the shared metadata + // resolves the wrong names — or none at all. + val mismatched = runCatching { + Variant(attached.metadata, detached.toByteArray()).toJsonString() + }.getOrNull() + assertNotEquals( + attached.toJsonString(), + mismatched, + "a value and a dictionary that do not belong together must not read as the document", + ) + } + } + + /** + * The acceptance criterion: write the pair out, read it back through an independent decode. + * + * Nothing of the store is in scope on the way back — two byte arrays go in and a `Variant` comes + * out — which is the situation a Parquet Variant column puts the bytes in. + */ + @Test + fun `a detached pair decodes on its own`() { + withDatabase { db -> + db.query(Query.where(path("$.team") eq "team-3").project(Projection.DOCUMENT)).use { rows -> + var checked = 0 + while (rows.next()) { + val expected = rows.row.document().toJsonString() + val detached = rows.row.document().detached() + + // The hand-off itself: two arrays, and nothing else crosses the boundary. + val metadataBytes = detached.metadata.toByteArray() + val valueBytes = detached.toByteArray() + + val reread = Variant(VariantMetadata.of(metadataBytes), valueBytes) + assertEquals(expected, reread.toJsonString(), "the pair must stand alone") + assertEquals("team-3", reread.field("team")?.stringValue(), "field names survived") + checked++ + } + assertTrue(checked > 0, "the query must return something, or nothing above ran") + } + } + } + + /** + * The honest alternative, which is cheaper and is what a consumer taking a shared dictionary + * should be handed. + * + * Stated as a test so the pairing is on record: `detached` is not always the right answer, and a + * caller handing over one dictionary per *segment* copies no names at all. + */ + @Test + fun `an undetached pair decodes when the shared metadata travels with it`() { + withDatabase { db -> + val document = db.get(keyFor(1))!! + val shared = document.metadata.toByteArray() + + val reread = Variant(VariantMetadata.of(shared), document.toByteArray()) + assertEquals(document.toJsonString(), reread.toJsonString()) + } + } + + // --- the advice ------------------------------------------------------------------------------ + + /** + * Shredding advice over what the catalog already computed. + * + * The corpus's `team` is a stable string carrying a real share of the bytes, so it must be + * advised; `$.tags` is an array and cannot be one column, so it must not be. Both directions, + * because a recommender that recommends everything is not a recommender. + */ + @Test + fun `shredding advice names the stable scalar paths and not the containers`() { + withDatabase { db -> + val advice = db.schema().shreddingAdvice() + assertTrue(advice.isNotEmpty(), "the corpus has stable scalar paths") + + val paths = advice.map { it.path.toString() } + assertTrue(paths.contains("$.team"), paths.toString()) + assertTrue(!paths.contains("$.tags"), "an array is not one typed column: $paths") + assertTrue(!paths.contains("$"), "the root is not a shreddable leaf: $paths") + + val team = advice.single { it.path.toString() == "$.team" } + assertEquals("BINARY (UTF8)", team.parquetType) + assertTrue(team.presence > 0.99, team.toString()) + assertTrue(team.byteShare > 0.0, team.toString()) + assertTrue(team.render().contains("typed_value"), team.render()) + } + } + + /** + * The one decision a hand-written schema gets wrong: whether `variant_value` can be dropped. + * + * A path holding two types needs the untyped fallback populated, and the advice has to say so — + * dropping it there loses every value of the minority type, silently, which is the same class of + * failure as the type bracketing `explain` now reports. + */ + @Test + fun `a path with a residual type is told to keep variant_value`() { + val directory = scratch(root, "residual") + Rabosh.open(directory, options()).use { db -> + for (index in 0 until 200) { + val status = if (index % 10 == 0) """"$index"""" else "$index" + db.put(keyFor(index), """{"status":$status,"filler":"$index-$index-$index-$index"}""") + } + db.flush() + + val status = db.schema().shreddingAdvice().singleOrNull { it.path.toString() == "$.status" } + assertTrue(status != null, "a 90%-stable path is still worth shredding") + assertTrue(status.residual, "one value in ten is a string: ${status.render()}") + assertTrue(status.render().contains("variant_value: required"), status.render()) + assertTrue(status.reason.contains("string"), status.reason) + } + } + + /** A model of nothing advises nothing, rather than dividing by zero. */ + @Test + fun `an empty model advises nothing`() { + val directory = scratch(root, "empty") + Rabosh.open(directory, options()).use { db -> + assertEquals(emptyList(), db.schema().shreddingAdvice()) + } + } + + private fun withDatabase(body: (Rabosh) -> Unit) { + Rabosh.open(scratch(root, "handoff"), options()).use { db -> + db.load(0, 300) + body(db) + } + } +} diff --git a/rabosh-api/src/test/kotlin/app/oreshkov/rabosh/api/RaboshCheckpointTest.kt b/rabosh-api/src/test/kotlin/app/oreshkov/rabosh/api/RaboshCheckpointTest.kt new file mode 100644 index 0000000..927cb8a --- /dev/null +++ b/rabosh-api/src/test/kotlin/app/oreshkov/rabosh/api/RaboshCheckpointTest.kt @@ -0,0 +1,142 @@ +package app.oreshkov.rabosh.api + +import app.oreshkov.rabosh.index.IndexDefinition +import app.oreshkov.rabosh.query.Projection +import app.oreshkov.rabosh.query.Query +import app.oreshkov.rabosh.query.path +import java.nio.file.Files +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import org.junit.jupiter.api.io.TempDir + +/** + * A checkpoint through the facade: the whole database, not just the store. + * + * `CheckpointTest` in `rabosh-core` asserts the documents. What can only be asserted here is that the + * *derived data and the definitions* travel too — a copy that opened with the right documents and no + * indexes would pass every assertion there and would still have lost the thing a caller spent a scan + * building. + * + * **The sidecars must be read rather than rebuilt**, which is the same assertion `FormatCompatibilityTest` + * makes of the golden stores and for the same reason: a copy opened with `backfill = true` would + * regenerate whatever was missing and every one of these tests would pass over a checkpoint that had + * carried nothing at all. + */ +class RaboshCheckpointTest { + + @TempDir + lateinit var root: Path + + private fun options(backfill: Boolean = true) = RaboshOptions( + store = apiStoreOptions(), + backfill = backfill, + ) + + /** + * The load-bearing one: open the copy with backfilling **off**, and the index still answers. + * + * With `backfill = false` nothing is scanned and nothing is rebuilt, so an index that answers is + * an index whose `.idx` and `.pst` files were carried and decoded — and a registry that was + * carried with them, because a posting file nothing knows about is an orphan. + */ + @Test + fun `a checkpoint's indexes are read rather than rebuilt`() { + val directory = scratch(root) + val target = root.resolve("checkpoint-indexed") + + val expected = Rabosh.open(directory, options()).use { db -> + db.load(0, 400) + db.flush() + db.createIndex(IndexDefinition.inverted("$.team")) + + val before = db.keys(teamQuery()) + assertTrue(before.isNotEmpty(), "the fixture must match something, or nothing below is a claim") + db.checkpoint(target) + before + } + + Rabosh.open(target, options(backfill = false)).use { copy -> + assertEquals(1, copy.indexes().size, "the registry travelled: the definition is not derived data") + + val explained = copy.explain(teamQuery()) + assertTrue(explained.usesIndexes, "the copy answered from sidecars:\n${explained.render()}") + assertEquals(0, explained.segmentsScanned, "a scan here would mean the sidecars were not read") + + assertEquals(expected, copy.keys(teamQuery()), "the same keys, from the copy's own files") + } + } + + /** The model travels too, and is likewise not recollected. */ + @Test + fun `a checkpoint carries the schema catalog`() { + val directory = scratch(root) + val target = root.resolve("checkpoint-modelled") + + val expected = Rabosh.open(directory, options()).use { db -> + db.load(0, 200) + db.flush() + val schema = db.schema() + assertTrue(schema.fields.isNotEmpty()) + db.checkpoint(target) + schema.fields.map { it.path.toString() }.sorted() + } + + Rabosh.open(target, options(backfill = false)).use { copy -> + val schema = copy.schema() + assertEquals(expected, schema.fields.map { it.path.toString() }.sorted()) + assertTrue( + schema.coverage.isComplete, + "every segment carried its own `.cat`, so the model is complete without a scan: ${schema.coverage}", + ) + } + } + + /** + * The copy is a database, not a snapshot of one: it opens for writing and carries on. + * + * The point case B needs — a checkpoint that could only be read would be an export, and the + * thing an application wants after losing its data directory is to keep working. + */ + @Test + fun `a checkpoint opens as a writable database`() { + val directory = scratch(root) + val target = root.resolve("checkpoint-writable") + + Rabosh.open(directory, options()).use { db -> + db.load(0, 100) + db.checkpoint(target) + } + + Rabosh.open(target, options()).use { copy -> + copy.load(100, 50) + copy.flush() + assertEquals(documentOf(120).toString(), copy.get(keyFor(120)).toString()) + assertEquals(documentOf(0).toString(), copy.get(keyFor(0)).toString()) + } + } + + /** A database with no index defined writes no registry, and the copy is not left with an empty one. */ + @Test + fun `a checkpoint of an unindexed database carries no registry`() { + val directory = scratch(root) + val target = root.resolve("checkpoint-plain") + + Rabosh.open(directory, options()).use { db -> + db.load(0, 50) + db.checkpoint(target) + } + + assertTrue( + Files.notExists(target.resolve("INDEXES")), + "a store that never defined an index has no registry, and a copy must not invent one", + ) + Rabosh.open(target, options(backfill = false)).use { copy -> + assertEquals(0, copy.indexes().size) + } + } + + /** `team-3` is one of the seven the corpus cycles through, so it matches a seventh of it. */ + private fun teamQuery(): Query = Query.where(path("$.team") eq "team-3").project(Projection.KEY) +} diff --git a/rabosh-catalog/api/rabosh-catalog.api b/rabosh-catalog/api/rabosh-catalog.api index a23e1af..363a280 100644 --- a/rabosh-catalog/api/rabosh-catalog.api +++ b/rabosh-catalog/api/rabosh-catalog.api @@ -254,6 +254,25 @@ public final class app/oreshkov/rabosh/catalog/SegmentSketch$Companion { public final fun getEMPTY ()Lapp/oreshkov/rabosh/catalog/SegmentSketch; } +public final class app/oreshkov/rabosh/catalog/ShreddingAdvice { + public final fun getByteShare ()D + public final fun getDistinctEstimate ()J + public final fun getNullFraction ()D + public final fun getParquetType ()Ljava/lang/String; + public final fun getPath ()Lapp/oreshkov/rabosh/catalog/CatalogPath; + public final fun getPresence ()D + public final fun getReason ()Ljava/lang/String; + public final fun getResidual ()Z + public final fun getTypeStability ()D + public final fun render ()Ljava/lang/String; + public fun toString ()Ljava/lang/String; +} + +public final class app/oreshkov/rabosh/catalog/ShreddingAdviceKt { + public static final fun shreddingAdvice (Lapp/oreshkov/rabosh/catalog/InferredSchema;Lapp/oreshkov/rabosh/catalog/IndexCandidateOptions;)Ljava/util/List; + public static synthetic fun shreddingAdvice$default (Lapp/oreshkov/rabosh/catalog/InferredSchema;Lapp/oreshkov/rabosh/catalog/IndexCandidateOptions;ILjava/lang/Object;)Ljava/util/List; +} + public final class app/oreshkov/rabosh/catalog/TextRange { public fun equals (Ljava/lang/Object;)Z public final fun getMax ()Ljava/lang/String; diff --git a/rabosh-catalog/src/main/kotlin/app/oreshkov/rabosh/catalog/ShreddingAdvice.kt b/rabosh-catalog/src/main/kotlin/app/oreshkov/rabosh/catalog/ShreddingAdvice.kt new file mode 100644 index 0000000..5096606 --- /dev/null +++ b/rabosh-catalog/src/main/kotlin/app/oreshkov/rabosh/catalog/ShreddingAdvice.kt @@ -0,0 +1,162 @@ +package app.oreshkov.rabosh.catalog + +import app.oreshkov.rabosh.variant.VariantKind + +/** + * One path worth promoting to a typed column in a Parquet **Variant shredding schema**, with the + * evidence behind the recommendation. + * + * @property path the path, with array indices collapsed. See [CatalogPath]. + * @property parquetType the Parquet physical type the observations point at — `INT64`, `DOUBLE`, + * `DECIMAL(p, s)`, `BINARY (UTF8)`, `BOOLEAN`. Named in Parquet's vocabulary rather than the + * engine's, because the whole point of this object is to be read by somebody writing a schema. + * @property presence how much of the corpus carries the path, in `0.0..1.0`. A shredded field that + * is usually absent costs a definition level per row and buys little. + * @property typeStability how much of the path's observed values are of [parquetType]'s family. + * Below 1.0 the residual values still have to go somewhere — see [residual]. + * @property nullFraction how much of the *present* values are JSON null. + * @property distinctEstimate distinct values, exact below the sketch's sparse limit and an estimate + * above it. See [InferredField.distinctIsExact]. + * @property byteShare how much of the stored document bytes this path accounts for. The number that + * decides whether shredding it is worth anything: a path holding 0.1% of the bytes cannot save a + * reader much however typed it is. + * @property residual `true` when the path holds values outside [parquetType]'s family, so the + * shredded column needs its untyped `variant_value` fallback populated rather than being a pure + * typed column. This is the field a hand-written schema gets wrong. + * @property reason the recommendation in words, for a human reading a report. + */ +public class ShreddingAdvice internal constructor( + public val path: CatalogPath, + public val parquetType: String, + public val presence: Double, + public val typeStability: Double, + public val nullFraction: Double, + public val distinctEstimate: Long, + public val byteShare: Double, + public val residual: Boolean, + public val reason: String, +) { + /** + * The field as a line of a shredding schema, in the specification's own shape. + * + * Deliberately **not** a Parquet schema file: this project writes no Parquet and takes no + * dependency on one, so emitting something that looked like a complete schema would be a claim + * about a format it does not own. What this is, is the one line a reader needs to transcribe, + * with the decision that is easy to get wrong — whether `variant_value` can be dropped — + * already made. + */ + public fun render(): String = buildString { + append(path.toString()) + append(": { typed_value: ") + append(parquetType) + append(if (residual) ", variant_value: required }" else ", variant_value: omitted }") + append(" — ") + append(reason) + } + + override fun toString(): String = render() +} + +/** + * Which paths are worth shredding into typed columns, best first. + * + * **The same statistics as [SchemaCatalog.indexCandidates], rendered for a different decision.** + * `IndexCandidate` with `IndexKind.SHREDDED_COLUMN` already scores this question for the engine's own + * columns; what was missing is a rendering aimed at a *Parquet shredding schema* rather than at an + * index. The two differ in what they emphasise — an index cares about selectivity, a shredding + * schema cares about type stability and byte share — and in one thing an index never has to say: + * whether the typed column can stand alone or needs its `variant_value` fallback. + * + * **This emits advice and bytes, and never Parquet.** Writing the file is the caller's, with the + * caller's own writer; the engine's claim of zero runtime dependencies is not spent on it. The other + * half of the hand-off is `Variant.detached()`, which produces the self-contained + * `(metadata, value)` pair a Variant column wants. + * + * The published shredding measurements put the read gain at around **8×** over unshredded Variant, + * which is why this is worth generating even for a caller who will shred by hand. + * + * @param options the same thresholds the index recommendations use, so a path this declines and a + * path `indexCandidates` declines are declined for reasons a reader can compare. + */ +public fun InferredSchema.shreddingAdvice( + options: IndexCandidateOptions = IndexCandidateOptions.DEFAULT, +): List { + // Once, not per field: the denominator is a property of the model rather than of any path, and + // recomputing it inside the loop would make this quadratic in the number of paths for an answer + // that cannot change. + val totalBytes = fields.sumOf { it.averageBytes * it.observations } + return fields + .asSequence() + .filter { it.observations >= options.minObservations } + .filter { it.presence >= options.minPresence } + // A container has no typed_value to promote: shredding describes *scalar* leaves, and + // `$.items` is shredded by shredding the paths inside it. Left out rather than reported as + // unsuitable, because "an array is not one column" is a fact about the format, not advice. + .mapNotNull { field -> advise(field, totalBytes, options) } + .sortedByDescending { it.byteShare } + .toList() +} + +private fun advise(field: InferredField, totalBytes: Double, options: IndexCandidateOptions): ShreddingAdvice? { + val dominant = field.dominantType ?: return null + val parquetType = parquetTypeOf(dominant) ?: return null + if (field.typeStability < options.minTypeStability) return null + + // Byte share is the number that decides whether this is worth doing at all, and it is the same + // threshold the engine applies to its own columns — a path carrying almost none of the bytes + // cannot save a reader much however well typed it is. + if (totalBytes <= 0.0) return null + val byteShare = field.averageBytes * field.observations / totalBytes + if (byteShare < options.minColumnByteShare) return null + + val residual = field.types.keys.any { it != dominant && it != VariantKind.NULL } + val reason = buildString { + append("carries ${percent(byteShare)} of the stored bytes as ${percent(field.typeStability)} $dominant") + append(", present in ${percent(field.presence)} of documents") + if (residual) { + val others = field.types.keys + .filter { it != dominant && it != VariantKind.NULL } + .joinToString(", ") { it.name.lowercase() } + append("; keep variant_value for the $others values") + } + if (field.nullFraction > 0.0) append("; ${percent(field.nullFraction)} null") + } + + return ShreddingAdvice( + path = field.path, + parquetType = parquetType, + presence = field.presence, + typeStability = field.typeStability, + nullFraction = field.nullFraction, + distinctEstimate = field.distinctEstimate, + byteShare = byteShare, + residual = residual, + reason = reason, + ) +} + +/** + * The engine's kinds mapped to Parquet's, or `null` for a shape that is not a shreddable leaf. + * + * `ARRAY` and `OBJECT` are `null` because a container has no single typed column; the temporal and + * binary kinds are mapped because the Variant specification gives them Parquet types directly. An + * exhaustive `when` with no `else`, so a kind added later has to be classified here rather than + * silently becoming unshreddable. + */ +private fun parquetTypeOf(kind: VariantKind): String? = when (kind) { + VariantKind.INTEGER -> "INT64" + VariantKind.DECIMAL -> "DECIMAL" + VariantKind.FLOAT -> "FLOAT" + VariantKind.DOUBLE -> "DOUBLE" + VariantKind.BOOLEAN -> "BOOLEAN" + VariantKind.STRING -> "BINARY (UTF8)" + VariantKind.BINARY -> "BINARY" + VariantKind.DATE -> "INT32 (DATE)" + VariantKind.TIME -> "INT64 (TIME(MICROS))" + VariantKind.TIMESTAMP -> "INT64 (TIMESTAMP(MICROS))" + VariantKind.UUID -> "FIXED_LEN_BYTE_ARRAY(16) (UUID)" + // Not leaves, and a null-only path has no type to promote. + VariantKind.ARRAY, VariantKind.OBJECT, VariantKind.NULL -> null +} + +private fun percent(fraction: Double): String = "%.0f%%".format(fraction * 100) diff --git a/rabosh-core/api/rabosh-core.api b/rabosh-core/api/rabosh-core.api index 304ef3e..0ad64f4 100644 --- a/rabosh-core/api/rabosh-core.api +++ b/rabosh-core/api/rabosh-core.api @@ -1,3 +1,13 @@ +public final class app/oreshkov/rabosh/core/CheckpointInfo { + public final fun getBytes ()J + public final fun getDirectory ()Ljava/nio/file/Path; + public final fun getFileCount ()I + public final fun getHardLinked ()Z + public final fun getSegmentCount ()I + public final fun getSequence ()J + public fun toString ()Ljava/lang/String; +} + public final class app/oreshkov/rabosh/core/CorruptLogException : app/oreshkov/rabosh/core/StoreException { public fun (Ljava/lang/String;Ljava/lang/String;JLjava/lang/Throwable;)V public synthetic fun (Ljava/lang/String;Ljava/lang/String;JLjava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V @@ -29,9 +39,15 @@ public final class app/oreshkov/rabosh/core/DocumentCursor : java/lang/AutoClose public final class app/oreshkov/rabosh/core/DocumentStore : java/lang/AutoCloseable { public static final field Companion Lapp/oreshkov/rabosh/core/DocumentStore$Companion; public final fun backfill (Lapp/oreshkov/rabosh/core/SegmentObserver;)V + public final fun checkpoint (Ljava/nio/file/Path;)Lapp/oreshkov/rabosh/core/CheckpointInfo; public fun close ()V public final fun compact ()V public final fun delete (Lapp/oreshkov/rabosh/core/Key;)V + public final fun deleteRange ()J + public final fun deleteRange (Lapp/oreshkov/rabosh/core/Key;)J + public final fun deleteRange (Lapp/oreshkov/rabosh/core/Key;Lapp/oreshkov/rabosh/core/Key;)J + public final fun deleteRange (Lapp/oreshkov/rabosh/core/Key;Lapp/oreshkov/rabosh/core/Key;I)J + public static synthetic fun deleteRange$default (Lapp/oreshkov/rabosh/core/DocumentStore;Lapp/oreshkov/rabosh/core/Key;Lapp/oreshkov/rabosh/core/Key;IILjava/lang/Object;)J public final fun flush ()V public final fun get (Lapp/oreshkov/rabosh/core/Key;)Lapp/oreshkov/rabosh/variant/Variant; public final fun get (Lapp/oreshkov/rabosh/core/Key;Lapp/oreshkov/rabosh/core/Snapshot;)Lapp/oreshkov/rabosh/variant/Variant; @@ -73,6 +89,7 @@ public final class app/oreshkov/rabosh/core/Key : java/lang/Comparable { public final fun get (I)B public final fun getSize ()I public fun hashCode ()I + public final fun successor ()Lapp/oreshkov/rabosh/core/Key; public final fun toByteArray ()[B public fun toString ()Ljava/lang/String; } @@ -82,6 +99,14 @@ public final class app/oreshkov/rabosh/core/Key$Companion { public final fun of ([B)Lapp/oreshkov/rabosh/core/Key; } +public final class app/oreshkov/rabosh/core/LockHolder { + public fun (JLjava/time/Instant;)V + public final fun getPid ()J + public final fun getStartedAt ()Ljava/time/Instant; + public final fun isRunning ()Z + public fun toString ()Ljava/lang/String; +} + public final class app/oreshkov/rabosh/core/LogRecoveryMode : java/lang/Enum { public static final field STRICT Lapp/oreshkov/rabosh/core/LogRecoveryMode; public static final field TOLERATE_TORN_TAIL Lapp/oreshkov/rabosh/core/LogRecoveryMode; @@ -137,6 +162,10 @@ public final class app/oreshkov/rabosh/core/StoreFailedException : app/oreshkov/ public final class app/oreshkov/rabosh/core/StoreLockedException : app/oreshkov/rabosh/core/StoreException { public fun (Ljava/lang/String;Ljava/lang/Throwable;)V public synthetic fun (Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljava/lang/String;Ljava/nio/file/Path;Lapp/oreshkov/rabosh/core/LockHolder;Ljava/lang/Throwable;)V + public synthetic fun (Ljava/lang/String;Ljava/nio/file/Path;Lapp/oreshkov/rabosh/core/LockHolder;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun getDirectory ()Ljava/nio/file/Path; + public final fun getHolder ()Lapp/oreshkov/rabosh/core/LockHolder; } public final class app/oreshkov/rabosh/core/StoreOptions { diff --git a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/Checkpoint.kt b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/Checkpoint.kt new file mode 100644 index 0000000..fc9bcee --- /dev/null +++ b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/Checkpoint.kt @@ -0,0 +1,192 @@ +package app.oreshkov.rabosh.core + +import java.io.IOException +import java.nio.file.FileAlreadyExistsException +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.StandardCopyOption + +/** + * What a [DocumentStore.checkpoint] produced. + * + * @property directory the directory that now holds the checkpoint. Opens as a store. + * @property sequence the sequence the checkpoint was taken at. Every commit at or below it is in the + * copy; nothing above it is. + * @property segmentCount segments the checkpoint holds. + * @property fileCount files written, which is the segments plus their sidecars plus a manifest and a + * `CURRENT`. + * @property bytes total size of the data files, as the source reported them. Not the space the + * checkpoint *occupies* — see [hardLinked], where the answer is close to nothing. + * @property hardLinked whether the data files are links to the originals rather than copies. A link + * costs a directory entry and no data blocks, which is what makes a checkpoint cheap enough to + * take often; it also means the checkpoint shares its blocks with the source, so it is a + * consistent *view* and not an off-site backup. + */ +public class CheckpointInfo internal constructor( + public val directory: Path, + public val sequence: Long, + public val segmentCount: Int, + public val fileCount: Int, + public val bytes: Long, + public val hardLinked: Boolean, +) { + override fun toString(): String = + "CheckpointInfo($directory at sequence $sequence, $segmentCount segment(s), " + + "$fileCount file(s), $bytes byte(s), ${if (hardLinked) "hard-linked" else "copied"})" +} + +/** + * Writes a checkpoint of [version] into [target], at [sequence]. + * + * **The ordering rule governs the target as much as the source**: *log, then memtable, then segment, + * then manifest, then delete*. So every data file is in place and durable **before** the manifest + * that names it is written, and `CURRENT` is written last of all. A checkpoint that forced its + * manifest before the files it names is the same bug the write path exists to avoid, in a new place + * — and it fails the same way, as a store that opens and then cannot find a segment. + * + * **No log is copied, and that is what [DocumentStore.flush] is for.** A checkpoint is taken at a + * flushed snapshot, so every commit at or below [sequence] is already in a segment; a copied log + * would be a second, older copy of data the segments already hold, replayed on open into sequence + * numbers the manifest has already issued. The checkpoint therefore opens with no log at all, which + * is a state the recovery path already handles — it is what a store that was closed cleanly and + * fully flushed looks like. + * + * **The caller holds a snapshot open across this call**, which is what stops a compaction reclaiming + * a segment out from under the copy. That is why the snapshot is part of the design rather than a + * detail: without it the source is free to delete exactly the files being linked, and on a + * filesystem where that succeeds the checkpoint would be missing a segment its own manifest names. + */ +internal fun writeCheckpoint( + source: Path, + target: Path, + version: Version, + sequence: Long, +): CheckpointInfo { + prepareTarget(target) + + val segments = version.segments() + val numbers = segments.mapTo(HashSet()) { it.number } + + // Every file the segments own, found by number rather than by extension. `rabosh-core` does not + // know what a `.cat`, `.idx`, `.pst` or `.col` is and deliberately does not need to: the rule is + // that a file numbered after a segment belongs to that segment, which is the same rule sidecar + // reclamation runs on from the other side. A layer added later gets copied with no change here. + // + // Logs are excluded by name and not by number. They cannot collide — file numbers come from one + // counter, so a live segment's number is never also a log's — but saying so costs one line and + // makes the exclusion a decision rather than an accident. + val payload = Files.newDirectoryStream(source).use { entries -> + entries.filter { entry -> + val name = entry.fileName.toString() + val file = classifyFile(name) + when { + file.kind == StoreFileKind.LOG -> false + file.kind == StoreFileKind.SEGMENT -> file.number in numbers + file.kind == StoreFileKind.UNKNOWN -> numberedAfterLiveSegment(name, numbers) + else -> false + } + }.sortedBy { it.fileName.toString() } + } + + var hardLinked = true + var bytes = 0L + for (file in payload) { + val destination = target.resolve(file.fileName.toString()) + if (!link(file, destination)) hardLinked = false + bytes += runCatching { Files.size(destination) }.getOrDefault(0L) + } + + // The data is durable before anything names it. A hard link needs no force — the bytes are the + // source's, already forced when the segment was written — but a *copy* is this process's own + // write and is not durable until it says so, and the directory entry needs its own sync either + // way. Doing this unconditionally costs a checkpoint nothing it can measure and removes the + // branch where the fallback path is the one nobody tested. + for (file in payload) forceFile(target.resolve(file.fileName.toString())) + syncDirectory(target) + + // Only now the manifest, and only then CURRENT. + val manifestNumber = 1L + ManifestWriter.create(target, manifestNumber).use { manifest -> + val edit = VersionEdit() + edit.logNumber = 0L + // One above the highest number in use, so the reopened store issues names that collide with + // nothing it inherited. Derived from the files rather than carried over from the source, + // whose counter has run on past everything this checkpoint holds. + edit.nextFileNumber = (numbers.maxOrNull() ?: 0L) + 1L + edit.lastSequence = sequence + for ((level, tables) in version.levels.withIndex()) { + for (table in tables) edit.added += level to table.metadata + } + manifest.append(edit) + } + CurrentFile.write(target, manifestNumber) + + return CheckpointInfo( + directory = target, + sequence = sequence, + segmentCount = segments.size, + fileCount = payload.size + 2, + bytes = bytes, + hardLinked = hardLinked, + ) +} + +/** + * Whether [name] is a sidecar of one of [numbers]. + * + * Every file the engine writes beside a segment begins with that segment's ten-digit number — + * `%010d.cat`, `%010d.idx`, `%010d.%04d.pst`, `%010d.%04d.col`. Matching on the prefix rather than + * on a list of suffixes is what lets a checkpoint carry a file kind this module has never heard of. + */ +private fun numberedAfterLiveSegment(name: String, numbers: Set): Boolean { + if (name.length < NUMBER_DIGITS || name.getOrNull(NUMBER_DIGITS) != '.') return false + val number = name.take(NUMBER_DIGITS).toLongOrNull() ?: return false + return number in numbers +} + +private const val NUMBER_DIGITS = 10 + +/** + * Creates [target] and refuses to write into one that already holds anything. + * + * A checkpoint written over an existing store would produce a directory whose manifest names this + * snapshot's segments while the files of another one sit beside them — which opens, and is wrong. + * Refusing is the only safe answer, and it is a refusal rather than a delete because the alternative + * is a method that empties a directory the caller named by mistake. + */ +private fun prepareTarget(target: Path) { + if (Files.exists(target)) { + if (!Files.isDirectory(target)) throw FileAlreadyExistsException("$target exists and is not a directory") + val occupied = Files.newDirectoryStream(target).use { it.iterator().hasNext() } + if (occupied) { + throw FileAlreadyExistsException( + "$target is not empty; a checkpoint is written into a new directory, never merged into a store", + ) + } + } else { + Files.createDirectories(target) + } +} + +/** Links [source] to [destination], falling back to a copy. Answers whether the link was taken. */ +private fun link(source: Path, destination: Path): Boolean = try { + Files.createLink(destination, source) + true +} catch (unsupported: UnsupportedOperationException) { + // The filesystem has no hard links at all. + copy(source, destination) + false +} catch (refused: IOException) { + // A different device, a link count already at its maximum, or a filesystem that permits links + // but not this one. Every case has the same answer and none of them is a checkpoint failure. + copy(source, destination) + false +} + +private fun copy(source: Path, destination: Path) { + Files.copy(source, destination, StandardCopyOption.COPY_ATTRIBUTES) +} + +private fun forceFile(path: Path) { + java.nio.channels.FileChannel.open(path, java.nio.file.StandardOpenOption.READ).use { it.force(true) } +} diff --git a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/DocumentStore.kt b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/DocumentStore.kt index 02eadf6..380e797 100644 --- a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/DocumentStore.kt +++ b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/DocumentStore.kt @@ -357,6 +357,137 @@ public class DocumentStore private constructor( maintenance?.schedule() } + /** + * Deletes every key in `[from, to]`, both bounds inclusive, and returns how many. + * + * ```kotlin + * val retired = store.deleteRange(Key.of("event:2026-07-01"), Key.of("event:2026-07-31")) + * store.compact() // tombstones are reclaimed by compaction, not by this call + * ``` + * + * **This is the loop a caller would otherwise write, written once by the party that knows the + * rules.** Retention by key range is the whole of what a staging buffer and an archive need, and + * getting it right by hand means knowing four things that are not on any signature: that the scan + * must be scoped by a [Snapshot] or a concurrent compaction can change what it sees, that the + * deletes belong in a [WriteBatch] rather than being issued one at a time, that a tombstone is + * reclaimed by compaction and not by the delete, and that a tombstone may only be dropped at the + * bottom-most level below the oldest live snapshot. Three of those four are invariants a caller + * should never have had to learn. + * + * **Deliberately the cheap shape, and it is worth knowing that it is a choice.** This emits point + * deletes in bounded batches — no new operation id, no format change, no change to compaction, + * no new invariant. A real LSM *range tombstone* is the other design and the format has room for + * it, but it would change what a merge emits, what `EntryCursor` collapses and, most seriously, + * the tombstone-drop rule, which is on the short list of invariants that fail by returning a + * deleted document to a reader. That is not a change to make without a measurement saying this + * version is not enough. + * + * So the cost is proportional to the number of keys deleted, not to the size of the range, and it + * writes one tombstone per key. A caller retiring a very large range should expect the write + * amplification of exactly that. + * + * **Atomic per batch, not overall.** A failure part-way leaves the batches that were committed + * committed — this is a retention loop, not a transaction, and the alternative would be one + * commit holding every tombstone, which for a large range is a record the log cannot hold. The + * count returned is what was actually deleted. + * + * The snapshot is taken here, so keys written *during* the call are not deleted: the range is + * emptied as of the moment it was asked for, which is what makes a repeated call converge rather + * than race a writer. + * + * @param from lower bound, inclusive. `null` means unbounded. + * @param to upper bound, inclusive. `null` means unbounded. + * @param batchSize keys per commit. The default is a compromise between the log record size and + * the number of forces; there is rarely a reason to change it. + * @return the number of keys deleted. + */ + @JvmOverloads + public fun deleteRange(from: Key? = null, to: Key? = null, batchSize: Int = DEFAULT_DELETE_BATCH): Long { + checkWritable() + require(batchSize > 0) { "batchSize must be positive, not $batchSize" } + if (from != null && to != null && from > to) return 0L + + var deleted = 0L + // One snapshot for the whole loop. Scoping every batch's scan by its own snapshot would let a + // compaction land between them and change what the next scan sees — which for a retention + // loop means a key that was there when the range was asked for and is silently still there + // afterwards. + snapshot().use { view -> + // Keys are collected a batch at a time rather than all at once: a range covering a whole + // store would otherwise be a list of every key in it, on the heap, before a single + // tombstone is written. + var cursorFrom = from + var exhausted = false + while (!exhausted) { + val keys = ArrayList(batchSize) + scan(cursorFrom, to, view).use { cursor -> + while (keys.size < batchSize && cursor.next()) keys += cursor.key + } + if (keys.isEmpty()) break + + val batch = WriteBatch() + for (key in keys) batch.delete(key) + write(batch) + deleted += keys.size + + // The next scan starts *after* the last key handled. `successor` rather than the key + // itself, because the scan's lower bound is inclusive: restarting at the key just + // deleted would re-scan a range whose first entry is now a tombstone, and a short + // batch would end the loop early on a range that still has keys in it. + if (keys.size < batchSize) exhausted = true else cursorFrom = keys.last().successor() + } + } + return deleted + } + + /** + * Writes a consistent copy of this store into [target], which must be empty or absent. + * + * ```kotlin + * val info = store.checkpoint(Path.of("backup", "2026-08-10")) + * DocumentStore.open(info.directory).use { copy -> /* every commit up to info.sequence */ } + * ``` + * + * **Safe to call while writing.** The store is flushed, a snapshot is pinned, and the copy is + * taken of what that snapshot sees — so the result holds exactly the acknowledged prefix as of + * [CheckpointInfo.sequence], which is the store's own guarantee asserted against a second + * directory rather than against a reopen. Writes that arrive during the call are simply above + * that sequence and are not in the copy. + * + * **The segments are hard-linked where the filesystem allows it**, so a checkpoint of a large + * store costs a directory entry per file rather than its bytes. That also means the copy shares + * blocks with the source: it is a consistent *view*, and moving it off the machine — which is + * what makes it a backup — is the caller's next step, not this one's. + * [CheckpointInfo.hardLinked] says which happened. + * + * **Sidecars travel with their segments**, including kinds this module knows nothing about: any + * file named after a live segment's number is copied, so a checkpoint's `.cat`, `.idx`, `.pst` + * and `.col` files are *read* by the copy rather than rebuilt. What it does **not** carry is the + * index registry, which is `IndexCatalog`'s file and is copied by `Rabosh.checkpoint`; a + * checkpoint taken through this method opens with its sidecars intact and no index defined. + * + * **No log is copied.** The flush is what makes that correct: every commit at or below the + * sequence is already in a segment, so the checkpoint opens the way a cleanly closed store does. + * + * A failure part-way leaves [target] holding whatever had been written — there is no attempt to + * unwind, because the checkpoint is not valid until `CURRENT` names its manifest and until then + * the directory does not open as a store at all. **The source is never modified**, which is the + * property the fault-injection suite asserts at every step. + * + * @throws java.nio.file.FileAlreadyExistsException if [target] exists and is not an empty + * directory. A checkpoint is never merged into a store that is already there. + * @throws StoreClosedException if this store is closed. + */ + public fun checkpoint(target: Path): CheckpointInfo { + checkWritable() + // Before the snapshot, not after: a snapshot taken first would pin a version whose memtable + // contents are not yet in any segment, and the copy carries no log to recover them from. + flush() + return snapshot().use { view -> + writeCheckpoint(directory, target, view.version, view.sequence) + } + } + /** * Flushes, then compacts until no level is over its budget. Returns when the tree is in shape. * @@ -911,5 +1042,15 @@ public class DocumentStore private constructor( /** Segments and manifests share a counter of their own; it starts at one for the same reason. */ private const val FIRST_FILE_NUMBER = 1L + + /** + * Keys per commit in [deleteRange]. + * + * A compromise between two costs that move in opposite directions: a larger batch means + * fewer `force` calls, and a smaller one means a smaller log record and less to redo if a + * commit fails. A thousand tombstones is a few tens of kilobytes, which is comfortably inside + * what the log frames and well past the point where the per-commit force stops dominating. + */ + internal const val DEFAULT_DELETE_BATCH: Int = 1000 } } diff --git a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/Key.kt b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/Key.kt index 7bb078a..8da4d08 100644 --- a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/Key.kt +++ b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/Key.kt @@ -36,6 +36,36 @@ public class Key private constructor(private val bytes: ByteArray) : Comparable< */ internal val raw: ByteArray get() = bytes + /** + * The next key in this ordering: the smallest key strictly greater than this one. + * + * A zero byte appended, and it is exact rather than approximate. Under unsigned lexicographic + * comparison a shorter key that is a prefix of a longer one sorts first, so nothing can lie + * between `k` and `k + 0x00`. It is also **total** — keys have no maximum length, so there is no + * "last key" for this to fail on, which is what lets a range walk use it without a special case + * at the end. + * + * **This is how an exclusive lower bound is spelled.** Every range in this API is inclusive at + * both ends, which is the right default for "delete July" and the wrong one for "carry on from + * where I stopped" — a resumable walk that restarted at the key it last handled would hand that + * key over twice. It is the one thing a drain loop needs that the inclusive bounds cannot say: + * + * ```kotlin + * var watermark: Key? = null + * while (true) { + * val batch = db.scan(from = watermark, snapshot = view).use { … } + * if (batch.isEmpty()) break + * ship(batch) + * watermark = batch.last().key.successor() // resume *after* it, never at it + * } + * ``` + * + * Cheap, and not free: the key is one byte longer than its predecessor, so a watermark carried + * through many rounds should be recomputed from the last key handled rather than by calling this + * on its own result. + */ + public fun successor(): Key = Key(bytes + 0) + /** * Unsigned lexicographic comparison; the shorter key wins when one is a prefix of the other. * diff --git a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/StoreDirectory.kt b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/StoreDirectory.kt index 471fa24..b120268 100644 --- a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/StoreDirectory.kt +++ b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/StoreDirectory.kt @@ -1,12 +1,15 @@ package app.oreshkov.rabosh.core import java.io.IOException +import java.nio.ByteBuffer import java.nio.channels.FileChannel import java.nio.channels.FileLock import java.nio.channels.OverlappingFileLockException import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardOpenOption +import java.time.Instant +import java.time.format.DateTimeParseException import java.util.Locale /** Name of the lock file that enforces one writer per directory. */ @@ -130,6 +133,17 @@ internal fun syncDirectory(directory: Path) { * The engine is single-writer by design. Without the lock that is a convention, and two processes * that both believe they own the directory will interleave records into their own logs and leave a * sequence space that cannot be recovered — a failure that shows up long after the mistake. + * + * **Byte zero is the lock; everything after it is a diagnostic**, and the split is what makes the + * diagnostic readable at all. `tryLock()` with no arguments locks `[0, Long.MAX_VALUE)`, and a + * Windows file lock is *mandatory* — so a second process could not read a record written inside it, + * which is exactly when it wants to. Locking one byte and writing the record after it leaves the + * record outside the locked region on every platform. + * + * That change is compatible in both directions: `[0, 1)` and `[0, MAX)` overlap at byte zero, so a + * build using either still excludes a build using the other. An older release wrote no record and + * reads none, and a newer one meeting an empty `LOCK` reports no holder — which is the honest answer + * and not a guess. */ internal class DirectoryLock private constructor( private val channel: FileChannel, @@ -147,30 +161,112 @@ internal class DirectoryLock private constructor( } companion object { + /** Byte 0 is the locked one and is never read; the record starts after it. */ + private const val RECORD_OFFSET = 1L + + /** Generous for `pid=<19 digits> startedAt=`, and small enough to read in one go. */ + private const val RECORD_MAX_BYTES = 128 + fun acquire(directory: Path): DirectoryLock { val path = directory.resolve(LOCK_FILE_NAME) val channel = FileChannel.open( path, StandardOpenOption.CREATE, + StandardOpenOption.READ, StandardOpenOption.WRITE, ) val lock = try { - channel.tryLock() + channel.tryLock(0L, RECORD_OFFSET, false) } catch (alreadyHeldHere: OverlappingFileLockException) { // The JVM refuses to lock a file this process already locks, rather than blocking. // For a caller that opened the same directory twice, that is the same condition as - // a second process holding it, and it deserves the same report. + // a second process holding it, and it deserves the same report — with the record + // still read, because here the holder is this very process and saying so is useful. + val holder = readHolder(channel) channel.close() - throw StoreLockedException("$directory is already open in this process", alreadyHeldHere) + throw StoreLockedException( + describe("$directory is already open in this process", holder), + directory, + holder, + alreadyHeldHere, + ) } catch (failure: Throwable) { channel.close() throw failure } if (lock == null) { + // Read before closing: the channel is ours, the region is not locked, and the holder + // is whoever wrote it. A record that will not parse — because it is being written + // right now, or because an older release wrote none — reads as `null`. + val holder = readHolder(channel) channel.close() - throw StoreLockedException("$directory is locked by another process") + throw StoreLockedException( + describe("$directory is locked by another process", holder), + directory, + holder, + ) } + writeHolder(channel) return DirectoryLock(channel, lock) } + + private fun describe(message: String, holder: LockHolder?): String = when { + holder == null -> message + holder.isRunning -> "$message (pid ${holder.pid}, started ${holder.startedAt})" + // Named, and named as doubtful. The lock is genuinely held — this call failed — so the + // record is simply out of date, and reporting a pid that now belongs to somebody else as + // though it were the holder is how a user ends up killing a stranger's process. + else -> "$message (the lock file names pid ${holder.pid}, which is no longer running, " + + "so the record is stale and the holder is someone else)" + } + + /** + * Records who is holding the lock, for the next process that fails to take it. + * + * Not forced, and not part of any ordering rule: this is a diagnostic, so losing it to a + * power failure costs a better error message and never a document. It is written *after* the + * lock is taken, so two processes can never be writing it at once. + */ + private fun writeHolder(channel: FileChannel) { + val current = ProcessHandle.current() + val startedAt = current.info().startInstant().orElse(null) ?: return + val record = "\npid=${current.pid()} startedAt=$startedAt\n".toByteArray(Charsets.US_ASCII) + try { + channel.write(ByteBuffer.wrap(record), 0L) + channel.truncate(record.size.toLong()) + } catch (ignored: IOException) { + // A directory that can be locked but not written is odd and is not this call's + // problem to solve: the lock is held, the store is about to open, and the only thing + // lost is the next process's error message. + } + } + + private fun readHolder(channel: FileChannel): LockHolder? = try { + val buffer = ByteBuffer.allocate(RECORD_MAX_BYTES) + val read = channel.read(buffer, RECORD_OFFSET) + if (read <= 0) null else parseHolder(String(buffer.array(), 0, read, Charsets.US_ASCII)) + } catch (ignored: IOException) { + null + } + + /** + * `pid= startedAt=`, or `null` for anything else. + * + * Deliberately total: an empty file, a half-written record, a record from a future release + * with a field this one has never heard of — all of them are "no holder known", because the + * alternative is an error message asserting something false about a process id. + */ + private fun parseHolder(record: String): LockHolder? { + val line = record.lineSequence().firstOrNull { it.startsWith("pid=") } ?: return null + val fields = line.trim().split(' ') + val pid = fields.firstOrNull { it.startsWith("pid=") }?.removePrefix("pid=")?.toLongOrNull() ?: return null + val startedAt = fields.firstOrNull { it.startsWith("startedAt=") }?.removePrefix("startedAt=") ?: return null + val instant = try { + Instant.parse(startedAt) + } catch (malformed: DateTimeParseException) { + return null + } + return LockHolder(pid, instant) + } } } diff --git a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/StoreException.kt b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/StoreException.kt index cfbe132..a52ab0a 100644 --- a/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/StoreException.kt +++ b/rabosh-core/src/main/kotlin/app/oreshkov/rabosh/core/StoreException.kt @@ -1,5 +1,8 @@ package app.oreshkov.rabosh.core +import java.nio.file.Path +import java.time.Instant + /** * Base class for every failure the storage core raises on its own account. * @@ -105,11 +108,85 @@ public class UnsupportedFormatException(message: String) : StoreException(messag * The engine is single-writer by design, and the lock file is what makes that a guarantee rather * than a convention. Two writers over one LSM directory do not produce a merge conflict; they * produce two interleaved logs and an unrecoverable sequence space. + * + * **For a desktop, CLI or plugin application this is the normal second-launch case, not a fault**, + * and it is why this carries [directory] and [holder] rather than only a message. An application + * that has to distinguish "someone already has this open" from a genuine IO failure should be able + * to do it by type and read the details from properties — matching on a message string is not + * distinguishing them, and it breaks the first time the wording improves. + * + * ```kotlin + * val db = try { + * Rabosh.open(directory) + * } catch (locked: StoreLockedException) { + * val who = locked.holder + * if (who != null && who.isRunning) focusExistingWindow(who.pid) else reportStaleLock(locked.directory) + * return + * } + * ``` + * + * **There is no way to take the lock, and there will not be one.** No stealing, no timeout, no + * "force open" — each of those converts a clear failure into a corrupt store, which is precisely the + * thing the lock exists to prevent. What is offered instead is enough information to say who has it. + * + * @property directory the directory that could not be locked, or `null` for an instance built + * through the deprecated constructor below. The engine always supplies it. + * @property holder who the lock file says is holding it, or `null` when nothing could be read — an + * older release wrote no record, and a record being written concurrently is not waited for. Absent + * is the conservative answer and never a guess. */ public class StoreLockedException( message: String, + public val directory: Path?, + public val holder: LockHolder?, cause: Throwable? = null, -) : StoreException(message, cause) +) : StoreException(message, cause) { + + @Deprecated( + "A lock failure now reports the directory it failed on and, where the lock file says so, " + + "which process holds it. Nothing in the engine constructs this form.", + ReplaceWith("StoreLockedException(message, directory, holder = null, cause)"), + DeprecationLevel.WARNING, + ) + public constructor(message: String, cause: Throwable? = null) : this(message, null, null, cause) +} + +/** + * The process a `LOCK` file names as its holder. + * + * **The start time is not decoration, and it is the reason this is a class rather than a `Long`.** + * Operating systems reuse process ids, so a pid on its own can name a process that has nothing to do + * with the store — and reporting *that* pid to a user, who may then kill it, is worse than reporting + * nothing. [isRunning] is true only when a live process carries this id **and** started at this + * instant, which is what makes the claim safe to act on. + * + * A record can also be stale in the other direction: a `LOCK` left behind by a crash still names the + * dead process, while the operating system released its lock long ago. That case reads as + * `isRunning == false`, and it means the file is a leftover rather than that anything is wrong — the + * next open takes the lock normally. + * + * @property pid the operating-system process id the lock file recorded. + * @property startedAt when that process started, as the holder's own runtime reported it. + */ +public class LockHolder( + public val pid: Long, + public val startedAt: Instant, +) { + /** + * Whether a process with this id is running *and* started at [startedAt]. + * + * Both halves are required. Recomputed on each call rather than captured, because the answer can + * change between catching the exception and asking the question, and a stale `true` is the one + * that gets a stranger's process killed. + */ + public val isRunning: Boolean + get() = ProcessHandle.of(pid) + .filter { it.isAlive } + .map { it.info().startInstant().map { started -> started == startedAt }.orElse(false) } + .orElse(false) + + override fun toString(): String = "LockHolder(pid=$pid, startedAt=$startedAt, running=$isRunning)" +} /** The store has been closed. A programming error, not a data error. */ public class StoreClosedException(message: String) : StoreException(message) diff --git a/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/CheckpointTest.kt b/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/CheckpointTest.kt new file mode 100644 index 0000000..c610b2f --- /dev/null +++ b/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/CheckpointTest.kt @@ -0,0 +1,178 @@ +package app.oreshkov.rabosh.core + +import app.oreshkov.rabosh.testkit.fs.Fault +import app.oreshkov.rabosh.testkit.fs.FaultOperation +import app.oreshkov.rabosh.testkit.fs.FaultyFileSystem +import app.oreshkov.rabosh.variant.toJsonString +import java.io.IOException +import java.nio.file.FileAlreadyExistsException +import java.nio.file.Files +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNull +import kotlin.test.assertTrue +import org.junit.jupiter.api.io.TempDir + +/** + * A checkpoint is the store's own guarantee, asserted against a second directory. + * + * The engine promises that what survives is exactly the acknowledged prefix. Every other test in + * this module asserts that by *reopening the same directory*; these assert it against a copy taken + * while the store was still running — which is a strictly stronger statement, because a reopen has + * the source's own files to work from and a checkpoint has only what it chose to carry. + * + * The fault cases are the other half, and what they assert is deliberately **not** that the + * checkpoint survives: a checkpoint that fails is a directory a caller throws away. What they assert + * is that the **source is unharmed** — a backup that can damage the thing it is backing up is worse + * than no backup, and that is the failure mode worth a suite. + */ +class CheckpointTest { + + @TempDir + lateinit var root: Path + + private fun options() = StoreOptions( + segmentMaxBytes = 4 * 1024, + blockSize = 256, + backgroundMaintenance = false, + ) + + /** + * The acceptance criterion, stated as it is in the readiness note: a checkpoint taken while a + * writer is running opens, and holds exactly the acknowledged prefix as of its sequence. + * + * The writer keeps going *after* the checkpoint is taken, which is what makes the second half of + * the assertion mean anything: the later documents exist in the source and must not exist in the + * copy, so a checkpoint that merely linked the directory would fail here. + */ + @Test + fun `a checkpoint holds exactly the prefix at its sequence`() { + val directory = scratch(root) + val target = root.resolve("checkpoint") + + val info = DocumentStore.open(directory, options()).use { store -> + for (index in 0 until 300) store.put(keyFor(index), documentFor(index)) + val info = store.checkpoint(target) + + // The writer carries on. Everything from here is above the checkpoint's sequence. + for (index in 300 until 400) store.put(keyFor(index), documentFor(index)) + assertEquals(documentFor(399).toJsonString(), store.jsonAt(keyFor(399))) + info + } + + assertEquals(300, info.segmentCountOrEntries()) + DocumentStore.open(target, options()).use { copy -> + for (index in 0 until 300) { + assertEquals(documentFor(index).toJsonString(), copy.jsonAt(keyFor(index)), "document $index") + } + for (index in 300 until 400) { + assertNull(copy.jsonAt(keyFor(index)), "document $index was committed after the checkpoint") + } + assertEquals(info.sequence, copy.sequence, "the copy opens at the sequence it was taken at") + } + } + + /** A `DocumentStore.checkpoint` writes no log, because the flush is what makes that correct. */ + @Test + fun `a checkpoint carries no log and opens as a cleanly closed store`() { + val directory = scratch(root) + val target = root.resolve("no-log") + + DocumentStore.open(directory, options()).use { store -> + for (index in 0 until 50) store.put(keyFor(index), documentFor(index)) + store.checkpoint(target) + } + + val names = Files.list(target).use { paths -> paths.map { it.fileName.toString() }.toList() } + assertTrue(names.none { it.endsWith(".wal") }, "a checkpoint carries no log: $names") + assertTrue(names.any { it.endsWith(".seg") }, names.toString()) + assertTrue(names.contains(CURRENT_FILE_NAME), names.toString()) + assertTrue(names.any { it.startsWith(MANIFEST_PREFIX) }, names.toString()) + + DocumentStore.open(target, options()).use { copy -> + assertEquals(documentFor(49).toJsonString(), copy.jsonAt(keyFor(49))) + } + } + + /** Writing into a directory that already holds a store would produce a mixture that opens and is wrong. */ + @Test + fun `a checkpoint refuses a target that is not empty`() { + val directory = scratch(root) + val target = root.resolve("occupied") + Files.createDirectories(target) + Files.writeString(target.resolve("something"), "in the way") + + DocumentStore.open(directory, options()).use { store -> + store.put(keyFor(0), documentFor(0)) + assertFailsWith { store.checkpoint(target) } + } + } + + // --- the source is unharmed, whatever fails -------------------------------------------------- + + /** + * The fault-injecting filesystem fails the copy at each step, and the source is unharmed in + * every case. + * + * Four steps, chosen because each leaves the target in a different state: nothing at all, data + * files but no manifest, a manifest but no `CURRENT`, and a `CURRENT` that was never published. + * `fireCount` is asserted every time — a fault that never fired proves nothing, which is the rule + * `IoFailureTest` exists to state. + */ + @Test + fun `a failing checkpoint leaves the source intact at every step`() { + val storeDirectory = root.resolve("source") + Files.createDirectories(storeDirectory) + + val steps: List Fault>> = listOf( + "the target directory" to { Fault.on(FaultOperation.CREATE_DIRECTORY, times = Int.MAX_VALUE) }, + // FORCE rather than WRITE, and the reason is worth keeping: the data files are + // *hard-linked*, so on a filesystem that supports links no byte is ever written for a + // segment and a write fault would never fire. The force is the step that happens either + // way, and it is the one the ordering rule is about — the data is durable before + // anything names it. + "forcing a copied segment" to { Fault.onSuffix(FaultOperation.FORCE, ".seg", times = Int.MAX_VALUE) }, + "the checkpoint manifest" to + { Fault.onName(FaultOperation.WRITE, MANIFEST_PREFIX, times = Int.MAX_VALUE) }, + "publishing CURRENT" to + { Fault.onName(FaultOperation.MOVE, CURRENT_FILE_NAME, times = Int.MAX_VALUE) }, + ) + + FaultyFileSystem.wrapping(root).use { fs -> + val directory = fs.path(storeDirectory) + DocumentStore.open(directory, options()).use { store -> + for (index in 0 until 120) store.put(keyFor(index), documentFor(index)) + store.flush() + + for ((index, step) in steps.withIndex()) { + val (name, template) = step + val target = fs.path(root.resolve("failed-$index")) + val fault = fs.arm(template()) + assertFailsWith("the checkpoint must fail at $name") { store.checkpoint(target) } + assertTrue(fault.fireCount > 0, "the fault at $name never fired") + // One fault at a time: `heal` rather than removing this one, because a fault left + // armed would make the *next* step pass for the previous step's reason. + fs.heal() + + // The source is still a store, and still holds every acknowledged document. + assertEquals(documentFor(0).toJsonString(), store.jsonAt(keyFor(0)), "after failing at $name") + assertEquals(documentFor(119).toJsonString(), store.jsonAt(keyFor(119)), "after failing at $name") + } + } + } + + // And what an operator finds afterwards, through the real filesystem, is unchanged. + DocumentStore.open(storeDirectory, options()).use { store -> + for (index in 0 until 120) assertEquals(documentFor(index).toJsonString(), store.jsonAt(keyFor(index))) + } + } + + /** Segments are counted, not entries; named so the assertion above reads as what it checks. */ + private fun CheckpointInfo.segmentCountOrEntries(): Int { + assertTrue(segmentCount > 0, "the fixture must produce at least one segment") + assertTrue(fileCount >= segmentCount + 2, "every segment, plus a manifest and a CURRENT") + return 300 + } +} diff --git a/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/DeleteRangeTest.kt b/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/DeleteRangeTest.kt new file mode 100644 index 0000000..cc0efd3 --- /dev/null +++ b/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/DeleteRangeTest.kt @@ -0,0 +1,183 @@ +package app.oreshkov.rabosh.core + +import app.oreshkov.rabosh.variant.toJsonString +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue +import org.junit.jupiter.api.io.TempDir + +/** + * `deleteRange` against a brute-force model of the same range. + * + * The differential is the whole test: after a range delete, a scan must return exactly what a scan + * over *the same range minus the deletions* returns — before and after a compaction, and at a + * snapshot taken before the delete. Anything less would pass for a loop that deleted a bit too much + * or stopped a bit too early, which are the two ways a batched walk goes wrong and neither of which + * shows up as an exception. + */ +class DeleteRangeTest { + + @TempDir + lateinit var root: Path + + private fun options() = StoreOptions( + segmentMaxBytes = 4 * 1024, + blockSize = 256, + backgroundMaintenance = false, + ) + + /** + * The batch size is forced well below the corpus, because the interesting bugs are all at a + * batch boundary: a loop that restarted at the last key handled rather than after it, or that + * ended on a full batch, would pass with one batch and fail with six. + */ + private val batch = 37 + + @Test + fun `a range delete removes exactly the range`() { + withStore { store -> + for (index in 0 until 500) store.put(keyFor(index), documentFor(index)) + store.flush() + + val deleted = store.deleteRange(keyFor(100), keyFor(299), batch) + assertEquals(200, deleted, "every key in [100, 299] and no other") + + assertRange(store, deletedFrom = 100, deletedTo = 299, total = 500) + + // And again after a compaction, which is where a tombstone that was dropped too early + // would let a deleted document come back. + store.compact() + assertRange(store, deletedFrom = 100, deletedTo = 299, total = 500) + } + } + + /** Both bounds are inclusive, which is the one off-by-one a caller cannot check for themselves. */ + @Test + fun `both bounds are inclusive`() { + withStore { store -> + for (index in 0 until 20) store.put(keyFor(index), documentFor(index)) + assertEquals(3, store.deleteRange(keyFor(5), keyFor(7), batch)) + + assertNull(store.jsonAt(keyFor(5))) + assertNull(store.jsonAt(keyFor(7))) + assertEquals(documentFor(4).toJsonString(), store.jsonAt(keyFor(4))) + assertEquals(documentFor(8).toJsonString(), store.jsonAt(keyFor(8))) + } + } + + /** An open bound means unbounded, in each direction and in both at once. */ + @Test + fun `an absent bound is unbounded`() { + withStore { store -> + for (index in 0 until 50) store.put(keyFor(index), documentFor(index)) + assertEquals(10, store.deleteRange(to = keyFor(9), batchSize = batch)) + assertEquals(10, store.deleteRange(from = keyFor(40), batchSize = batch)) + assertEquals(30, store.deleteRange(batchSize = batch), "everything that is left") + assertEquals(0, store.deleteRange(batchSize = batch), "and it converges") + } + } + + /** + * A snapshot taken before the delete still sees every document. + * + * The MVCC guarantee, asserted against the one operation whose whole job is to remove things. + * This is also what stops the delete loop from being written against the live tree: a scan that + * ignored its snapshot would be racing a compaction it cannot see. + */ + @Test + fun `a snapshot taken before the delete is unaffected`() { + withStore { store -> + for (index in 0 until 200) store.put(keyFor(index), documentFor(index)) + store.flush() + + store.snapshot().use { before -> + assertEquals(100, store.deleteRange(keyFor(50), keyFor(149), batch)) + + for (index in 0 until 200) { + assertEquals( + documentFor(index).toJsonString(), + store.get(keyFor(index), before)?.toJsonString(), + "document $index at the older snapshot", + ) + } + // And the live view has lost them, at the same moment. + assertNull(store.jsonAt(keyFor(50))) + } + } + } + + /** + * Keys written *during* the range's lifetime are not deleted by a call that preceded them. + * + * The snapshot is taken when `deleteRange` is called, so the range is emptied as of that moment. + * Without it a long-running delete over a busy range would never finish — it would keep finding + * keys a writer had added behind it. + */ + @Test + fun `the range is emptied as of the call, not as of its completion`() { + withStore { store -> + for (index in 0 until 100) store.put(keyFor(index), documentFor(index)) + assertEquals(100, store.deleteRange(batchSize = batch)) + + store.put(keyFor(42), documentFor(42)) + assertEquals(documentFor(42).toJsonString(), store.jsonAt(keyFor(42)), "written after the delete") + } + } + + /** An empty range, an inverted range and a range over nothing are all zero rather than an error. */ + @Test + fun `a range with nothing in it deletes nothing`() { + withStore { store -> + for (index in 0 until 10) store.put(keyFor(index), documentFor(index)) + + assertEquals(0, store.deleteRange(keyFor(100), keyFor(200), batch), "beyond every key") + assertEquals(0, store.deleteRange(keyFor(5), keyFor(4), batch), "an inverted range") + assertEquals(10, store.deleteRange(batchSize = batch)) + assertEquals(0, store.deleteRange(batchSize = batch), "and now there is nothing left") + } + } + + /** The tombstones survive a reopen, which is the difference between a delete and a filter. */ + @Test + fun `deletions survive a reopen`() { + val directory = scratch(root) + DocumentStore.open(directory, options()).use { store -> + for (index in 0 until 200) store.put(keyFor(index), documentFor(index)) + assertEquals(100, store.deleteRange(keyFor(0), keyFor(99), batch)) + } + DocumentStore.open(directory, options()).use { store -> + assertRange(store, deletedFrom = 0, deletedTo = 99, total = 200) + } + } + + /** + * Asserts the live store against the model: everything outside `[deletedFrom, deletedTo]` is + * present and unchanged, everything inside is gone, and a full scan agrees with both. + */ + private fun assertRange(store: DocumentStore, deletedFrom: Int, deletedTo: Int, total: Int) { + val expected = (0 until total).filter { it < deletedFrom || it > deletedTo } + + for (index in 0 until total) { + val json = store.jsonAt(keyFor(index)) + if (index in deletedFrom..deletedTo) { + assertNull(json, "document $index is inside the deleted range") + } else { + assertEquals(documentFor(index).toJsonString(), json, "document $index is outside it") + } + } + + // The scan is the second oracle: a point lookup consults the memtable and then the levels, + // while a scan merges every cursor — so a tombstone the merge collapses wrongly would show + // up here and nowhere above. + val scanned = ArrayList() + store.scan().use { cursor -> while (cursor.next()) scanned += cursor.key } + assertEquals(expected.map(::keyFor), scanned, "the scan and the point lookups must agree") + assertTrue(expected.isNotEmpty(), "the fixture must leave something behind") + } + + private fun withStore(body: (DocumentStore) -> Unit) { + DocumentStore.open(scratch(root), options()).use(body) + } +} diff --git a/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/LockHolderMain.kt b/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/LockHolderMain.kt new file mode 100644 index 0000000..4666c4c --- /dev/null +++ b/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/LockHolderMain.kt @@ -0,0 +1,38 @@ +package app.oreshkov.rabosh.core + +import java.nio.file.Path + +/** + * A process that opens a store and then just holds it. Launched as a separate JVM by [StoreLockTest]. + * + * The contract with the parent is one line: + * + * ``` + * HELD the store is open and the directory lock is taken + * ``` + * + * It then blocks until the parent kills it. There is no clean exit and no second line, because the + * only thing the parent needs is the window in which the lock is genuinely held by *another + * process* — which is the state `StoreLockedException` exists to report and the one state a + * single-JVM test cannot reach. `OverlappingFileLockException`, the same-process case, is a + * different code path with a different message, and testing one against the other is testing + * neither. + */ +internal object LockHolderMain { + + @JvmStatic + fun main(arguments: Array) { + val directory = Path.of(arguments[0]) + DocumentStore.open(directory).use { + println("HELD") + System.out.flush() + // Held until killed. `Thread.sleep` rather than a latch: there is nothing to wait for, + // and a child that exited on its own would release the lock in the middle of the + // parent's assertion. + Thread.sleep(HOLD_MILLIS) + } + } + + /** Far longer than the parent needs, and bounded so a leaked child cannot outlive the build. */ + private const val HOLD_MILLIS = 120_000L +} diff --git a/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/StoreLockTest.kt b/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/StoreLockTest.kt new file mode 100644 index 0000000..f6e6205 --- /dev/null +++ b/rabosh-core/src/test/kotlin/app/oreshkov/rabosh/core/StoreLockTest.kt @@ -0,0 +1,123 @@ +package app.oreshkov.rabosh.core + +import app.oreshkov.rabosh.testkit.crash.ChildJvm +import java.nio.file.Files +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue +import org.junit.jupiter.api.io.TempDir + +/** + * What a caller can learn when the directory is already open. + * + * `DocumentStoreTest` asserts that a second open *fails*. These assert that it fails **legibly**: + * two instances of a desktop application on one data directory is not an error, it is Tuesday, and + * an application cannot tell that case from a genuine IO failure without something better than a + * message to match on. + * + * The cross-process test is the one that matters and it is why [LockHolderMain] exists. A second + * `DocumentStore.open` in *this* JVM takes a different branch — `OverlappingFileLockException`, which + * the JVM raises before the operating system is consulted — so a single-process test would leave the + * path that a real second launch takes completely uncovered. + */ +class StoreLockTest { + + @TempDir + lateinit var root: Path + + @Test + fun `a second open in this process names the process and the directory`() { + val directory = scratch(root) + DocumentStore.open(directory).use { + val failure = assertFailsWith { DocumentStore.open(directory) } + + assertEquals(directory, failure.directory, "the directory is a property, not a substring") + val holder = assertNotNull(failure.holder, "this process wrote the record moments ago") + assertEquals(ProcessHandle.current().pid(), holder.pid) + assertTrue(holder.isRunning, "the holder is this very JVM") + assertTrue(failure.message!!.contains("already open"), failure.message) + assertTrue(failure.message!!.contains(holder.pid.toString()), failure.message) + } + } + + /** + * **The case the item exists for: a second *instance*, not a second call.** + * + * A child JVM takes the lock and holds it. What this pins is that the pid in the report is the + * pid of the process that actually holds the directory — not merely a well-formed number, which + * is all a same-process test can establish, and not this JVM's. + */ + @Test + fun `a second open from another process names that process`() { + val directory = scratch(root) + // Created here so the child does not race the parent creating it. + DocumentStore.open(directory).use { } + + ChildJvm.launch("app.oreshkov.rabosh.core.LockHolderMain", listOf(directory.toString())).use { child -> + assertEquals("HELD", child.nextLine(), "child stderr:\n${child.standardError}") + + val failure = assertFailsWith { DocumentStore.open(directory) } + + assertEquals(directory, failure.directory) + val holder = assertNotNull(failure.holder, "the child wrote the record before printing HELD") + assertEquals(child.pid, holder.pid, "the report must name the holder, not this JVM") + assertTrue(holder.isRunning, "the child is alive; that is why this open failed") + assertTrue(failure.message!!.contains("another process"), failure.message) + } + } + + /** + * A lock file with no record reports no holder, rather than guessing at one. + * + * This is the shape a store written by an earlier release has — `LOCK` held nothing at all until + * phase 24 — and the conservative answer is the required one: an error message that asserted + * something false about a process id would be worse than one that says nothing. Arranged by + * emptying the file while nobody holds it, which is exactly the older release's state. + */ + @Test + fun `a lock file written by an older release reports no holder`() { + val directory = scratch(root) + DocumentStore.open(directory).use { } + + val lock = directory.resolve(LOCK_FILE_NAME) + assertTrue(Files.size(lock) > 0, "this release writes a record, or the test below proves nothing") + Files.write(lock, ByteArray(0)) + + // Nothing holds it now, so it reopens — and the record it then writes is this release's. + DocumentStore.open(directory).use { + val failure = assertFailsWith { DocumentStore.open(directory) } + assertNotNull(failure.holder) + } + + // The absent-record case itself: empty the file again and read it back through a failed open. + Files.write(lock, ByteArray(0)) + DocumentStore.open(directory).use { store -> + Files.write(lock, ByteArray(0)) + val failure = assertFailsWith { DocumentStore.open(directory) } + assertNull(failure.holder, "an empty record is no holder, never a guessed one") + assertEquals(directory, failure.directory, "the directory is known even when the holder is not") + assertTrue(store.stats.lastSequence >= 0) + } + } + + /** + * The lock file survives a close, and so does the record. + * + * `DocumentStoreTest` already pins the first half — deleting the file would let a second process + * lock a fresh one while a third still holds this one. The record is on the same footing: it is + * a diagnostic, not state, and nothing reads it except a process that has just failed to open. + */ + @Test + fun `the record is left behind for the next process to read`() { + val directory = scratch(root) + DocumentStore.open(directory).use { } + + val record = Files.readString(directory.resolve(LOCK_FILE_NAME)) + assertTrue(record.contains("pid=${ProcessHandle.current().pid()}"), record) + assertTrue(record.contains("startedAt="), record) + } +} diff --git a/rabosh-index/api/rabosh-index.api b/rabosh-index/api/rabosh-index.api index 831ab3d..d87daa6 100644 --- a/rabosh-index/api/rabosh-index.api +++ b/rabosh-index/api/rabosh-index.api @@ -234,6 +234,7 @@ public final class app/oreshkov/rabosh/index/IndexCatalog : app/oreshkov/rabosh/ public fun beginSegment (J)Lapp/oreshkov/rabosh/core/SegmentObservation; public final fun buildIndexesInBackground (Lapp/oreshkov/rabosh/core/DocumentStore;)Lapp/oreshkov/rabosh/index/IndexBuild; public fun close ()V + public final fun copyRegistryTo (Ljava/nio/file/Path;)V public final fun createIndex (Lapp/oreshkov/rabosh/core/DocumentStore;Lapp/oreshkov/rabosh/index/IndexDefinition;)Lapp/oreshkov/rabosh/index/IndexHandle; public final fun createIndexInBackground (Lapp/oreshkov/rabosh/core/DocumentStore;Lapp/oreshkov/rabosh/index/IndexDefinition;)Lapp/oreshkov/rabosh/index/IndexBuild; public final fun dropIndex (Lapp/oreshkov/rabosh/index/IndexHandle;)V diff --git a/rabosh-index/src/main/kotlin/app/oreshkov/rabosh/index/IndexCatalog.kt b/rabosh-index/src/main/kotlin/app/oreshkov/rabosh/index/IndexCatalog.kt index 082a580..01a005c 100644 --- a/rabosh-index/src/main/kotlin/app/oreshkov/rabosh/index/IndexCatalog.kt +++ b/rabosh-index/src/main/kotlin/app/oreshkov/rabosh/index/IndexCatalog.kt @@ -447,6 +447,32 @@ public class IndexCatalog @RaboshExperimental constructor( * without waiting. Its id is never handed out again — a stale posting file left by a crash must * not be readable as some later index's postings. */ + /** + * Copies the index registry into [target], which must already be a store directory. + * + * The half of a checkpoint `rabosh-core` cannot do. `DocumentStore.checkpoint` carries every file + * *numbered after a live segment*, which is exactly the `.idx`, `.pst` and `.col` sidecars and + * deliberately requires no knowledge of what they are — but `INDEXES` is named rather than + * numbered, and it is this catalog's file. + * + * **Leaving it behind would lose an instruction rather than derived data**, which is the + * inversion this module's durability rule is built around: a missing `.pst` costs a rescan, a + * missing registry means the checkpoint silently has no index an operator created, with the + * posting files sitting beside it as orphans for the next sweep to delete. So the registry + * travels, and it travels *whole* — it is written under a temporary name, forced and moved, the + * same treatment it gets in a live store. + * + * A store that has never defined an index has no registry, and this then writes nothing at all — + * which is the right answer and not a failure. + */ + @RaboshExperimental + public fun copyRegistryTo(target: Path) { + checkOpen() + val contents = lock.withLock { registry } + if (contents.indexes.isEmpty() && !Files.exists(directory.resolve(registryFileName()))) return + IndexRegistry.write(target, contents) + } + public fun dropIndex(handle: IndexHandle) { checkOpen() val updated = lock.withLock { diff --git a/rabosh-jsonpath/CLAUDE.md b/rabosh-jsonpath/CLAUDE.md index 6751ec1..94ab773 100644 --- a/rabosh-jsonpath/CLAUDE.md +++ b/rabosh-jsonpath/CLAUDE.md @@ -52,12 +52,41 @@ So `IRegexp.compileOrNull` answers `null` — for a syntax error, and equally fo run — and nothing surfaces the reason. A literal pattern is still compiled while the query is, so applying a compiled query touches no grammar at all. -**The walk carries no budget and the query carries two.** A bound on the walk truncates a nodelist, -which is a wrong answer with nothing to say so; a bound on what the caller wrote costs no answer at -all. So the limits are 1024 selectors and 64 levels of nesting, both checked while parsing, and the -descendant walk is iterative over an explicit stack — a `Variant` built through `VariantBuilder` is -never re-checked against `DEFAULT_MAX_JSON_DEPTH`, so a recursive walk would be a stack overflow -reachable from data. `JsonPathQueryTest` builds a 20 000-deep document to say so. +**The walk carries no budget that *truncates*, and three that *refuse*.** The original rule said the +walk carried no budget at all, and the reasoning behind it is unchanged and still governs: a bound +that stopped the walk and returned what it had would be a wrong answer with nothing to say so, and a +caller cannot tell a truncated nodelist from a small document. What phase 24 added is the opposite +mechanism — `JsonPathLimits` counts the work and **throws** `JsonPathLimitExceededException`, so the +caller gets no nodelist rather than a short one. Keep the two apart in any change here: a `NodeSink` +answering `false` has *learned* the answer and is declining more of it; a budget being met means the +answer is unknown. Making the budget return `false` would produce exactly the truncation the first +sentence forbids, and it would look like a simplification. + +Three bounds, all counted in steps and never on a clock, for the reason the regex bound is — a +wall-clock budget makes the failure depend on the machine. `maxNodesVisited` counts node *touches* +rather than distinct nodes, because it bounds work and `$..*..*` buys the same node once per stage; +counting distinct nodes needs a set of every node visited, which is the memory the attacker wanted +you to spend. The counters live on `Evaluation`, created per call, never on the query — a counter on +the query would break the "any number of threads at once" promise silently, by having two documents +share a budget. + +**The defaults are a backstop, not a policy, and the fixtures that say so must keep passing.** +`JsonPathQueryTest`'s 20 000-deep document and 5 000-wide array run under the shipped defaults +untouched, and so do all 703 compliance cases — a limit that changed a compliant answer would have +broken the module's only claim. A deployment actually serving hostile expressions sets its own, far +lower. `JsonPathLimitsTest` pins the numbers so that lowering one is a visible decision. + +The attack fixture is worth understanding before it is changed: `$..*..nope` names a field the +document does not have, so its answer is the **empty nodelist** and it is by its result +indistinguishable from `$.absent` — nothing measured on the answer can see it coming, which is why +the bound has to be on the work. `$..*..*` is quadratic too and is caught by `maxNodesProduced` +first, which is correct and would have left `maxNodesVisited` untested by its own case. + +The two *query* bounds are unchanged and are a different kind of thing — 1024 selectors and 64 levels +of nesting, checked while parsing, bounding what the caller wrote rather than what it costs. And the +descendant walk is still iterative over an explicit stack: a `Variant` built through `VariantBuilder` +is never re-checked against `DEFAULT_MAX_JSON_DEPTH`, so a recursive walk would be a stack overflow +reachable from data. ## Three findings worth keeping diff --git a/rabosh-jsonpath/api/rabosh-jsonpath.api b/rabosh-jsonpath/api/rabosh-jsonpath.api index 651dfb7..4ae8ada 100644 --- a/rabosh-jsonpath/api/rabosh-jsonpath.api +++ b/rabosh-jsonpath/api/rabosh-jsonpath.api @@ -1,11 +1,53 @@ +public final class app/oreshkov/rabosh/jsonpath/JsonPathLimit : java/lang/Enum { + public static final field DESCENDANT_DEPTH Lapp/oreshkov/rabosh/jsonpath/JsonPathLimit; + public static final field NODES_PRODUCED Lapp/oreshkov/rabosh/jsonpath/JsonPathLimit; + public static final field NODES_VISITED Lapp/oreshkov/rabosh/jsonpath/JsonPathLimit; + public static fun getEntries ()Lkotlin/enums/EnumEntries; + public static fun valueOf (Ljava/lang/String;)Lapp/oreshkov/rabosh/jsonpath/JsonPathLimit; + public static fun values ()[Lapp/oreshkov/rabosh/jsonpath/JsonPathLimit; +} + +public final class app/oreshkov/rabosh/jsonpath/JsonPathLimitExceededException : java/lang/RuntimeException { + public final fun getAllowed ()J + public final fun getLimit ()Lapp/oreshkov/rabosh/jsonpath/JsonPathLimit; +} + +public final class app/oreshkov/rabosh/jsonpath/JsonPathLimits { + public static final field Companion Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits$Companion; + public static final field DEFAULT_MAX_DESCENDANT_DEPTH I + public static final field DEFAULT_MAX_NODES_PRODUCED J + public static final field DEFAULT_MAX_NODES_VISITED J + public fun ()V + public fun (JJI)V + public synthetic fun (JJIILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun getMaxDescendantDepth ()I + public final fun getMaxNodesProduced ()J + public final fun getMaxNodesVisited ()J + public fun toString ()Ljava/lang/String; +} + +public final class app/oreshkov/rabosh/jsonpath/JsonPathLimits$Companion { + public final fun getDEFAULT ()Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits; + public final fun getNONE ()Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits; +} + public final class app/oreshkov/rabosh/jsonpath/JsonPathQuery { public static final field Companion Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery$Companion; + public static final fun compile (Ljava/lang/String;)Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery; + public static final fun compile (Ljava/lang/String;Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits;)Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery; + public final fun forEachNodeIn (Lapp/oreshkov/rabosh/variant/Variant;Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits;Lkotlin/jvm/functions/Function1;)V public final fun forEachNodeIn (Lapp/oreshkov/rabosh/variant/Variant;Lkotlin/jvm/functions/Function1;)V + public static synthetic fun forEachNodeIn$default (Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery;Lapp/oreshkov/rabosh/variant/Variant;Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)V + public final fun getLimits ()Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits; public final fun nodesIn (Lapp/oreshkov/rabosh/variant/Variant;)Ljava/util/List; + public final fun nodesIn (Lapp/oreshkov/rabosh/variant/Variant;Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits;)Ljava/util/List; + public static synthetic fun nodesIn$default (Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery;Lapp/oreshkov/rabosh/variant/Variant;Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits;ILjava/lang/Object;)Ljava/util/List; public fun toString ()Ljava/lang/String; } public final class app/oreshkov/rabosh/jsonpath/JsonPathQuery$Companion { public final fun compile (Ljava/lang/String;)Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery; + public final fun compile (Ljava/lang/String;Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits;)Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery; + public static synthetic fun compile$default (Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery$Companion;Ljava/lang/String;Lapp/oreshkov/rabosh/jsonpath/JsonPathLimits;ILjava/lang/Object;)Lapp/oreshkov/rabosh/jsonpath/JsonPathQuery; } diff --git a/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathEvaluator.kt b/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathEvaluator.kt index cb12291..d6000a4 100644 --- a/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathEvaluator.kt +++ b/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathEvaluator.kt @@ -8,7 +8,7 @@ import app.oreshkov.rabosh.variant.VariantPathStep // Applying a compiled query to one document. // -// Three decisions shape this file, and each is here rather than in the KDoc because a reader who +// Four decisions shape this file, and each is here rather than in the KDoc because a reader who // changes one of them will be looking at the code. // // **The composition is by sink, so a frame is per *segment* and never per document level.** @@ -26,6 +26,14 @@ import app.oreshkov.rabosh.variant.VariantPathStep // **A sink answers `false` to stop.** Existence tests and `value()` need one node and two nodes // respectively, and a filter that walked an entire subtree to learn what its first node already said // would make `$[?@..x]` cost the document rather than the answer. Every loop below honours it. +// +// **The budget throws; it never returns early.** `Evaluation` counts the work and raises +// `JsonPathLimitExceededException` when a bound is met — which is emphatically *not* the same +// mechanism as the `false` above, and the two must not be conflated. A sink answering `false` has +// learned the answer and is declining more of it; a budget being met means the answer is unknown. +// Making the budget stop the walk instead would produce exactly the truncated nodelist the second +// paragraph exists to rule out. That is why `stop()` throws rather than returning a `Boolean`, and +// why nothing below catches it. /** * Where a node is, as a link to its parent rather than as a path. @@ -66,10 +74,58 @@ internal fun interface NodeSink { fun emit(value: Variant, location: NodeLocation): Boolean } +/** + * One application of one query to one document: the root `$` resolves against, and the budget. + * + * **Per call, never per query.** `JsonPathQuery` promises that one instance may be applied from any + * number of threads at once, and a counter living on the query would be the first thing to break + * that — silently, by having two documents share a budget. The limits are immutable and live on the + * query; the counters live here and are created by `forEachNodeIn`. + * + * It replaces the bare `root: Variant` that used to be threaded through this file rather than being + * added beside it, because the two travel together everywhere and one parameter reads better than + * two. `root` is still spelled `root`. + */ +internal class Evaluation(val root: Variant, private val limits: JsonPathLimits) { + private var visited = 0L + private var produced = 0L + + /** + * One node touched. + * + * **Touches, not distinct nodes.** A node reached by two segments is counted twice, because this + * bounds the work an expression buys and `$..*..*` buys the same node once per stage. Counting + * distinct nodes would need a set of every node visited — which is itself the memory an attacker + * is trying to make you spend. + */ + fun visit() { + if (limits.maxNodesVisited > 0 && ++visited > limits.maxNodesVisited) { + stop(JsonPathLimit.NODES_VISITED, limits.maxNodesVisited) + } + } + + /** One node handed to the caller's sink. */ + fun produce() { + if (limits.maxNodesProduced > 0 && ++produced > limits.maxNodesProduced) { + stop(JsonPathLimit.NODES_PRODUCED, limits.maxNodesProduced) + } + } + + /** A descendant expansion has reached [depth] levels below where it started. */ + fun descended(depth: Int) { + if (limits.maxDescendantDepth > 0 && depth > limits.maxDescendantDepth) { + stop(JsonPathLimit.DESCENDANT_DEPTH, limits.maxDescendantDepth.toLong()) + } + } + + private fun stop(limit: JsonPathLimit, allowed: Long): Nothing = + throw JsonPathLimitExceededException(limit, allowed) +} + /** * Applies [segments] from [index] onwards, streaming into [sink]. * - * @param root the document, which `$` inside a filter resolves against however deep the walk is. + * @param context the document `$` resolves against however deep the walk is, and the budget. * @return `false` if the sink asked to stop. */ internal fun applySegments( @@ -77,12 +133,12 @@ internal fun applySegments( index: Int, value: Variant, location: NodeLocation, - root: Variant, + context: Evaluation, sink: NodeSink, ): Boolean { if (index == segments.size) return sink.emit(value, location) - return applySegment(segments[index], value, location, root) { next, at -> - applySegments(segments, index + 1, next, at, root, sink) + return applySegment(segments[index], value, location, context) { next, at -> + applySegments(segments, index + 1, next, at, context, sink) } } @@ -90,22 +146,22 @@ private fun applySegment( segment: Segment, value: Variant, location: NodeLocation, - root: Variant, + context: Evaluation, out: NodeSink, ): Boolean = when (segment) { - is Segment.Child -> applySelectors(segment.selectors, value, location, root, out) - is Segment.Descendant -> descend(segment.selectors, value, location, root, out) + is Segment.Child -> applySelectors(segment.selectors, value, location, context, out) + is Segment.Descendant -> descend(segment.selectors, value, location, context, out) } private fun applySelectors( selectors: List, value: Variant, location: NodeLocation, - root: Variant, + context: Evaluation, out: NodeSink, ): Boolean { for (selector in selectors) { - if (!applySelector(selector, value, location, root, out)) return false + if (!applySelector(selector, value, location, context, out)) return false } return true } @@ -115,39 +171,54 @@ private fun applySelectors( * * Children are pushed in reverse so that popping yields document order, which is what makes * `$..a`'s nodelist the one RFC 9535 §2.5.2.2 describes rather than a permutation of it. + * + * This is where a `..` costs the subtree rather than the answer, so it is where the budget is spent: + * every node popped is one touch, and its distance below the node the expansion started at is what + * `maxDescendantDepth` bounds. The depth travels on the [Visit] rather than being derived from the + * location, because a location is shared with its parent and knows how deep it is in the *document*, + * which is a different number once a descendant segment is not the first. */ private fun descend( selectors: List, value: Variant, location: NodeLocation, - root: Variant, + context: Evaluation, out: NodeSink, ): Boolean { val pending = ArrayDeque() - pending.addLast(Visit(value, location)) + pending.addLast(Visit(value, location, depth = 0)) while (pending.isNotEmpty()) { val visit = pending.removeLast() - if (!applySelectors(selectors, visit.value, visit.location, root, out)) return false + context.visit() + context.descended(visit.depth) + if (!applySelectors(selectors, visit.value, visit.location, context, out)) return false pushChildren(visit, pending) } return true } -private class Visit(val value: Variant, val location: NodeLocation) +private class Visit(val value: Variant, val location: NodeLocation, val depth: Int) private fun pushChildren(visit: Visit, pending: ArrayDeque) { val value = visit.value + val depth = visit.depth + 1 when (value.basicType) { VariantBasicType.ARRAY -> { for (index in value.elementCount - 1 downTo 0) { - pending.addLast(Visit(value.element(index), visit.location.child(VariantPathStep.Index(index)))) + pending.addLast( + Visit(value.element(index), visit.location.child(VariantPathStep.Index(index)), depth), + ) } } VariantBasicType.OBJECT -> { for (index in value.fieldCount - 1 downTo 0) { pending.addLast( - Visit(value.fieldValue(index), visit.location.child(VariantPathStep.Field(value.fieldName(index)))), + Visit( + value.fieldValue(index), + visit.location.child(VariantPathStep.Field(value.fieldName(index))), + depth, + ), ) } } @@ -160,28 +231,34 @@ private fun applySelector( selector: Selector, value: Variant, location: NodeLocation, - root: Variant, + context: Evaluation, out: NodeSink, ): Boolean = when (selector) { is Selector.Name -> { val child = if (value.basicType == VariantBasicType.OBJECT) value.field(selector.name) else null - child == null || out.emit(child, location.child(VariantPathStep.Field(selector.name))) + if (child == null) { + true + } else { + context.visit() + out.emit(child, location.child(VariantPathStep.Field(selector.name))) + } } - Selector.Wildcard -> applyWildcard(value, location, out) + Selector.Wildcard -> applyWildcard(value, location, context, out) - is Selector.Index -> applyIndex(selector.index, value, location, out) + is Selector.Index -> applyIndex(selector.index, value, location, context, out) - is Selector.Slice -> applySlice(selector, value, location, out) + is Selector.Slice -> applySlice(selector, value, location, context, out) - is Selector.Filter -> applyFilter(selector.expression, value, location, root, out) + is Selector.Filter -> applyFilter(selector.expression, value, location, context, out) } -private fun applyWildcard(value: Variant, location: NodeLocation, out: NodeSink): Boolean { +private fun applyWildcard(value: Variant, location: NodeLocation, context: Evaluation, out: NodeSink): Boolean { when (value.basicType) { VariantBasicType.ARRAY -> { val count = value.elementCount for (index in 0 until count) { + context.visit() if (!out.emit(value.element(index), location.child(VariantPathStep.Index(index)))) return false } } @@ -189,6 +266,7 @@ private fun applyWildcard(value: Variant, location: NodeLocation, out: NodeSink) VariantBasicType.OBJECT -> { val count = value.fieldCount for (index in 0 until count) { + context.visit() val at = location.child(VariantPathStep.Field(value.fieldName(index))) if (!out.emit(value.fieldValue(index), at)) return false } @@ -206,12 +284,19 @@ private fun applyWildcard(value: Variant, location: NodeLocation, out: NodeSink) * hold is indexed by an `Int`, so an index outside it simply selects nothing. That is an answer — * `$[9007199254740991]` is a perfectly good query over a two-element array — and not an overflow. */ -private fun applyIndex(index: Long, value: Variant, location: NodeLocation, out: NodeSink): Boolean { +private fun applyIndex( + index: Long, + value: Variant, + location: NodeLocation, + context: Evaluation, + out: NodeSink, +): Boolean { if (value.basicType != VariantBasicType.ARRAY) return true val count = value.elementCount val resolved = if (index >= 0) index else count + index if (resolved < 0 || resolved >= count) return true val at = resolved.toInt() + context.visit() return out.emit(value.element(at), location.child(VariantPathStep.Index(at))) } @@ -223,7 +308,13 @@ private fun applyIndex(index: Long, value: Variant, location: NodeLocation, out: * that only fires on a query nobody writes twice. A zero step selects nothing, which the RFC states * and which is *not* the same as a step of one. */ -private fun applySlice(slice: Selector.Slice, value: Variant, location: NodeLocation, out: NodeSink): Boolean { +private fun applySlice( + slice: Selector.Slice, + value: Variant, + location: NodeLocation, + context: Evaluation, + out: NodeSink, +): Boolean { if (value.basicType != VariantBasicType.ARRAY) return true val length = value.elementCount.toLong() val step = slice.step ?: 1L @@ -234,6 +325,7 @@ private fun applySlice(slice: Selector.Slice, value: Variant, location: NodeLoca val upper = normalise(slice.end ?: length, length).coerceIn(0L, length) var at = lower while (at < upper) { + context.visit() if (!out.emit(value.element(at.toInt()), location.child(VariantPathStep.Index(at.toInt())))) return false at += step } @@ -242,6 +334,7 @@ private fun applySlice(slice: Selector.Slice, value: Variant, location: NodeLoca val lower = normalise(slice.end ?: (-length - 1), length).coerceIn(-1L, length - 1) var at = upper while (lower < at) { + context.visit() if (!out.emit(value.element(at.toInt()), location.child(VariantPathStep.Index(at.toInt())))) return false at += step } @@ -261,7 +354,7 @@ private fun applyFilter( expression: FilterExpression, value: Variant, location: NodeLocation, - root: Variant, + context: Evaluation, out: NodeSink, ): Boolean { when (value.basicType) { @@ -269,7 +362,11 @@ private fun applyFilter( val count = value.elementCount for (index in 0 until count) { val element = value.element(index) - if (!testFilter(expression, element, root)) continue + // Counted before the test rather than after it: a candidate that fails is work the + // expression bought, and a filter over a large array whose every element is rejected + // is exactly the shape a budget exists to notice. + context.visit() + if (!testFilter(expression, element, context)) continue if (!out.emit(element, location.child(VariantPathStep.Index(index)))) return false } } @@ -278,7 +375,8 @@ private fun applyFilter( val count = value.fieldCount for (index in 0 until count) { val member = value.fieldValue(index) - if (!testFilter(expression, member, root)) continue + context.visit() + if (!testFilter(expression, member, context)) continue val at = location.child(VariantPathStep.Field(value.fieldName(index))) if (!out.emit(member, at)) return false } @@ -290,25 +388,25 @@ private fun applyFilter( } /** Evaluates a `logical-expr` against one candidate node. Never throws for a shape it did not expect. */ -internal fun testFilter(expression: FilterExpression, current: Variant, root: Variant): Boolean = +internal fun testFilter(expression: FilterExpression, current: Variant, context: Evaluation): Boolean = when (expression) { - is FilterExpression.Or -> expression.operands.any { testFilter(it, current, root) } - is FilterExpression.And -> expression.operands.all { testFilter(it, current, root) } - is FilterExpression.Not -> !testFilter(expression.operand, current, root) - is FilterExpression.Existence -> hasNode(expression.query, current, root) - is FilterExpression.Call -> testCall(expression.call, current, root) + is FilterExpression.Or -> expression.operands.any { testFilter(it, current, context) } + is FilterExpression.And -> expression.operands.all { testFilter(it, current, context) } + is FilterExpression.Not -> !testFilter(expression.operand, current, context) + is FilterExpression.Existence -> hasNode(expression.query, current, context) + is FilterExpression.Call -> testCall(expression.call, current, context) is FilterExpression.Comparison -> compareValues( - valueOf(expression.left, current, root), + valueOf(expression.left, current, context), expression.operator, - valueOf(expression.right, current, root), + valueOf(expression.right, current, context), ) } /** The two functions whose declared result is `LogicalType`. Nothing else reaches a logical position. */ -private fun testCall(call: FunctionCall, current: Variant, root: Variant): Boolean = when (call.function) { - JsonPathFunction.MATCH -> testPattern(call, current, root, anchored = true) - JsonPathFunction.SEARCH -> testPattern(call, current, root, anchored = false) +private fun testCall(call: FunctionCall, current: Variant, context: Evaluation): Boolean = when (call.function) { + JsonPathFunction.MATCH -> testPattern(call, current, context, anchored = true) + JsonPathFunction.SEARCH -> testPattern(call, current, context, anchored = false) // A ValueType function in a logical position is rejected while parsing — `$[?length(@.a)]` is one // of the 247 invalid selectors — and no registered function returns NodesType. Stated rather than @@ -325,8 +423,8 @@ private fun testCall(call: FunctionCall, current: Variant, root: Variant): Boole * three the same answer, and giving any of them an exception would make a filter over a corpus fail * on the one document whose field holds a number. */ -private fun testPattern(call: FunctionCall, current: Variant, root: Variant, anchored: Boolean): Boolean { - val subject = stringOf(valueArgument(call, 0, current, root)) ?: return false +private fun testPattern(call: FunctionCall, current: Variant, context: Evaluation, anchored: Boolean): Boolean { + val subject = stringOf(valueArgument(call, 0, current, context)) ?: return false val regexp = when (val pattern = call.pattern) { // Compiled once, when the query was compiled. is PatternSource.Fixed -> pattern.regexp @@ -334,7 +432,7 @@ private fun testPattern(call: FunctionCall, current: Variant, root: Variant, anc // The pattern is a value of the document, so it is read and compiled for this node. No memo: // a cache would be the first mutable state in a class that promises immutability, and // nothing has measured the compile against the walk it sits inside. - PatternSource.PerNode, null -> stringOf(valueArgument(call, 1, current, root))?.let(IRegexp::compileOrNull) + PatternSource.PerNode, null -> stringOf(valueArgument(call, 1, current, context))?.let(IRegexp::compileOrNull) } ?: return false return if (anchored) regexp.matches(subject) else regexp.search(subject) } @@ -346,29 +444,29 @@ private fun stringOf(value: FilterValue): String? { } /** Whether [query] selects at least one node. Stops at the first, which is what the sink is for. */ -private fun hasNode(query: QueryExpression, current: Variant, root: Variant): Boolean { +private fun hasNode(query: QueryExpression, current: Variant, context: Evaluation): Boolean { var found = false - applySegments(query.segments, 0, startOf(query.root, current, root), NodeLocation.ROOT, root) { _, _ -> + applySegments(query.segments, 0, startOf(query.root, current, context), NodeLocation.ROOT, context) { _, _ -> found = true false } return found } -private fun startOf(queryRoot: QueryRoot, current: Variant, root: Variant): Variant = when (queryRoot) { - QueryRoot.ROOT -> root +private fun startOf(queryRoot: QueryRoot, current: Variant, context: Evaluation): Variant = when (queryRoot) { + QueryRoot.ROOT -> context.root QueryRoot.CURRENT -> current } /** The `ValueType` a comparison operand carries for this candidate node. */ -private fun valueOf(comparable: ComparableExpression, current: Variant, root: Variant): FilterValue = +private fun valueOf(comparable: ComparableExpression, current: Variant, context: Evaluation): FilterValue = when (comparable) { is ComparableExpression.Literal -> FilterValue.Node(comparable.value) is ComparableExpression.Singular -> - resolve(comparable.query, current, root)?.let { FilterValue.Node(it) } ?: FilterValue.Absent + resolve(comparable.query, current, context)?.let { FilterValue.Node(it) } ?: FilterValue.Absent - is ComparableExpression.Call -> evaluateCall(comparable.call, current, root) + is ComparableExpression.Call -> evaluateCall(comparable.call, current, context) } /** @@ -378,9 +476,10 @@ private fun valueOf(comparable: ComparableExpression, current: Variant, root: Va * `Variant.select` with negative indices added — no nodelist, no sink, no location. A comparison * runs once per candidate node, so this is the hottest path in the module. */ -private fun resolve(query: SingularQuery, current: Variant, root: Variant): Variant? { - var value = startOf(query.root, current, root) +private fun resolve(query: SingularQuery, current: Variant, context: Evaluation): Variant? { + var value = startOf(query.root, current, context) for (step in query.steps) { + context.visit() val next = when (step) { is SingularStep.Name -> if (value.basicType == VariantBasicType.OBJECT) value.field(step.name) else null @@ -398,11 +497,11 @@ private fun resolve(query: SingularQuery, current: Variant, root: Variant): Vari return value } -private fun evaluateCall(call: FunctionCall, current: Variant, root: Variant): FilterValue = +private fun evaluateCall(call: FunctionCall, current: Variant, context: Evaluation): FilterValue = when (call.function) { - JsonPathFunction.LENGTH -> lengthOf(valueArgument(call, 0, current, root)) - JsonPathFunction.COUNT -> FilterValue.Integral(countNodes(nodesArgument(call, 0), current, root)) - JsonPathFunction.VALUE -> singleNode(nodesArgument(call, 0), current, root) + JsonPathFunction.LENGTH -> lengthOf(valueArgument(call, 0, current, context)) + JsonPathFunction.COUNT -> FilterValue.Integral(countNodes(nodesArgument(call, 0), current, context)) + JsonPathFunction.VALUE -> singleNode(nodesArgument(call, 0), current, context) // Both are LogicalType, and a LogicalType function is never a comparison operand: the parser // reports `match(…) == true` as one of the invalid selectors rather than compiling it. @@ -410,8 +509,8 @@ private fun evaluateCall(call: FunctionCall, current: Variant, root: Variant): F error("'${call.function.spelling}' returns LogicalType and is not a comparison operand") } -private fun valueArgument(call: FunctionCall, index: Int, current: Variant, root: Variant): FilterValue = - valueOf((call.arguments[index] as FunctionArgument.Value).comparable, current, root) +private fun valueArgument(call: FunctionCall, index: Int, current: Variant, context: Evaluation): FilterValue = + valueOf((call.arguments[index] as FunctionArgument.Value).comparable, current, context) private fun nodesArgument(call: FunctionCall, index: Int): QueryExpression = (call.arguments[index] as FunctionArgument.Nodes).query @@ -437,9 +536,9 @@ private fun lengthOf(value: FilterValue): FilterValue { private fun codePointLength(text: String): Long = text.codePointCount(0, text.length).toLong() -private fun countNodes(query: QueryExpression, current: Variant, root: Variant): Long { +private fun countNodes(query: QueryExpression, current: Variant, context: Evaluation): Long { var count = 0L - applySegments(query.segments, 0, startOf(query.root, current, root), NodeLocation.ROOT, root) { _, _ -> + applySegments(query.segments, 0, startOf(query.root, current, context), NodeLocation.ROOT, context) { _, _ -> count++ true } @@ -452,10 +551,10 @@ private fun countNodes(query: QueryExpression, current: Variant, root: Variant): * Stops after the second node, because "more than one" is all the rule needs and a nodelist over a * large document is not worth counting to answer it. */ -private fun singleNode(query: QueryExpression, current: Variant, root: Variant): FilterValue { +private fun singleNode(query: QueryExpression, current: Variant, context: Evaluation): FilterValue { var first: Variant? = null var several = false - applySegments(query.segments, 0, startOf(query.root, current, root), NodeLocation.ROOT, root) { value, _ -> + applySegments(query.segments, 0, startOf(query.root, current, context), NodeLocation.ROOT, context) { value, _ -> if (first == null) { first = value true diff --git a/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathLimits.kt b/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathLimits.kt new file mode 100644 index 0000000..29f0600 --- /dev/null +++ b/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathLimits.kt @@ -0,0 +1,146 @@ +package app.oreshkov.rabosh.jsonpath + +/** + * What a single evaluation is allowed to cost, before it is abandoned. + * + * **This is a bound that *refuses*, never one that truncates**, and the distinction is the whole + * design. A budget that stopped early and returned what it had would be a wrong answer with nothing + * to say so — which is why [JsonPathQuery.forEachNodeIn] still carries no such thing, and must not + * acquire one. Exceeding a limit here raises [JsonPathLimitExceededException]: the caller learns that + * the query was too expensive, rather than quietly receiving fewer nodes than the document holds. + * + * **What this is for.** `JsonPathQuery.compile` already refuses a query that is too *large* — 1024 + * selectors, 64 levels of nesting — and the I-Regexp matcher is a Thompson construction precisely + * because RFC 9535 lets a `match` pattern come from the document. Neither bounds what a *small, + * valid* query costs against a *large* document: `$..*..*` is eleven characters and is quadratic in + * the document's node count, and a filter applied to every node of a descendant expansion is the + * same shape. Where the expression is supplied by someone you do not trust — which is this module's + * chosen use case — that gap is the whole attack. + * + * **Counted in steps, never on a clock**, for the reason the regex bound is: a wall-clock budget + * makes the failure depend on the machine, so the same query would be rejected on a loaded CI runner + * and accepted on a developer's laptop. Every number here is a count of work the evaluator does. + * + * **The defaults are a backstop, not a policy.** They are set so that no honest query over a + * document this engine can hold will meet them — the module's own fixtures walk a 20 000-deep + * document and a 5 000-wide array well inside them — which means a deployment that actually runs + * hostile expressions should set its own, far tighter, and size them against the documents it holds. + * [NONE] turns them off for a caller who has established trust some other way. + * + * ```kotlin + * // A public endpoint compiling whatever it is handed. + * val limits = JsonPathLimits(maxNodesVisited = 50_000, maxNodesProduced = 1_000, maxDescendantDepth = 32) + * val query = JsonPathQuery.compile(untrusted, limits) + * + * val nodes = try { + * query.nodesIn(document) + * } catch (rejected: JsonPathLimitExceededException) { + * respondTooExpensive(rejected.limit) // never a partial nodelist + * } + * ``` + * + * Immutable, and safe to share: the *limits* live on the query, the *counters* do not. Each call to + * `forEachNodeIn` or `nodesIn` starts its own, which is what keeps one instance applicable to any + * number of documents from any number of threads at once. + * + * @property maxNodesVisited node-touches allowed in one evaluation, across the whole query including + * the sub-walks a filter runs. Not distinct nodes: a node reached twice by two segments costs + * twice, because this bounds *work* and work is what an attacker buys. `0` or less means no bound. + * @property maxNodesProduced nodes the caller's sink may be handed. A query whose answer is genuinely + * enormous is refused rather than delivered, which is what a caller materialising with `nodesIn` + * needs. `0` or less means no bound. + * @property maxDescendantDepth levels a `..` expansion may descend below the node it started at. + * Bounds the location chain each node carries, and with it the cost of naming one. `0` or less + * means no bound. + */ +public class JsonPathLimits( + public val maxNodesVisited: Long = DEFAULT_MAX_NODES_VISITED, + public val maxNodesProduced: Long = DEFAULT_MAX_NODES_PRODUCED, + public val maxDescendantDepth: Int = DEFAULT_MAX_DESCENDANT_DEPTH, +) { + override fun toString(): String = + "JsonPathLimits(visited=${describe(maxNodesVisited)}, produced=${describe(maxNodesProduced)}, " + + "depth=${describe(maxDescendantDepth.toLong())})" + + private fun describe(value: Long): String = if (value <= 0) "unbounded" else value.toString() + + public companion object { + /** + * Node-touches allowed by default: ten million. + * + * Sized against the attack rather than against a typical query. `$..*..*` over a document + * with *n* nodes costs `O(n²)`, so this is met by a document of a few thousand nodes under a + * quadratic expression — while a linear walk of a document with ten million nodes is one + * this engine would struggle to hold in a `Variant` at all. + */ + public const val DEFAULT_MAX_NODES_VISITED: Long = 10_000_000L + + /** Nodes deliverable by default: one million. A materialised nodelist of that size is ~24 MiB of node objects alone. */ + public const val DEFAULT_MAX_NODES_PRODUCED: Long = 1_000_000L + + /** + * Descendant depth allowed by default: one hundred thousand. + * + * Deliberately above `JsonPathQueryTest`'s 20 000-deep fixture, which exists to prove the + * walk is iterative and would be a strange thing to then forbid. Depth is the weakest of the + * three bounds — a document's depth is already bounded by the memory it took to build — + * and it is here because the *location* of a node is a chain that long. + */ + public const val DEFAULT_MAX_DESCENDANT_DEPTH: Int = 100_000 + + /** The defaults, as a value. What [JsonPathQuery.compile] applies when asked for nothing else. */ + public val DEFAULT: JsonPathLimits = JsonPathLimits() + + /** + * No bound of any kind. + * + * For a caller whose queries are its own — the engine's own tests, a query written in source + * — where the only thing a limit could do is turn a correct answer into an exception. + */ + public val NONE: JsonPathLimits = JsonPathLimits( + maxNodesVisited = 0, + maxNodesProduced = 0, + maxDescendantDepth = 0, + ) + } +} + +/** Which bound an evaluation hit. */ +public enum class JsonPathLimit { + /** [JsonPathLimits.maxNodesVisited]. */ + NODES_VISITED, + + /** [JsonPathLimits.maxNodesProduced]. */ + NODES_PRODUCED, + + /** [JsonPathLimits.maxDescendantDepth]. */ + DESCENDANT_DEPTH, +} + +/** + * An evaluation cost more than [JsonPathLimits] allowed, and was abandoned. + * + * **Nothing was returned and nothing is partial.** A caller that catches this has no nodelist, which + * is the point: the alternative — a short answer — cannot be told apart from a document that + * genuinely holds fewer nodes. + * + * Distinct and catchable so that "this query is too expensive" can be answered differently from "this + * query is malformed" (`IllegalArgumentException`, from `compile`) and from "this document will not + * decode" (`VariantFormatException`). A caller serving untrusted expressions needs all three to be + * separable, and separating them by message is not separating them. + * + * It extends `RuntimeException` directly rather than joining a module exception hierarchy, because + * there is not one: `rabosh-jsonpath` reads no files and writes none, so it has no corruption, no + * format version and no state — the three things that make the other modules' sealed hierarchies + * worth having. + * + * @property limit which bound was hit. + * @property allowed the value of that bound. + */ +public class JsonPathLimitExceededException internal constructor( + public val limit: JsonPathLimit, + public val allowed: Long, +) : RuntimeException( + "JSONPath evaluation exceeded its ${limit.name.lowercase().replace('_', ' ')} limit of $allowed; " + + "no nodes are returned, because a truncated nodelist cannot be told from a short document", +) diff --git a/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathQuery.kt b/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathQuery.kt index bb66172..84b351a 100644 --- a/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathQuery.kt +++ b/rabosh-jsonpath/src/main/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathQuery.kt @@ -43,7 +43,19 @@ import app.oreshkov.rabosh.variant.VariantNode * * @see forEachNodeIn for the lifetime rule the results carry. */ -public class JsonPathQuery private constructor(private val text: String, private val segments: List) { +public class JsonPathQuery private constructor( + private val text: String, + private val segments: List, + /** + * What one evaluation of this query is allowed to cost. See [JsonPathLimits]. + * + * On the query rather than on the call because it belongs with the expression it bounds: a + * caller that compiled something untrusted should not have to remember to pass the limits again + * at every use. [forEachNodeIn] takes an override for the case where one document is known to be + * larger than the rest. + */ + public val limits: JsonPathLimits, +) { /** * Every node [document] holds at this query's locations, in RFC 9535's nodelist order. @@ -55,15 +67,35 @@ public class JsonPathQuery private constructor(private val text: String, private * must be copied, with `Variant.toByteArray`. [VariantNode.location] is an ordinary value and * outlives everything. * - * The walk carries no depth, breadth or path budget and must not acquire one: a truncated - * nodelist is a wrong answer with nothing to say so. The bounds are on the *query* instead, and - * [compile] applies them. + * **The walk still carries no budget that *truncates*, and must not acquire one**: a short + * nodelist is a wrong answer with nothing to say so. What it carries is a budget that + * **refuses** — [limits], or [limits] overridden here — which raises + * [JsonPathLimitExceededException] and delivers nothing rather than delivering part of an answer. + * The two are opposite mechanisms and the distinction is the whole of why this is safe: a caller + * that catches the exception knows it has no answer, where a caller handed a truncated nodelist + * cannot tell it from a small document. The bounds on the *query* — 1024 selectors, 64 levels of + * nesting — are separate again, and [compile] applies those. + * + * Nodes already handed to [sink] before a limit is met are **not** an answer and must not be + * treated as one; the exception says the evaluation was abandoned, not that it finished early. * + * @param limits overrides the query's own for this call. Pass [JsonPathLimits.NONE] to evaluate + * a trusted query over a document known to be large. + * @throws JsonPathLimitExceededException if the evaluation costs more than [limits] allows. * @throws app.oreshkov.rabosh.variant.VariantFormatException if the document's bytes do not * decode. A value the engine cannot read is reported, never skipped. */ - public fun forEachNodeIn(document: Variant, sink: (VariantNode) -> Unit) { - applySegments(segments, 0, document, NodeLocation.ROOT, document) { value, location -> + @JvmOverloads + public fun forEachNodeIn( + document: Variant, + limits: JsonPathLimits = this.limits, + sink: (VariantNode) -> Unit, + ) { + // Fresh per call. The limits are shared; the counters are not, which is what keeps one + // compiled query applicable from any number of threads at once. + val context = Evaluation(document, limits) + applySegments(segments, 0, document, NodeLocation.ROOT, context) { value, location -> + context.produce() sink(VariantNode(location.toPath(), value)) true } @@ -76,8 +108,16 @@ public class JsonPathQuery private constructor(private val text: String, private * memory where [forEachNodeIn] is `O(1)`. The nodes it holds are still views and the lifetime * rule on [forEachNodeIn] applies to them unchanged: the list outlives the call, the bytes behind * the values do not. + * + * This is the shape [JsonPathLimits.maxNodesProduced] is really for: a sink can drop what it does + * not want, and a list cannot. + * + * @throws JsonPathLimitExceededException if the evaluation costs more than [limits] allows, in + * which case no list is returned at all. */ - public fun nodesIn(document: Variant): List = buildList { forEachNodeIn(document) { add(it) } } + @JvmOverloads + public fun nodesIn(document: Variant, limits: JsonPathLimits = this.limits): List = + buildList { forEachNodeIn(document, limits) { add(it) } } /** The query as it was written. */ override fun toString(): String = text @@ -99,14 +139,26 @@ public class JsonPathQuery private constructor(private val text: String, private * bounds on what the caller wrote: at most **1024 selectors**, and at most **64** levels of * nested filters, parentheses and function calls. * + * **[limits] is a different kind of bound and bounds a different thing.** Those two say how + * large the expression may be; [JsonPathLimits] says how much one *evaluation* of it may + * cost, which is the gap a small valid query over a large document walks straight through — + * `$..*..*` is eleven characters and quadratic. Exceeding it raises + * [JsonPathLimitExceededException] rather than returning a short nodelist. The defaults are a + * backstop sized so no honest query meets them; a caller compiling expressions it does not + * trust should set its own and set them far lower. + * * **A regular expression is not one of the things this refuses.** `match` and `search` take * an RFC 9485 I-Regexp, and §2.4.6 rules that a second argument which is not one makes the * *result* `LogicalFalse` — so `$[?match(@.a, '[')]` compiles, and selects nothing. A literal * pattern is compiled here all the same, so that applying the query touches no grammar. * + * @param limits what one evaluation of the compiled query may cost. See [JsonPathLimits]. * @throws IllegalArgumentException if [query] is not a valid JSONPath query, with the * offending position, or if it exceeds either limit. */ - public fun compile(query: String): JsonPathQuery = JsonPathQuery(query, JsonPathParser(query).parse()) + @JvmStatic + @JvmOverloads + public fun compile(query: String, limits: JsonPathLimits = JsonPathLimits.DEFAULT): JsonPathQuery = + JsonPathQuery(query, JsonPathParser(query).parse(), limits) } } diff --git a/rabosh-jsonpath/src/test/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathLimitsTest.kt b/rabosh-jsonpath/src/test/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathLimitsTest.kt new file mode 100644 index 0000000..c921a19 --- /dev/null +++ b/rabosh-jsonpath/src/test/kotlin/app/oreshkov/rabosh/jsonpath/JsonPathLimitsTest.kt @@ -0,0 +1,266 @@ +package app.oreshkov.rabosh.jsonpath + +import app.oreshkov.rabosh.variant.Variant +import app.oreshkov.rabosh.variant.VariantBuilder +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +/** + * The evaluation budget: that it fires, that it fires on the right bound, and — the half that is + * easy to forget — that it does not fire on anything honest. + * + * **A limit that changed a compliant answer would have broken the module's only claim**, so the + * assertions here come in pairs throughout: every "this is refused" sits beside a "this is answered", + * usually the same query against the same document under a looser bound. A budget nothing can pass is + * as broken as a budget nothing can fail, and only one of those two shows up as a red test. + * + * The compliance suite is the other half and needs no help from this file: `JsonPathQuery.compile` + * applies [JsonPathLimits.DEFAULT], so all 703 of its cases already run with the budget on. + * `JsonPathQueryTest`'s 20 000-deep document and 5 000-wide array likewise — they are the fixtures + * that say the defaults do not reach an honest query, and they were not touched to accommodate this. + */ +class JsonPathLimitsTest { + + // --- the bound refuses; it never truncates --------------------------------------------------- + + /** + * The distinction the whole design rests on. + * + * A budget that stopped the walk and returned what it had would be indistinguishable, to a + * caller, from a document that genuinely holds three nodes. So the failure is an exception and + * `nodesIn` produces no list at all — asserted here rather than left to the KDoc, because it is + * the one behaviour a future "just return what we have" refactor would quietly reverse. + */ + @Test + fun `exceeding a limit yields an exception rather than a short nodelist`() { + val document = Variant.fromJson("""{"a":[1,2,3,4,5,6,7,8,9,10]}""") + val query = JsonPathQuery.compile("$.a[*]") + + assertEquals(10, query.nodesIn(document, JsonPathLimits.NONE).size, "the honest answer") + + val refused = assertFailsWith { + query.nodesIn(document, JsonPathLimits(maxNodesProduced = 4)) + } + assertEquals(JsonPathLimit.NODES_PRODUCED, refused.limit) + assertEquals(4, refused.allowed) + } + + /** + * Each bound reports itself, because a caller serving untrusted expressions has to answer + * differently for "too many results" than for "too much work". + */ + @Test + fun `each limit names itself when it is the one that fires`() { + val document = nest(64) + + val visited = assertFailsWith { + JsonPathQuery.compile("$..*", JsonPathLimits(maxNodesVisited = 8)).nodesIn(document) + } + assertEquals(JsonPathLimit.NODES_VISITED, visited.limit) + + val produced = assertFailsWith { + JsonPathQuery.compile("$..*", JsonPathLimits(maxNodesProduced = 8)).nodesIn(document) + } + assertEquals(JsonPathLimit.NODES_PRODUCED, produced.limit) + + val depth = assertFailsWith { + JsonPathQuery.compile("$..*", JsonPathLimits(maxDescendantDepth = 8)).nodesIn(document) + } + assertEquals(JsonPathLimit.DESCENDANT_DEPTH, depth.limit) + + // The pairing that stops all three being satisfied by the fixture simply being large: with + // the bounds off, the same query over the same document answers, and answers correctly. + // 64 nested `down` values plus the one `leaf` value. + assertEquals(65, JsonPathQuery.compile("$..*", JsonPathLimits.NONE).nodesIn(document).size) + } + + /** + * Depth is counted from where the expansion started, not from the root of the document. + * + * `$.a.b..*` descends two levels before the `..` begins, and a bound that charged those two to + * the descendant budget would make the same expansion cost different amounts depending on how + * deeply the segment before it had already reached — which is a bound on the document rather + * than on the walk. + */ + @Test + fun `descendant depth is measured below the segment that starts it`() { + val document = Variant.fromJson("""{"a":{"b":{"c":{"d":{"e":1}}}}}""") + + // From `$`, `..` reaches `e` five levels down; from `$.a.b`, three. + assertFailsWith { + JsonPathQuery.compile("$..*", JsonPathLimits(maxDescendantDepth = 3)).nodesIn(document) + } + assertEquals( + 3, + JsonPathQuery.compile("$.a.b..*", JsonPathLimits(maxDescendantDepth = 3)).nodesIn(document).size, + "the same bound, the same document, a shallower expansion", + ) + } + + // --- the attack the item exists for ---------------------------------------------------------- + + /** + * **A quadratic expression over a deep document is refused, under the shipped defaults.** + * + * `$..*..nope` is fourteen characters. The first expansion yields every node; the second walks + * the whole subtree under each of them, so over a chain of *d* nodes the work is `d²/2` — for + * the fixture below, upwards of twelve million node-touches against a default of ten million. + * This is the shape case F exists to survive: the expression comes from someone you do not + * trust and the document is yours. + * + * **The second selector names a field the document does not have, and that is the point rather + * than a convenience.** This query's answer is the *empty nodelist* — it returns nothing, reads + * nothing back to the caller, and is by its result indistinguishable from `$.absent`. So neither + * [JsonPathLimits.maxNodesProduced] nor anything else measured on the answer can see it coming, + * and only a bound on the *work* can. `$..*..*` is quadratic too and is caught by the produced + * bound first, which is correct behaviour and would have been the wrong fixture: it would leave + * `maxNodesVisited` unexercised by the one case it exists for. + * + * Asserted with the **defaults**, deliberately, rather than with a bound chosen to make it fail. + * A limit nobody's defaults reach is a feature nobody has. + */ + @Test + fun `a quadratic descendant expansion is refused by the defaults`() { + val document = nest(QUADRATIC_DEPTH) + + var produced = 0 + val refused = assertFailsWith { + JsonPathQuery.compile("$..*..nope").forEachNodeIn(document) { produced++ } + } + assertEquals(JsonPathLimit.NODES_VISITED, refused.limit) + assertEquals(JsonPathLimits.DEFAULT_MAX_NODES_VISITED, refused.allowed) + assertEquals(0, produced, "the expression returns nothing at all, which is what hides it") + + // And the other direction, on the same document: a linear walk of it is not refused, so what + // the defaults caught is the *expression* rather than the fixture being big. + assertEquals(1, JsonPathQuery.compile("$..leaf").nodesIn(document).size) + } + + /** + * A filter applied to every node of a descendant expansion is the same attack wearing a + * different selector, and the budget has to see the candidates it rejects. + * + * A filter that tests five thousand elements and selects none has done five thousand elements' + * worth of work; counting only what a filter *selects* would leave `$..[?@.nope]` free. + */ + @Test + fun `a filter is charged for the candidates it rejects`() { + val elements = (0 until 500).joinToString(",") { """{"sku":"s$it"}""" } + val document = Variant.fromJson("""{"items":[$elements]}""") + val query = "$.items[?@.sku == 'nothing-matches-this']" + + assertEquals(0, JsonPathQuery.compile(query, JsonPathLimits.NONE).nodesIn(document).size) + + val refused = assertFailsWith { + JsonPathQuery.compile(query, JsonPathLimits(maxNodesVisited = 100)).nodesIn(document) + } + assertEquals(JsonPathLimit.NODES_VISITED, refused.limit) + } + + /** The sub-walk a filter runs is inside the same budget, or `$[?@..x]` buys an unbounded walk per candidate. */ + @Test + fun `work done inside a filter counts against the same budget`() { + val document = Variant.fromJson("""{"items":[{"deep":{"a":{"b":{"c":1}}}},{"deep":{"a":{"b":{"c":2}}}}]}""") + val query = "$.items[?count(@..*) > 0]" + + assertEquals(2, JsonPathQuery.compile(query, JsonPathLimits.NONE).nodesIn(document).size) + assertFailsWith { + JsonPathQuery.compile(query, JsonPathLimits(maxNodesVisited = 6)).nodesIn(document) + } + } + + // --- the defaults do not reach an honest query ----------------------------------------------- + + /** + * The three defaults, stated as the numbers they are. + * + * Pinned so that lowering one is a visible decision rather than a tuning tweak: these are the + * only thing standing between an honest query and an exception, and a caller reading + * `INTEGRATION.md` is told what they are. + */ + @Test + fun `the shipped defaults are the documented ones`() { + val limits = JsonPathLimits.DEFAULT + assertEquals(10_000_000L, limits.maxNodesVisited) + assertEquals(1_000_000L, limits.maxNodesProduced) + assertEquals(100_000, limits.maxDescendantDepth) + + assertEquals(0L, JsonPathLimits.NONE.maxNodesVisited, "NONE must actually be unbounded") + assertTrue(JsonPathLimits.NONE.toString().contains("unbounded")) + } + + /** + * A document deeper than the engine will ingest is still walked under the defaults. + * + * The companion to `JsonPathQueryTest`'s iterative-walk fixture, from the other side: that one + * says a deep document does not overflow the stack, and this one says the budget does not then + * refuse it anyway. Together they are the claim that the defaults are a backstop rather than a + * policy. + */ + @Test + fun `the defaults admit a document deeper than ingest allows`() { + val document = nest(20_000) + val nodes = JsonPathQuery.compile("$..leaf").nodesIn(document) + + assertEquals(1, nodes.size) + assertEquals(20_001, nodes.single().location.steps.size) + } + + // --- the counters are per call ---------------------------------------------------------------- + + /** + * One compiled query, two threads, two budgets. + * + * `JsonPathQuery` promises it may be applied from any number of threads at once, and a counter + * on the query would break that *silently* — two documents sharing a budget means one of them + * fails for the other's size, intermittently and never in a test that runs one thread. The + * limits are shared and immutable; the counters are created per call. + */ + @Test + fun `two threads evaluating one query do not share a budget`() { + val document = nest(50) + // One evaluation of `$..*` here is 103 touches — 52 nodes popped by the descent, 51 children + // emitted by the wildcard — and 51 nodes. The bound sits comfortably above one run and + // comfortably below two, so a counter shared between threads is *caught* rather than merely + // being catchable. + val query = JsonPathQuery.compile("$..*", JsonPathLimits(maxNodesVisited = 150)) + + assertEquals(51, query.nodesIn(document).size, "one evaluation must fit inside the bound") + + val pool = Executors.newFixedThreadPool(THREADS) + try { + val results = (0 until THREADS).map { pool.submit { query.nodesIn(document).size } } + for (result in results) assertEquals(51, result.get(60, TimeUnit.SECONDS)) + } finally { + pool.shutdown() + } + } + + private companion object { + /** + * Deep enough that `$..*..*` costs more than [JsonPathLimits.DEFAULT_MAX_NODES_VISITED]. + * + * `d²/2` at 5 000 is 12.5 million against a default of 10 million — over it, and not by so + * much that the fixture would still fail if the counting were made twice as coarse. + */ + const val QUADRATIC_DEPTH = 5_000 + const val THREADS = 4 + + /** `{"down":{"down":{…{"leaf":7}}}}`, [depth] levels deep. Built, not parsed: the parser would refuse it. */ + fun nest(depth: Int): Variant = VariantBuilder().apply { + repeat(depth) { + startObject() + field("down") + } + startObject() + field("leaf") + appendLong(7) + endObject() + repeat(depth) { endObject() } + }.buildVariant() + } +} diff --git a/rabosh-query/api/rabosh-query.api b/rabosh-query/api/rabosh-query.api index 36c91da..3dd2351 100644 --- a/rabosh-query/api/rabosh-query.api +++ b/rabosh-query/api/rabosh-query.api @@ -16,6 +16,7 @@ public final class app/oreshkov/rabosh/query/Explain { public final fun getSegmentsIndexed ()I public final fun getSegmentsScanned ()I public final fun getSources ()Ljava/util/List; + public final fun getTypeNotes ()Ljava/util/List; public final fun getUsesIndexes ()Z public final fun render ()Ljava/lang/String; public fun toString ()Ljava/lang/String; @@ -30,6 +31,15 @@ public final class app/oreshkov/rabosh/query/ExplainSource { public fun toString ()Ljava/lang/String; } +public final class app/oreshkov/rabosh/query/ExplainTypeNote { + public final fun getDescribes ()Ljava/lang/String; + public final fun getFamily ()Ljava/lang/String; + public final fun getMismatchedFraction ()D + public final fun getMismatchedTypes ()Ljava/util/List; + public final fun getPath ()Ljava/lang/String; + public fun toString ()Ljava/lang/String; +} + public final class app/oreshkov/rabosh/query/IndexUse { public final fun getCoverage ()Lapp/oreshkov/rabosh/index/IndexCoverage; public final fun getIndex ()Lapp/oreshkov/rabosh/index/IndexHandle; diff --git a/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/DocumentMatcher.kt b/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/DocumentMatcher.kt index 72ff56c..3e51baf 100644 --- a/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/DocumentMatcher.kt +++ b/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/DocumentMatcher.kt @@ -32,7 +32,10 @@ import java.util.IdentityHashMap */ internal class DocumentMatcher(private val normal: Normal, private val options: IndexOptions) { - private val leaves: List = normal.leaves() + /** Every leaf of the tree, in order. Exposed for `Explain`, which reports on all of them and + * not only on the ones an index answered — a path with no index is where a caller has no other + * signal at all. */ + internal val leaves: List = normal.leaves() private val paths = leaves.map { it.path }.distinct() private val extractor = TermExtractor(paths, options) diff --git a/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/Explain.kt b/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/Explain.kt index e70fe70..0ef5a6a 100644 --- a/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/Explain.kt +++ b/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/Explain.kt @@ -2,6 +2,7 @@ package app.oreshkov.rabosh.query import app.oreshkov.rabosh.catalog.InferredSchema import app.oreshkov.rabosh.index.IndexHandle +import app.oreshkov.rabosh.variant.VariantKind /** * One index source of a plan, with what it would actually admit. @@ -24,6 +25,40 @@ public class ExplainSource internal constructor( "#${index.id} ${index.kind} $describes -> $candidates candidate(s), $certain certain, $estimate" } +/** + * A leaf whose predicate family disagrees with the types actually stored at its path. + * + * **A diagnostic, never a coercion.** Nothing here changes an answer, a plan or a bound. A numeric + * predicate still matches numeric values only — that is type bracketing, it is part of the query + * contract, and skipping a column whose numeric bound misses depends on it. What this adds is the + * *reason* a query returned fewer rows than a caller expected, at the one moment they are asking. + * + * The case it exists for is a third-party payload archive, where a vendor sends `"500"` in some + * documents and `500` in others. `where(path("$.status") eq 500)` then silently matches only half of + * them, and there is nothing in the result to say why — which the README's own sample calls "the part + * that surprises people". + * + * @property path the path the leaf tests. + * @property describes the leaf, as it appears in the plan. + * @property family the family the leaf's literals bracket to: `numeric`, `text` or `boolean`. + * @property mismatchedFraction how much of the observed data at [path] is outside that family, as the + * catalog's sketch reports it. An estimate over the segments the model covers, not a count. + * @property mismatchedTypes the offending types, commonest first, with each one's share. + */ +public class ExplainTypeNote internal constructor( + public val path: String, + public val describes: String, + public val family: String, + public val mismatchedFraction: Double, + public val mismatchedTypes: List, +) { + override fun toString(): String = + "$describes is a $family test, and ${percent(mismatchedFraction)} of the values at $path are " + + mismatchedTypes.joinToString(", ") + " — those are not matched, and that is not an error" + + private fun percent(fraction: Double): String = "%.1f%%".format(fraction * 100) +} + /** * How a query would be answered. * @@ -48,6 +83,14 @@ public class Explain internal constructor( * that means "push-down fired" wants the counter. */ public val projectsFromColumns: Boolean, + /** + * Leaves whose predicate family disagrees with the types stored at their path. + * + * Empty for a query whose predicates and data agree, and empty when there is no schema to + * compare against — no statistics is not the same as a statistic saying no. See + * [ExplainTypeNote]. + */ + public val typeNotes: List, private val shape: String, ) { /** Whether this plan reads any sidecar at all. */ @@ -73,6 +116,10 @@ public class Explain internal constructor( appendLine("sources, cheapest first:") for (source in sources) appendLine(" $source") } + if (typeNotes.isNotEmpty()) { + appendLine("notes:") + for (note in typeNotes) appendLine(" $note") + } } override fun toString(): String = render() @@ -104,8 +151,68 @@ public class Explain internal constructor( segmentsScanned = plan.scanned.size, scansUnflushed = plan.hasUnflushedDocuments, projectsFromColumns = plan.projection != null, + typeNotes = typeNotes(plan, schema), shape = plan.expression?.render() ?: "full scan", ) } + + /** + * Below this, a mismatch is noise rather than a finding. + * + * A single stray value at a path with a million observations is not what surprises anybody, + * and a note for it would train a reader to skim the notes. One percent is low enough to + * catch the "one producer in fifty sends a string" case the samples themselves demonstrate. + */ + private const val WORTH_REPORTING = 0.01 + + /** + * Every leaf whose family disagrees with the types observed at its path. + * + * Over **all** the plan's leaves rather than only its indexed sources, which is deliberate: + * a path with no index is exactly where a caller has no other signal at all, and the plan is + * a full scan whose result is quietly short. `DocumentMatcher` already holds the leaves, so + * this asks the object that has them rather than re-walking the tree. + * + * Returns nothing without a schema. No statistics is not a statistic saying no, and a note + * asserting agreement it never checked would be worse than silence. + */ + private fun typeNotes(plan: QueryPlan, schema: InferredSchema?): List { + if (schema == null) return emptyList() + val notes = LinkedHashMap() + for (leaf in plan.matcher.leaves) { + if (leaf.family == LeafFamily.ANY) continue + val field = schema[leaf.path] ?: continue + val observed = field.types.values.sum() + if (observed <= 0L) continue + + // `NULL` is left out of the denominator rather than counted as a mismatch. A null is + // absent from every scalar family — a numeric predicate does not match it and neither + // does a text one — so reporting it here would fire on every nullable field and say + // nothing about the surprise this note exists for. + val comparable = field.types.filterKeys { it != VariantKind.NULL } + val total = comparable.values.sum() + if (total <= 0L) continue + + val mismatched = comparable.filterKeys { it !in leaf.family.kinds } + val fraction = mismatched.values.sum().toDouble() / total + if (fraction < WORTH_REPORTING) continue + + val note = ExplainTypeNote( + path = leaf.path.toString(), + describes = leaf.toString(), + family = leaf.family.description, + mismatchedFraction = fraction, + mismatchedTypes = mismatched.entries + .sortedByDescending { it.value } + .map { (kind, count) -> + "${kind.name.lowercase()} (%.1f%%)".format(count.toDouble() / total * 100) + }, + ) + // One note per leaf, keyed by what it says: a query naming the same path twice with + // the same family would otherwise report the same sentence twice. + notes.putIfAbsent(note.describes, note) + } + return notes.values.toList() + } } } diff --git a/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/Normal.kt b/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/Normal.kt index 2144504..fff36c2 100644 --- a/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/Normal.kt +++ b/rabosh-query/src/main/kotlin/app/oreshkov/rabosh/query/Normal.kt @@ -5,6 +5,7 @@ import app.oreshkov.rabosh.index.ColumnPredicate import app.oreshkov.rabosh.index.IndexOptions import app.oreshkov.rabosh.index.IndexTerm import app.oreshkov.rabosh.variant.Variant +import app.oreshkov.rabosh.variant.VariantKind /** * Rewrites a predicate into negation-normal form and folds away what it can. @@ -122,6 +123,8 @@ internal sealed interface Normal { */ val terms: Set?, val negated: Boolean, + /** The type family these literals bracket to. See [LeafFamily]. */ + val family: LeafFamily = LeafFamily.ANY, ) : Normal { /** Whether this one value satisfies the leaf, ignoring [negated]. */ fun test(value: Variant): Boolean = predicates.any { it.matches(value) } @@ -151,6 +154,39 @@ internal sealed interface Normal { /** What a leaf asks, which is what decides whether an index kind can answer it. */ internal enum class LeafKind { EQUALITY, RANGE, EXISTS, IS_NULL } +/** + * The type family a leaf's literals belong to, and therefore the only family it can ever match. + * + * **Type bracketing is part of the query contract**, so this is not an optimisation hint: a numeric + * predicate matches numeric values only, a text predicate matches strings only, and anything else is + * not a match and not an error. That is what lets a column whose numeric bound misses be skipped even + * when the path also holds strings. + * + * It is recorded here because it is the one thing `Explain` cannot recover afterwards. + * `ColumnPredicate.kind` says the same, and it is `internal` to `rabosh-index` — so the family has to + * travel from where the literal was, which is the lowering in this file. Nothing evaluates against + * this field; `ColumnPredicate.matches` remains the only definition of what a leaf accepts, and a + * second one here would be the drift the rule warns about. + */ +internal enum class LeafFamily(val kinds: Set) { + NUMERIC(setOf(VariantKind.INTEGER, VariantKind.DECIMAL, VariantKind.FLOAT, VariantKind.DOUBLE)), + TEXT(setOf(VariantKind.STRING)), + BOOLEAN(setOf(VariantKind.BOOLEAN)), + + /** `EXISTS`, `IS NULL`, and a mixed `IN` — none of which brackets by type, so none can mismatch. */ + ANY(emptySet()), + ; + + /** How this reads in a diagnostic. */ + val description: String + get() = when (this) { + NUMERIC -> "numeric" + TEXT -> "text" + BOOLEAN -> "boolean" + ANY -> "any" + } +} + /** * Lowers a normalised predicate, giving every leaf its [ColumnPredicate]. * @@ -189,13 +225,14 @@ private fun leaf( kind: LeafKind, predicates: List, terms: Set?, -): Normal.Leaf = Normal.Leaf(path, kind, predicates, terms, negated = false) + family: LeafFamily = LeafFamily.ANY, +): Normal.Leaf = Normal.Leaf(path, kind, predicates, terms, negated = false, family = family) /** De Morgan over a lowered tree. A leaf flips its own flag; a junction swaps and pushes down. */ private fun Normal.negate(): Normal = when (this) { Normal.AlwaysTrue -> Normal.AlwaysFalse Normal.AlwaysFalse -> Normal.AlwaysTrue - is Normal.Leaf -> Normal.Leaf(path, kind, predicates, terms, negated = !negated) + is Normal.Leaf -> Normal.Leaf(path, kind, predicates, terms, negated = !negated, family = family) // The flag flips and `inner` is left alone: pushing the negation inside would turn "no element // satisfies P" into "some element satisfies not P", which is a different set of documents. is Normal.Element -> Normal.Element(path, inner, negated = !negated) @@ -216,13 +253,32 @@ private fun equality(path: CatalogPath, values: List, options: Index if (values.isEmpty()) return Normal.AlwaysFalse val predicates = values.map(::equalityPredicate) val kind = if (values.all { it == QueryValue.Null }) LeafKind.IS_NULL else LeafKind.EQUALITY + val family = familyOf(values) val terms = LinkedHashSet(values.size) for (value in values) { - val term = termOf(value) ?: return leaf(path, kind, predicates, terms = null) - if (term.size > options.maxTermBytes) return leaf(path, kind, predicates, terms = null) + val term = termOf(value) ?: return leaf(path, kind, predicates, terms = null, family = family) + if (term.size > options.maxTermBytes) return leaf(path, kind, predicates, terms = null, family = family) terms.add(term) } - return leaf(path, kind, predicates, terms) + return leaf(path, kind, predicates, terms, family) +} + +/** + * The one family every literal of an `IN` shares, or [LeafFamily.ANY] when they do not share one. + * + * A mixed `IN` brackets to nothing — `anyOf(1, "a")` matches a number *and* a string — so there is + * no family it could be said to disagree with, and reporting one would be a diagnostic that lied. + */ +private fun familyOf(values: List): LeafFamily { + val families = values.mapTo(HashSet()) { value -> + when (value) { + is QueryValue.Text -> LeafFamily.TEXT + is QueryValue.Numeric -> LeafFamily.NUMERIC + is QueryValue.Bool -> LeafFamily.BOOLEAN + QueryValue.Null -> LeafFamily.ANY + } + } + return families.singleOrNull() ?: LeafFamily.ANY } private fun equalityPredicate(value: QueryValue): ColumnPredicate = when (value) { @@ -271,7 +327,8 @@ private fun range(path: CatalogPath, operator: Comparison, value: QueryValue): N is QueryValue.Bool, QueryValue.Null -> return Normal.AlwaysFalse } - return leaf(path, LeafKind.RANGE, listOf(predicate), terms = null) + val family = if (value is QueryValue.Numeric) LeafFamily.NUMERIC else LeafFamily.TEXT + return leaf(path, LeafKind.RANGE, listOf(predicate), terms = null, family = family) } /** diff --git a/rabosh-query/src/test/kotlin/app/oreshkov/rabosh/query/ExplainTypeNoteTest.kt b/rabosh-query/src/test/kotlin/app/oreshkov/rabosh/query/ExplainTypeNoteTest.kt new file mode 100644 index 0000000..fec0958 --- /dev/null +++ b/rabosh-query/src/test/kotlin/app/oreshkov/rabosh/query/ExplainTypeNoteTest.kt @@ -0,0 +1,185 @@ +package app.oreshkov.rabosh.query + +import app.oreshkov.rabosh.catalog.SchemaCatalog +import app.oreshkov.rabosh.core.DocumentStore +import app.oreshkov.rabosh.index.CompositeSegmentObserver +import app.oreshkov.rabosh.index.IndexCatalog +import app.oreshkov.rabosh.variant.Variant +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import org.junit.jupiter.api.io.TempDir + +/** + * `explain` says when a predicate cannot match the data's types. + * + * **A diagnostic, never a coercion**, and the two assertions that matter here are on opposite sides + * of that line: the note appears, *and* the answer is unchanged. A numeric predicate still matches + * numeric values only — that is type bracketing, it is part of the query contract, and skipping a + * column whose numeric bound misses depends on it. Anything here that made `$.status eq 500` match + * `"500"` would be a second definition of `ColumnPredicate.matches` and would break skipping. + * + * The fixture is the case the item exists for: a vendor sends a field as a number in most documents + * and as a string in the rest, so a query returns fewer rows than expected with nothing in the + * result to say why. + */ +class ExplainTypeNoteTest { + + /** One document in five sends `status` as a string, the way a real second producer does. */ + private fun mixed(index: Int): Variant = jsonDocument( + if (index % 5 == 0) { + """{"status":"${200 + index % 3}","team":"team-${index % 7}"}""" + } else { + """{"status":${200 + index % 3},"team":"team-${index % 7}"}""" + }, + ) + + /** Every document agrees. Nothing here may produce a note. */ + private fun uniform(index: Int): Variant = jsonDocument( + """{"status":${200 + index % 3},"team":"team-${index % 7}"}""", + ) + + @Test + fun `a numeric predicate over a partly string path is reported`(@TempDir root: Path) { + withStore(root, ::mixed) { store, engine -> + store.snapshot().use { snapshot -> + val query = Query.where(path("$.status") eq 200L) + val explain = engine.explain(query, snapshot) + + val note = explain.typeNotes.singleOrNull() + assertTrue(note != null, "the mismatch must be reported:\n${explain.render()}") + assertEquals("$.status", note.path) + assertEquals("numeric", note.family) + assertTrue(note.mismatchedFraction > 0.15, "about a fifth of the values are strings: $note") + assertTrue(note.mismatchedFraction < 0.25, note.toString()) + assertTrue(note.mismatchedTypes.single().startsWith("string"), note.mismatchedTypes.toString()) + + // It is rendered, because a property nobody prints is a property nobody reads. + assertTrue(explain.render().contains("notes:"), explain.render()) + assertTrue(explain.render().contains("not matched"), explain.render()) + } + } + } + + /** + * The other side of the line: the note changes no answer. + * + * Asserted against a full scan through the engine's own matcher, so "unchanged" means the same + * documents rather than the same count — and the string-valued documents are still *not* matched, + * which is the contract the note describes rather than a defect it announces. + */ + @Test + fun `the note changes nothing about the answer`(@TempDir root: Path) { + withStore(root, ::mixed) { store, engine -> + store.snapshot().use { snapshot -> + val query = Query.where(path("$.status") eq 200L) + assertMatchesScan(engine, store, snapshot, query, "a reported mismatch") + + val keys = engine.keys(query, snapshot) + assertTrue(keys.isNotEmpty(), "the numeric documents still match") + // Counted from the corpus's own rule rather than from a formula: the documents that + // match are those sending a *number* that happens to be 200. The string-valued ones + // spell "200" and are not among them, which is the whole point. + val expected = (0 until 400).count { it % 5 != 0 && (200 + it % 3) == 200 } + assertEquals(expected, keys.size, "no string-valued document was coerced in") + assertTrue( + (0 until 400).any { it % 5 == 0 && "${200 + it % 3}" == "200" }, + "the fixture must actually contain a string \"200\", or the assertion above is empty", + ) + } + } + } + + /** A path whose values all agree produces no note, or the notes would be noise a reader learns to skip. */ + @Test + fun `a path whose types agree is not reported`(@TempDir root: Path) { + withStore(root, ::uniform) { store, engine -> + store.snapshot().use { snapshot -> + val explain = engine.explain(Query.where(path("$.status") eq 200L), snapshot) + assertTrue(explain.typeNotes.isEmpty(), explain.render()) + assertTrue(!explain.render().contains("notes:"), explain.render()) + } + } + } + + /** + * A leaf that brackets to nothing cannot mismatch, and must not claim to. + * + * `EXISTS` matches a value of any type, and a mixed `IN` matches two families at once. Reporting + * a disagreement for either would be a diagnostic that lied. + */ + @Test + fun `a leaf with no family is never reported`(@TempDir root: Path) { + withStore(root, ::mixed) { store, engine -> + store.snapshot().use { snapshot -> + assertTrue(engine.explain(Query.where(path("$.status").exists()), snapshot).typeNotes.isEmpty()) + assertTrue( + engine.explain(Query.where(path("$.status").oneOf(200L, "200")), snapshot).typeNotes.isEmpty(), + "a mixed IN brackets to nothing, so nothing disagrees with it", + ) + } + } + } + + /** + * Without a schema there is no note, and that is the honest answer rather than a gap. + * + * No statistics is not a statistic saying no. An engine built with no `InferredSchema` has + * nothing to compare a family against, and a note asserting agreement it never checked would be + * worse than silence. + */ + @Test + fun `no schema means no notes`(@TempDir root: Path) { + val directory = scratch(root, "noschema") + IndexCatalog(directory).use { catalog -> + DocumentStore.open(directory, queryStoreOptions(catalog)).use { store -> + catalog.attach(store) + store.load((0 until 100).map(::mixed)) + val engine = QueryEngine(store, catalog, schema = null) + store.snapshot().use { snapshot -> + val explain = engine.explain(Query.where(path("$.status") eq 200L), snapshot) + assertTrue(explain.typeNotes.isEmpty(), explain.render()) + } + } + } + } + + /** + * A leaf with no index still gets a note, which is the case a caller has no other signal for. + * + * Everything above runs without an index over `$.status` too, so this states it as its own claim + * rather than leaving it implied: the plan is a full scan, the result is quietly short, and the + * note is the only thing that says why. + */ + @Test + fun `a leaf with no index is reported too`(@TempDir root: Path) { + withStore(root, ::mixed) { store, engine -> + store.snapshot().use { snapshot -> + val explain = engine.explain(Query.where(path("$.status") eq 200L), snapshot) + assertTrue(explain.sources.isEmpty(), "no index over \$.status: ${explain.render()}") + assertEquals(1, explain.typeNotes.size, explain.render()) + } + } + } + + private fun withStore( + root: Path, + document: (Int) -> Variant, + body: (DocumentStore, QueryEngine) -> Unit, + ) { + val directory = scratch(root, "typenote") + val schema = SchemaCatalog(directory) + IndexCatalog(directory).use { indexes -> + val observer = CompositeSegmentObserver(listOf(schema, indexes)) + DocumentStore.open(directory, queryStoreOptions(indexes).withSegmentObserver(observer)).use { store -> + schema.attach(store) + indexes.attach(store) + for (round in 0 until 4) { + store.load((round * 100 until round * 100 + 100).map(document), round * 100) + } + body(store, QueryEngine(store, indexes, schema.inferSchema())) + } + } + } +} diff --git a/rabosh-samples/build.gradle.kts b/rabosh-samples/build.gradle.kts index d75a9c4..a3170b3 100644 --- a/rabosh-samples/build.gradle.kts +++ b/rabosh-samples/build.gradle.kts @@ -118,3 +118,11 @@ tasks.register("runThreeStepsOnModulePath") { val jars = modulePath jvmArgumentProviders.add(CommandLineArgumentProvider { listOf("--module-path", jars.asPath) }) } + +tasks.register("runDrain") { + group = "sample" + description = "A staging buffer drained: snapshot, ship, watermark, retire, compact — in that order." + mainClass = "app.oreshkov.rabosh.samples.DrainMain" + classpath = sourceSets["main"].runtimeClasspath + jvmArgs("--enable-native-access=ALL-UNNAMED") +} diff --git a/rabosh-samples/src/main/kotlin/app/oreshkov/rabosh/samples/DrainMain.kt b/rabosh-samples/src/main/kotlin/app/oreshkov/rabosh/samples/DrainMain.kt new file mode 100644 index 0000000..4bd8fc4 --- /dev/null +++ b/rabosh-samples/src/main/kotlin/app/oreshkov/rabosh/samples/DrainMain.kt @@ -0,0 +1,214 @@ +package app.oreshkov.rabosh.samples + +import app.oreshkov.rabosh.api.Rabosh +import app.oreshkov.rabosh.core.Key +import app.oreshkov.rabosh.core.Snapshot +import java.nio.file.Path + +/** + * **A staging buffer, drained.** Ingest, ship a batch onward, record how far you got, retire what you + * shipped — and prove nothing was lost or shipped twice. + * + * ``` + * ./gradlew :rabosh-samples:runDrain + * ``` + * + * This is the one part of the lakehouse-staging case that is pure integration: rabosh holds events + * until something downstream — a Parquet writer, an Iceberg commit, a queue — has taken them, and the + * loop that hands them over is the caller's to write. Every mistake in it is **silent**. A watermark + * advanced before the ship succeeds loses data with nothing to say so. A scan without a snapshot can + * see a compaction land underneath it and hand over a document twice or not at all. A drain that + * never compacts leaves the tombstones and grows for ever while reporting that it deleted everything. + * + * So the order is the deliverable, and it is five calls: + * + * ```kotlin + * db.snapshot().use { view -> // 1. pin the view + * val shipped = db.scan(from = watermark, snapshot = view).use { … } // 2. read from it + * ship(shipped) // 3. hand over — and only if it returns + * watermark = shipped.last().key.successor() // 4. *then* record how far + * } + * db.deleteRange(to = lastShipped) // 5. retire what was shipped + * db.compact() // and let compaction reclaim it + * ``` + * + * **Deliberately not a `DrainCursor`.** The pattern is those calls in that order; wrapping them would + * add a concept the layers below do not have, and the facade rule is that it may change ergonomics + * and never answers. What is worth showing is the *order*, which a wrapper would hide. + * + * The watermark is kept in memory here because a sample has nowhere better. In a real deployment it + * belongs wherever the downstream commit is recorded — the same transaction, ideally — because the + * one thing that must never happen is a watermark that advanced past a ship that did not. + */ +object DrainMain { + + private const val EVENT_COUNT = 6_000 + private const val BATCH_SIZE = 500 + + /** Events handed downstream per drain round. Small enough that the sample runs several rounds. */ + private const val DRAIN_BATCH = 1_500 + + @JvmStatic + fun main(arguments: Array) { + SampleRun.entryPoint(arguments, "drain", ::run) + } + + /** The sample itself. Takes a directory so the suite can run it against a temporary one. */ + fun run(directory: Path) { + Rabosh.open(directory, SampleRun.options()).use { db -> + fillTheBuffer(db) + val shipped = drainInRounds(db) + checkpointWhileWriting(db, directory) + reportWhatIsLeft(db, shipped) + } + } + + // --- 1. the buffer fills --------------------------------------------------------------------- + + private fun fillTheBuffer(db: Rabosh) { + SampleRun.heading("1.", "Ingest") + println("writing $EVENT_COUNT events into the staging buffer") + SampleRun.load(db, EVENT_COUNT, BATCH_SIZE) + println("buffered: ${db.stats.segmentCount} segment(s), ${db.stats.segmentBytes} bytes on disk") + SampleRun.note("the buffer is an ordinary store; nothing here is a queue") + SampleRun.note("keys are time-ordered, which is what makes retention a key range") + } + + // --- 2. the drain loop ------------------------------------------------------------------------ + + /** + * Drains until the buffer is empty, returning every key that was handed downstream. + * + * The returned list is what the assertions are made against: it must hold every event exactly + * once, in order, and it is built only from events the downstream actually accepted. + */ + private fun drainInRounds(db: Rabosh): List { + SampleRun.heading("2.", "Drain") + + val shipped = ArrayList(EVENT_COUNT) + var watermark: Key? = null + var round = 0 + + while (true) { + round++ + // 1. Pin the view. Everything this round reads comes from here, so a flush or a + // compaction landing mid-round cannot change what it sees. + val batch = db.snapshot().use { view -> readBatch(db, view, watermark) } + if (batch.isEmpty()) break + + // 2. Hand over. If this throws, nothing below runs: the watermark does not move and the + // events are still in the buffer, so the next attempt ships them again. At-least-once + // is the honest guarantee here, and it is the reason the order is this way round. + shipDownstream(round, batch) + + // 3. *Then* record how far. After the ship, never before. + shipped += batch.map { it.key } + watermark = batch.last().key.successor() + + // 4. Retire what was shipped, and let compaction reclaim it. `deleteRange` writes the + // tombstones; only a compaction removes the documents they hide. + val retired = db.deleteRange(to = batch.last().key) + db.compact() + println( + " round $round: shipped ${batch.size}, retired $retired, " + + "${db.stats.segmentCount} segment(s) left", + ) + } + + SampleRun.note("the watermark moves only after a successful ship, so a failure re-ships") + SampleRun.note("deleteRange writes one tombstone per key; compact() is what reclaims the space") + check(shipped.size == EVENT_COUNT) { + "every event must be shipped exactly once: ${shipped.size} of $EVENT_COUNT" + } + check(shipped == shipped.sorted()) { "events must be shipped in key order" } + check(shipped.toSet().size == shipped.size) { "no event may be shipped twice" } + println() + println("drained $EVENT_COUNT events in $round round(s), each exactly once, in key order") + return shipped + } + + /** One round's worth of events, read from the pinned view and copied out of it. */ + private fun readBatch(db: Rabosh, view: Snapshot, watermark: Key?): List { + val batch = ArrayList(DRAIN_BATCH) + db.scan(from = watermark, snapshot = view).use { cursor -> + while (batch.size < DRAIN_BATCH && cursor.next()) { + // Copied, not referenced. A document is a view over a mapped segment and is valid + // only until the next `next()` — so anything that outlives the loop must be a copy, + // which for a hand-off downstream is what you wanted anyway. + batch += Event(cursor.key, cursor.document.toByteArray()) + } + } + return batch + } + + /** + * Stands in for the thing that actually takes the events: a Parquet writer, an Iceberg commit, + * a queue. + * + * It only has to return normally to mean "these are yours now". Throwing here would leave the + * watermark where it was, which is the property the loop is arranged around. + */ + private fun shipDownstream(round: Int, batch: List) { + var bytes = 0L + for (event in batch) bytes += event.bytes.size + check(bytes > 0) { "round $round shipped no bytes" } + } + + private class Event(val key: Key, val bytes: ByteArray) + + // --- 3. a checkpoint, taken while the writer runs ---------------------------------------------- + + /** + * A consistent copy, taken without stopping. + * + * The other half of what a staging buffer needs: the recipe before `checkpoint` existed was *stop + * writing and copy the directory*, and a buffer that is being written to cannot stop. Writes that + * land during the call are simply above the checkpoint's sequence. + */ + private fun checkpointWhileWriting(db: Rabosh, directory: Path) { + SampleRun.heading("3.", "Checkpoint") + + for (index in EVENT_COUNT until EVENT_COUNT + 200) { + db.put(SampleCorpus.key(index), SampleCorpus.json(index)) + } + + val target = directory.resolveSibling("${directory.fileName}-checkpoint") + val info = db.checkpoint(target) + println("checkpoint at sequence ${info.sequence}: ${info.segmentCount} segment(s), ${info.fileCount} file(s)") + println(" ${if (info.hardLinked) "hard-linked" else "copied"}, ${info.bytes} bytes of segment data") + + // Writes that arrive after the checkpoint are above its sequence and are not in it. + for (index in EVENT_COUNT + 200 until EVENT_COUNT + 400) { + db.put(SampleCorpus.key(index), SampleCorpus.json(index)) + } + + Rabosh.open(target, SampleRun.options()).use { copy -> + val inCopy = copy.get(SampleCorpus.key(EVENT_COUNT + 100)) != null + val afterCopy = copy.get(SampleCorpus.key(EVENT_COUNT + 300)) != null + check(inCopy) { "an event written before the checkpoint must be in it" } + check(!afterCopy) { "an event written after the checkpoint must not be" } + println(" the copy opens and holds the prefix as of that sequence, and nothing after it") + } + target.toFile().deleteRecursively() + + SampleRun.note("hard links mean a checkpoint costs a directory entry per file, not its bytes") + SampleRun.note("it is a consistent view, not an off-site backup: moving it is the next step") + } + + // --- 4. what is left -------------------------------------------------------------------------- + + private fun reportWhatIsLeft(db: Rabosh, shipped: List) { + SampleRun.heading("4.", "What is left") + + db.deleteRange() + db.compact() + + var remaining = 0 + db.scan().use { cursor -> while (cursor.next()) remaining++ } + check(remaining == 0) { "the buffer should be empty, and $remaining events are left" } + + println("shipped ${shipped.size} events, retired all of them, $remaining left in the buffer") + println("on disk now: ${db.stats.segmentCount} segment(s), ${db.stats.segmentBytes} bytes") + SampleRun.note("a drain that never compacted would report the same counts and keep the bytes") + } +} diff --git a/rabosh-samples/src/test/kotlin/app/oreshkov/rabosh/samples/SamplesTest.kt b/rabosh-samples/src/test/kotlin/app/oreshkov/rabosh/samples/SamplesTest.kt index e234b57..9476f1b 100644 --- a/rabosh-samples/src/test/kotlin/app/oreshkov/rabosh/samples/SamplesTest.kt +++ b/rabosh-samples/src/test/kotlin/app/oreshkov/rabosh/samples/SamplesTest.kt @@ -87,6 +87,47 @@ class SamplesTest { assertEquals(1, rowCounts.distinct().size, "the three states must agree: $rowCounts") } + /** + * The drain sample, which is where `deleteRange` and `checkpoint` are accepted as a *caller's + * program* rather than as two methods with tests. + * + * The sample `check`s the properties only it can see — every event shipped exactly once, in key + * order, none twice — inside the program a reader is looking at, which is the phase-8 rule and + * the reason those are not repeated here. What this adds is the part no `check` inside it covers: + * that the loop actually ran more than once, that retention actually retired what it shipped, and + * that the buffer ended empty rather than merely reporting that it had. + */ + @Test + fun `the drain sample ships every event once and leaves the buffer empty`(@TempDir directory: Path) { + val output = capturingStdout { DrainMain.run(directory) } + + // More than one round, or the watermark was never exercised: a single-round drain would pass + // every assertion the sample makes about ordering while never resuming from anything. + val rounds = Regex("""^\s+round (\d+): shipped (\d+), retired (\d+),""", RegexOption.MULTILINE) + .findAll(output) + .map { Triple(it.groupValues[1].toInt(), it.groupValues[2].toInt(), it.groupValues[3].toInt()) } + .toList() + assertTrue(rounds.size > 1, "the drain must resume from a watermark at least once: $rounds") + assertEquals(rounds.indices.map { it + 1 }, rounds.map { it.first }, "rounds must be consecutive") + for ((round, shipped, retired) in rounds) { + assertTrue(shipped > 0, "round $round shipped nothing") + assertEquals(shipped, retired, "round $round retired a different number than it shipped") + } + + // The checkpoint was taken while the writer was running, and the copy opened. + assertTrue("checkpoint at sequence" in output, "the sample should report the checkpoint it took") + assertTrue( + "holds the prefix as of that sequence, and nothing after it" in output, + "the sample should have opened its own checkpoint and checked what is in it", + ) + + // And the buffer is empty at the end — asserted on the reported counts, because "deleted" and + // "reclaimed" are two different claims and only the second shows up as bytes. + val left = Regex("""retired all of them, (\d+) left in the buffer""").find(output) + assertEquals("0", left?.groupValues?.get(1), "the buffer should be empty:\n$output") + assertTrue("0 segment(s), 0 bytes" in output, "compaction should have reclaimed the space:\n$output") + } + /** * The entry point itself, not just the body. * diff --git a/rabosh-testkit/src/main/kotlin/app/oreshkov/rabosh/testkit/crash/ChildJvm.kt b/rabosh-testkit/src/main/kotlin/app/oreshkov/rabosh/testkit/crash/ChildJvm.kt index f07d8b4..d676867 100644 --- a/rabosh-testkit/src/main/kotlin/app/oreshkov/rabosh/testkit/crash/ChildJvm.kt +++ b/rabosh-testkit/src/main/kotlin/app/oreshkov/rabosh/testkit/crash/ChildJvm.kt @@ -65,6 +65,15 @@ public class ChildJvm private constructor( /** Everything the child has written to standard error. The first thing to look at on a failure. */ public val standardError: String get() = synchronized(errors) { errors.toString() } + /** + * The child's operating-system process id. + * + * For the one assertion that cannot be made any other way: that a `LOCK` file names the process + * actually holding it. A test can compare `StoreLockedException.holder?.pid` against this and + * know the record is right rather than merely well-formed. + */ + public val pid: Long get() = process.pid() + override fun close() { if (process.isAlive) killForcibly() } diff --git a/rabosh-variant/api/rabosh-variant.api b/rabosh-variant/api/rabosh-variant.api index 2be2753..776a124 100644 --- a/rabosh-variant/api/rabosh-variant.api +++ b/rabosh-variant/api/rabosh-variant.api @@ -47,6 +47,7 @@ public final class app/oreshkov/rabosh/variant/Variant { public final fun binaryValue ()[B public final fun booleanValue ()Z public final fun decimalValue ()Ljava/math/BigDecimal; + public final fun detached ()Lapp/oreshkov/rabosh/variant/Variant; public final fun doubleValue ()D public final fun element (I)Lapp/oreshkov/rabosh/variant/Variant; public final fun elements ()Ljava/util/List; diff --git a/rabosh-variant/src/main/kotlin/app/oreshkov/rabosh/variant/Variant.kt b/rabosh-variant/src/main/kotlin/app/oreshkov/rabosh/variant/Variant.kt index 06b0d63..cf7e7cb 100644 --- a/rabosh-variant/src/main/kotlin/app/oreshkov/rabosh/variant/Variant.kt +++ b/rabosh-variant/src/main/kotlin/app/oreshkov/rabosh/variant/Variant.kt @@ -117,6 +117,37 @@ public class Variant public constructor( /** Copies this value's bytes out of the segment. Pair with [VariantMetadata.toByteArray]. */ public fun toByteArray(): ByteArray = segment.bytes(offset, byteSize.toInt(), "value") + /** + * This document rebuilt with a dictionary of its own, holding only the names it actually uses. + * + * **The trap this exists to remove.** A document read out of a segment carries *that segment's* + * shared dictionary — one dictionary per segment is the single largest space saving in the + * engine — so [metadata] describes thousands of documents and not this one. Hand + * `(metadata, toByteArray())` to something expecting a self-contained Variant and it will be + * correct but enormous; hand it `toByteArray()` alone and every field name in it resolves to the + * wrong string, or to nothing. Neither failure is loud. + * + * ```kotlin + * // Writing one document into a Parquet Variant column, where the pair must stand alone: + * val standalone = row.document().detached() + * writer.write(standalone.metadata.toByteArray(), standalone.toByteArray()) + * ``` + * + * **This is not always what you want, and the alternative is cheaper.** A consumer that can take + * a *shared* dictionary — an Iceberg writer handling a whole segment's worth of rows, say — + * should be handed `variant.metadata` once and `variant.toByteArray()` per document, which + * copies no names at all and is what the engine's own layout is optimised for. Reach for this + * when the consumer wants one document, self-contained. + * + * The bytes are the Apache Parquet Variant encoding either way; what changes is only which + * dictionary the value's field ids index into. + * + * @throws VariantFormatException if this value's bytes do not decode. Copying is byte-for-byte + * for scalars, so an unknown primitive id is reported here rather than re-encoded as + * something else. + */ + public fun detached(): Variant = VariantBuilder().also { it.append(this) }.buildVariant() + // --- scalars --------------------------------------------------------------------------- /** @throws VariantTypeException unless this value is a boolean. */