Skip to content

native_reclaim_cycles cannot catch #84: coalescing leaves a one-row free list, so the fix's removal changes nothing #1138

Description

@OffgridwithJD

test/native_reclaim_cycles.sh is the declared regression guard for #84. Delete the
#84 fix and the suite reports 12 passed / 0 failed, unchanged.
It cannot reach the
defect it exists for, and the reason generalises to any suite that reasons about the
free list.

The measurement

The fix is the CommandCounterIncrement() in PgColumnarAllocateFreeSpace
(src/columnar_metadata.c), the one whose comment says:

A single compaction command can allocate many blocks in a row. Make this consumption
(and any remnant) visible to the next allocation's scan within the same command;
otherwise it would still see this row as free, re-select it, and fail with "tuple
already updated by self" on the second delete.

Removing that one statement, rebuilding, and running the suite:

CONTROL   (fix present)   native_reclaim_cycles.sh: PASSED   12 passed + 0 failed
MUTATED   (fix removed)   native_reclaim_cycles.sh: PASSED   12 passed + 0 failed

Identical, arm for arm, including compact_rewrite cycle N returns a count (no self-conflict) — the arm named after the defect.

Why: reclaim_coalesce defaults ON, and coalescing destroys the precondition

#84 needs one command to allocate from the free list more than once. With
coalescing on, compaction merges adjacent freed ranges, so the free list holds one or
two rows however much is freed. Measured on the suite's own fixture, free_space rows
before each compact_rewrite:

cycle        1    2    3    4    5
free rows    0    1    2    2    2

One row is not two allocations. The just-consumed row is never re-selected, so the
missing CommandCounterIncrement costs nothing observable.

Turn coalescing off and the same workload keeps the ranges separate. Two cells, each
built from its own source, each printing its own .so hash so neither measured the
other's binary:

fix present   .so e95880e45673   30 groups, free list steady at 18, three clean cycles
fix removed   .so 42bb17933a55   first compact_rewrite raises
                                 InternalError: tuple already updated by self

That is #84, reproduced on demand.

What I would change, and what I have already done

The suite's fixture needs a fragmented free list, not merely a non-empty one. What
worked here: 30,000 rows in 30 groups of 1,000, SET pgcolumnar.reclaim_coalesce = off,
delete a large contiguous block of whole groups, compact() — which leaves 18 separate
reusable ranges — and only then run the rotating delete + compact_rewrite cycles.

The pytest port already does this (#1142, now open), and its removal proof is the one above:

compact_rewrite cycle 1 returns a count (no self-conflict):
    got 'bad:tuple already updated by self' want 'ok'

It also asserts the precondition rather than assuming it, which is the part that keeps
this from going quietly vacuous again:

premise: the free list is fragmented, so one command allocates from it more than once
    -> free_space rows >= 5   (measured: 18)

I have deliberately not changed the shell suite. Its fixture is a one-paragraph
change and it is yours to make or decline; I would rather file the measurement than
edit a suite in a pull request whose subject is the port.

The general shape, which is the part worth keeping

A guard whose defect needs a contended or fragmented state can be silently disarmed
by an optimisation that makes that state rare — and the optimisation does not have to be
in the code under test. Here a GUC that defaults on, in a neighbouring feature, removed
the precondition of an unrelated regression guard, and nothing reported it: the suite
stayed green, the check kept its name, and the name kept saying "no self-conflict".

Worth asking of the other reclaim suites: native_reclaim_frag.sh runs both coalesce
modes explicitly and is not exposed to this, but any suite that asserts something about
allocation while leaving reclaim_coalesce at its default is asserting it about a
one-row free list.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions