Drain a buffer, copy a store while it is being written, and bound a hostile expression - #14
Merged
Conversation
…ostile expression 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tier 1: one item per remaining gap, in the order the readiness note sequences them. Four use cases
were each blocked on something specific. No format change, no new dependency, and no answer any
query returns is different.
994 tests, zero failures. ABI dumps grew by 107 lines and shrank by none.
JsonPathLimits, three bounds, counted in stepsStoreLockedExceptionread as a crashLockHoldercheckpoint(target), hard-linkeddeleteRange(from, to)ExplainTypeNote(metadata, value)split is a trapVariant.detached(),shreddingAdvice():rabosh-samples:runDrainBounded evaluation for untrusted JSONPath
The module's chosen use case is expressions you did not write, and nothing bounded what a small,
valid query cost against a large document.
$..*..nopeis fourteen characters, quadratic in thedocument'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 it is the sharpest
statement of why the bound has to be on the work.
The bound refuses; it never truncates, and this reconciles T1-6 with a rule
rabosh-jsonpathalready had. Its conventions said the walk carries no budget, because one that stopped early and
returned what it had is a wrong answer with nothing to say so. That rule is unchanged and still
governs.
JsonPathLimitExceededExceptionis the opposite mechanism: no nodelist rather than a shortone.
Evaluation.stop()throws rather than returning aBooleandeliberately — reusing thesink's own "stop" would look like a simplification while reintroducing the exact truncation the rule
forbids. Please keep those apart in review.
All 703 compliance cases and the module's 20 000-deep and 5 000-wide fixtures pass under the shipped
defaults, untouched. The defaults are a backstop, not a policy.
Checkpoint
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
CURRENTis 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
.cator.pstis and a sidecar kind added later travels for free.INDEXESisnamed rather than numbered and cannot travel that way —
IndexCatalog.copyRegistryTocarries it,because losing it would lose an instruction rather than derived data and leave the posting files as
orphans. That is why
Rabosh.checkpointexists rather than the facade delegating and stopping.The fault suite fails the copy at four steps and asserts the source is unharmed at each. Note
which step it arms: the segments are hard-linked, so no byte is written for one and a
WRITEfaultnever fires.
FORCEhappens either way, and it is the step the ordering rule is actually about.The rest, briefly
deleteRangeis point deletes in bounded batches, deliberately: no new operation id, no formatchange, and above all no change to the tombstone-drop rule, which fails by returning a deleted
document to a reader.
ExplainTypeNoteis a diagnostic, never a coercion. Type bracketing is unchanged andColumnPredicate.matchesis still the only definition. The family travels onNormal.Leaffrom thelowering because
ColumnPredicate.kindisinternaltorabosh-index, and re-deriving it in thequery layer would be a second definition of bracketing arriving by the back door.
detached()— a document read from a segment carries that segment's dictionary, so handingthe pair over is a trap that sometimes works. The test uses document 50 rather than document 0,
because document 0's two names occupy the same ids in both dictionaries and read back perfectly.
LockHolder.isRunningchecks the start time as well as the pid. Operating systems reuse pids,and telling a user to kill pid 4242 on the strength of a number is how they kill 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 regions overlap, so a build using either scheme still excludes one using the other.
runDrainis the acceptance test for the two big items in the only way that matters, a caller'sprogram: snapshot, scan from the watermark, ship, then record the watermark, retire, compact.
Every mistake in that order is silent.
The deprecation cycle's first live test
StoreLockedExceptionneeded a new constructor. The old two-argument one is still there,@Deprecated(WARNING)with aReplaceWith— which is whydirectoryisPath?rather thanPath.Zero removals across all seven dumps. If you would rather take the break than carry the nullability,
say so and I will; the policy shipped last week and this is the first time it cost anything.
Verification
build,-p build-logic check, all three samples, the module-path run,publishToMavenLocal,dokkaGenerate(no new warnings). Assertions verified by breaking them:RaboshCheckpointTestfails on both the index and the schema$..*..nopeunderJsonPathLimits.NONEStill not addressed
Second commit: the tier audit is now a gate
The section that used to sit here said the tier-leak audit was still not wired into
buildandoffered to do it in a follow-up. It is done in
7cd8465, in this PR rather than a later one, becausethe first commit adds public surface to every module in the chain and that is precisely when the
claim needs holding up.
ApiTierAuditis the awk script promoted: plain Kotlin overFilebesidePublishedModulesandCentralBundleReport, 15 unit tests, and a rootcheckApiTierstask hung offcheck— so./gradlew buildruns it and CI needs no new step. Root rather than per-module because the leak iscross-module: a type marked in
rabosh-indexleaks through a signature inrabosh-query's dump.Both halves are derived and neither is listed — the marker set from the sources, the surface from
the committed dumps. A hand-maintained list of experimental types would disagree with the annotations
exactly once, silently, in the direction of not reporting a leak. The awk script is deleted rather
than kept beside it, for the same reason.
Promoting it found a bug the hand-written version never had to have. The script carried a
hard-coded type list, so it never worked out which type a marked member belonged to. The derived
version does, and its first attempt took the most recent type declaration above the annotation — which
put
IndexCatalog.readinside aprivate classdeclared two hundred lines earlier, and reported acorrectly marked method as a leak. A false positive is what gets a gate switched off, so nesting now
follows indentation with a strictly-less-than comparison, and the sibling case is a test.
IndexCatalog.readcheckApiTiersfails, naming the method,IndexReaderand the modulemarkedInthrows rather than returning a short set — under-reporting is the only failure that mattersCurrent output:
API tiers: 7 dump(s) checked against Tier(20 type(s), 12 member(s)), no leaks.One limit stated rather than assumed: the audit checks what a dump can express.
List<Bitmap>erases to
Ljava/util/List;, so an experimental type reachable only as a type argument is invisible toit. Nothing in the current surface has that shape, and the phase note records it as the reason to
remember why if one is ever added.
🤖 Generated with Claude Code