Skip to content

bug: native diff compares no fields when its span contains a refusal #534

Description

@MarcusKainth

Where

Driver loop (driver)

Commit

89e73f9, and every commit before it that has a refusal in the span asked for.

OS, architecture and ClickHouse version

macOS 15 arm64, ClickHouse 26.8.2.7.

What happened

native diff compares no fields at all when the span it is asked for contains
a refused tic. It reads unresolved for every tic it ran, stops at the first
tic that sets it, and exits 3 before the field comparison runs. The tics before
the refusal are never compared, though their rows are exactly as trustworthy as
they would be in a span that stopped short of the refusal.

The consequence is that the same database and the same binary give opposite
answers depending only on how many tics you ask for:

$ clickdoom native diff 215 --probe <trace>
clickdoom: error: tic 210 unresolved: PL_ACTION_NEEDED

$ clickdoom native diff 209 --probe <trace>
clickdoom: error: tic 197 mobj slot 258 m_frame: 3 against the probe's 8

The second is a real divergence, present in both runs. The first run reached
tic 210, so it walked through tic 197, produced the wrong row, and reported the
refusal instead.

This is not theoretical. The branch on #517 moves the first refused tic from
181 to 210, and every run of it asked for more tics than 210, so none of them
compared a single field. sim_parity_live passed, driver/tests/native_diff_live.rs
passed, and all 17 CI checks passed on a head carrying a divergence at tic 197.
It was found only because a differential between two builds needed both sides
to stop at the same tic, so the span was 209 rather than the usual 215.

sim_parity_live does compare every field, but against the committed fixture
at refemu/probe/fixtures/demo3-frames.9a6a47d01119.tsv, which holds a handful
of rows. So the broad comparison covers the tics that fixture carries, and the
full per-tic comparison against the whole trace only ever happens through
native diff.

Between them, on any branch that refuses, most fields at most tics are compared
by nothing.

What you expected instead

A run that refuses at tic N still reports the first field that differs before
tic N, or says in as many words that it compared nothing.

Either is fine. What is not fine is exit 3 with a message that names a tic and
a bit, because that reads as a clean result up to that point, and it is what
made three green checks and a CI run mean less than they appeared to.

The refusal should still stop the run and still exit 3. The tics before it have
already been produced and are already comparable; not comparing them is the
part with no argument behind it.

Reproduction

# Any tree whose first refused tic is inside the span, with a field that
# differs before it. #517 at dd444cf reproduces it directly.
CONN="--host localhost --port 18124 --password clickdoom --database clickdoom_native"
PROBE=refemu/reference_traces/demo3/probe.9a6a47d01119.tsv

clickdoom native load --fresh $CONN
clickdoom native load --probe $PROBE $CONN

clickdoom native diff 215 --probe $PROBE $CONN   # refusal at 210, no field compared
clickdoom native diff 209 --probe $PROBE $CONN   # the field that differs at 197

Output

$ clickdoom native diff 215 --probe refemu/reference_traces/demo3/probe.9a6a47d01119.tsv
# native diff elapsed=55.4s tics=213 tics/s=3.8
clickdoom: error: tic 210 unresolved: PL_ACTION_NEEDED
exit=3

$ clickdoom native diff 209 --probe refemu/reference_traces/demo3/probe.9a6a47d01119.tsv
# native diff elapsed=52.8s tics=198 tics/s=3.7
clickdoom: error: tic 197 mobj slot 258 m_frame: 3 against the probe's 8
exit=3

The divergence those runs found was #528, already fixed on main by #531; the
tree it was found on predated that fix. The masking is independent of which
bug was behind it, and would have hidden any other.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: nativeNative mode: the tic simulation and renderer as SQL, and the WAD loaderbugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions