Skip to content

The parallel covering clamp is no longer provably unreachable after #1155 #1209

Description

@OffgridwithJD

What

In the parallel covering-projection block of PgColumnarSetRelPathlist
(src/columnar_customscan.c):

ioRunProj = ioRun * projScale;
if (ioRunProj > projRun)
    ioRunProj = projRun;
cpuRunProj = projRun - ioRunProj;

#1127 shipped a comment stating this clamp was unreachable, and gave its reason:
projRun = serialRun * projScale, so multiplying an already-clamped ioRun by
the same non-negative factor preserves the order. It also named what would
falsify it — "if projRun is ever computed independently (for example from the
projection's own pages)".

#1155 did exactly that:

projRun = cpuRun * projScale + ioProj;
ioProj  = seq_page_cost * projPages * sel;

So the stated reason is gone. #1155 has corrected the comment in place. This
issue is the open question the comment now records: whether the clamp can bind at
all under the new formula.

What is established

The old premise is false. Measured with a probe at the clamp site, on a build
where the harness confirms source ... matches the binary under test:

test/projection_parallel.sh     block reached 3 times, clamp bound 0 times
    ioRunProj=24.4794   projRun=2122.2110
    ioRunProj=24.4794   projRun=2122.2110
    ioRunProj=0.2473    projRun=163.8619
test/projection_scan_io.sh      block not reached
test/base_scan_io.sh            block not reached

What is NOT established

Reachability is unproven. I built a fixture aimed at binding it — an
incompressible base relation with a covering projection of the single int
column, forced parallel — and it did not bind either:

reached 1   bound 0     ioRunProj=0.2504   projRun=148.2537

Binding requires projScale * (ioRun - cpuRun) > ioProj. sel cancels, leaving

2 * ioBase - serialRun  >  baseSurvival * seq_page_cost * projPages

so base I/O must exceed base CPU by more than roughly projPages. Both attempts
moved that margin the wrong way, 87x then 592x, because
pgcolumnar_scan_io_run_cost prices only the columns actually read and a single
int column's decode CPU dwarfs its page I/O.

So this is not a claim that the branch is live. It is a claim that the reason we
had for believing it was dead no longer applies, and nobody has replaced it.

Why it matters if it does bind

The comment records the consequence: when the clamp binds fully, cpuRunProj is
zero and the partial covering path totals exactly like the serial covering path,
so Gather loses. That would mean the parallel covering projection path added in
#1127 silently stops being chosen — in the shape #1155 targets, a wide base with
a narrow covering projection.

Suggested next step

Either construct a case that binds it — the algebra above says where to look:
maximise base page I/O relative to base decode CPU for the columns read, with
projPages small — or establish it cannot bind and restore a proof to the
comment. Until one of those, the clamp stays, because removing it would allow a
negative cpuRunProj, which is a reason that does not depend on reachability
either way.

Please do not let the comment drift to "reachable"; UNPROVEN is the measured
state.

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