Conversation
A session driven by wirelog_easy_step() with no delta callback installed re-derived every rule on every step, on top of the rows the previous evaluation had already derived. Every step appended another copy of each derived row. A removal never retracted the rows it had derived. A step with nothing pending re-ran every rule, so a value-position @call ran once per derived row on each idle step. With three `ready` rows, the derived relation held 3, 6, 9 and 12 rows after one to four steps. A snapshot taken afterwards emitted that state as the stable model, which is the "step then snapshot duplicates rows" pattern wirelog-easy.h warned about. Inserts and removals made without a callback already force a full evaluation, so a plain step is meant to re-derive everything. It now does so correctly: - The early return for a step with nothing pending no longer requires a delta callback. Every write entry point that changes input sets pending_input_change, and a new session starts with it set. - A plain step evaluates every stratum. Once the session has evaluated, it first clears the derived relations with col_session_clear_idb_rows and resets every stratum frontier, under the same has_evaluated condition the snapshot path uses before a full re-evaluation. Without the stratum reset, the transitive closure in the new test lost (1,4) after an edge was added. A resumed step keeps what its first attempt already merged. - The full mask is forced even when the pending input recorded only some strata, as an insert staged with a callback that is then removed does. Clearing alone would leave the other strata empty. - Before clearing, the step sets pending_full_input_eval, which only its commit resets. If the step then fails, the next snapshot evaluates in full instead of re-deriving only the strata the pending input reaches over the emptied relations. - A plain step no longer seeds retraction deltas ($r$<name>) left by a removal staged with a callback. Seeded, the first iteration read the removed rows in place of the relation and re-derived paths through them. Clearing derived rows also cleared input. A rule head can also hold input: an inline fact such as `reach(1).` beside `reach(y) :- reach(x), edge(x, y).`, or a host row inserted into it. It lost that input whenever a full re-evaluation reset it. On main a second snapshot after an insert already returned `reach` empty, and four-worker recursive steps did too. A rule head now keeps its input rows in a private `$in$<name>` shadow: - The first insert into a rule head registers its shadow; inline facts are loaded through that same insert. Creating shadows at session creation would raise the creation memory floor that test_session pins, and creating them when the first evaluation starts would leave a reservation behind after a refused step. - A direct-mapped cache of relation identities answers "not a rule head" for plain input relations. Without it, a million single-row inserts took 0.71 s against main's 0.51 s; with it the two match within noise, also when two relations alternate. - Inserts write the shadow first and truncate it if the relation's own append fails. - Removals plan the shadow change before the relation is touched and apply it once the removal has committed. The shadow records input, so a removal applies to it even when the relation no longer holds the row; a callback-mode step can already have dropped it. Such a removal leaves the model as it is, so it schedules no re-evaluation. - wl_columnar_session_restore_seed appends the shadow back after both resets: col_session_clear_idb_rows and the multi-worker recursive reset in eval.c. Tests: - New test_easy_plain_step: - idle steps leave one copy of each row; - inserts and removals in two strata hold exactly the model; - a recursive closure is extended and then cut; - a step after removing the callback re-derives everything, after a staged insert and after a staged removal; - inline facts survive plain steps on one and four workers, and snapshots alone; - host rows in a rule head without inline facts survive the same three modes; - host rows and an inline fact removed with a callback installed, or after it was removed, stay removed; - a host row inserted twice is gone after two removals. - test_extension_replay: - a no-callback case pins five invocations to load, zero per idle step, five for an insert into an unread relation and four after a retraction. Snapshots in that case leave the invocation counter unchanged, so they read the stable model rather than repairing it. - a plain step whose addon fails after the clear is followed by a snapshot that returns the full model. - removing input the model already lacks, with a callback installed, invokes no addon in another stratum. - Mutation checks: each of fourteen mutants fails a named check: - restore the callback requirement; - drop the clear, the stratum reset, the forced mask, or pending_full_input_eval; - re-enable the retraction seed; - skip the shadow on insert, on its creation, on plain removal, on the incremental removal hit or miss, on restore, or in the eval.c reset; - schedule a re-evaluation for an input-only removal. Docs: - wirelog-easy.h documents both step modes and drops the warning against a snapshot after a step. - docs/SEMANTICS.md specifies the no-callback invocation count. Its publication-cutoff state map now lists col_session_step_impl as a reader of has_evaluated, and session_seed_shadow_truncate as a reader and writer of base_nrows. A selftest anchor that the first edit made ambiguous now names the one row it edits. - Example 12's comment and README no longer claim that step followed by snapshot double-counts; checked on that program, it returns the same five rows. - CHANGELOG. Not changed: with a delta callback, a rule head's own input is mishandled. The first step publishes an inline fact as a retraction and leaves the relation empty, and a host row inserted into a rule head is also published as a retraction. Main does the same. Fixes #1994
This branch has not been deployed
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.
Fixes #1994.
Problem
wirelog_easy_step()on a session with no delta callback re-derived every rule on every step, on top of the rows it had already derived:@callran once per derived row on each idle step.wirelog-easy.hwarned about.Fix (
col_session_step_impl)has_evaluatedcondition the snapshot path uses. Without the frontier reset, a recursive closure lost(1,4). The full mask is forced even when the pending input recorded only some strata.pending_full_input_evalbefore clearing, so a failed step leaves a full re-evaluation pending.$r$retraction deltas left by a removal made with a callback installed.Rule-head input
Clearing derived rows also cleared input: inline facts (
reach(1).beside a rule forreach) and host rows inserted into a rule head. The snapshot path already had this defect on main: a second snapshot after an insert returnedreachempty, and so did four-worker recursive steps.A rule head now keeps its input in a private
$in$<name>shadow:col_session_clear_idb_rowsand the multi-worker recursive reset ineval.c.Placement and cost:
test_sessionpins. Creating them at the first evaluation would leave a reservation behind after a refused step.Tests
test_easy_plain_step(15 cases):test_extension_replay: no-callback addon counts, a failing plain step followed by a snapshot, and an input-only removal that invokes nothing in another stratum.Docs
wirelog-easy.hdocuments both step modes and drops the step-then-snapshot warning.docs/SEMANTICS.mdspecifies the no-callback invocation count and updates its state map.Validation
meson test -C build: 433 OK, 0 failed (before rebase).test_easy_plain_stepincluding the 4-worker cases.84487d59, the abi suite has one failure:python_encodingflagsscripts/perf/hosted_perf_calibration.py, which84487d59added. This PR does not touch it.Not changed
These behave the same on main:
Peer review: an independent reviewer did four rounds. Rounds 1–3 raised blocking findings, which are fixed here: failure recovery, staged removals, rule-head input loss, doc wording, and an over-eager re-evaluation on input-only removals. The final tree had no blocking findings.