Skip to content

fix(#1441): formalize relation generation publication - #1442

Merged
justinjoy merged 4 commits into
mainfrom
issue-1438-generations
Sep 10, 2026
Merged

fix(#1441): formalize relation generation publication#1442
justinjoy merged 4 commits into
mainfrom
issue-1438-generations

Conversation

@justinjoy

@justinjoy justinjoy commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • define relation identity, view-generation, and storage-generation publication semantics
  • instrument direct mutation, COW, rollback, sorting, TDD, and shared-view paths with exact-once generation updates
  • make shared-view metadata and allocation-failure paths transactional, including graph/compound metadata
  • preserve the legacy col_rel_t prefix layout and document the identity allocator atomic sites

Rebase onto main (6fac60f)

The two stacked commits this branch used to carry (#1384 pin primary arrangements during joins, #1435 protect materialization cache lifetimes) are on main as identical patches and were dropped; the branch is now the two commits for #1441 and #1438.

The #1441 commit conflicted with #1433 (feat: admit retained relation growth, merged as ddce4fd/d3ef048b) in relation.c, eval.c and eval_serial.c. #1433 admits retained-EDB growth through the memory governor on the same paths that this branch rewrote around private-storage preparation and generation publication. Resolution:

The #1438 commit applied cleanly.

CI on the rebased head (7a6c5fa) and fixes

Validation (after rebase)

  • build of both PRs' test sets; arrangement_cache_reuse, arrangement_lru_eviction, delta_retraction, diff_join, generation_cache, mat_cache_lifetime, relation_generations, simd_row_cmp, session, relation_append, compaction, delta_timestamp, memory_governor, mem_instrumentation_{default,nofusion}, tdd_decision_stats, tdd_multiworker, io_adapter, k_fusion_memory, arrangement, col_rel_deep_copy, recursive_scc_default: all pass
  • threading_doc: 94/94 atomic sites; clang_tidy_ratchet: pass
  • ASan/UBSan: 15 of the above (all relation/cache/generation tests plus tdd_multiworker) clean
  • uncrustify --check and git diff --check: clean

Scope

Cache consumers and borrowed-storage lifetime integration remain in #1384/#1435 (both merged). No merge is requested by this PR creation.

Binary size gate

CI measured this branch at 272,028 B of .text (ubuntu-latest / gcc) against the committed 262,305 B baseline (+9,723 B). Of that, +5,894 B is main itself (268,199 B at 6fac60f: #1384, #1433, #1435), which is over the 5,120 B budget on its own; this branch adds +3,829 B, inside the budget. #1461 re-anchors the baseline on main at 268,199 B; the last commit here sets the same value so the gate passes before that lands, and the two merge cleanly either way.

CI fixes on the rebased head

  • TSan / ubuntu-latest / gcc reported a data race in wl_columnar_relation_generation_advance from diff_join and tdd_decision_stats: the keyed, semijoin and cross-join parallel fill workers write disjoint row ranges of one fresh output through col_rel_set(), which now bumps view_generation per cell, so W workers raced on the same counter. dcf77187 adds col_rel_set_raw() (cell write without the shared-view detach and without generation publication; heap-owned, non-shared relations only) and uses it in col_join_write_pair_at(), col_semijoin_fill_worker_fn() and col_join_cross_fill_worker_fn(). The coordinator already publishes the view generation exactly once around each parallel fill. Verified with -Db_sanitize=thread -Dthreads=posix (the CI configuration): diff_join, tdd_decision_stats, tdd_decision_stats_nofusion, tdd_multiworker run with zero TSan warnings.
  • Build / windows-latest / msvc failed twice over: relation.c included <stdatomic.h> and used _Atomic with the non-_explicit calls (MSVC has no C11 atomics in its default C mode), and internal.h placed the col_rel_t layout _Static_assert(offsetof(...)) checks at file scope unconditionally (C2059/C2143 on every assert). 65d597d2 moves the identity allocator onto wl_atomic_u64 and the atomic_*_explicit shims mem_ledger.h already provides for MSVC and C11 (relaxed ordering; the counter only has to hand out distinct values), and guards the asserts with __STDC_VERSION__ >= 201112L exactly as diff_trace.c does. docs/THREADING.md rows updated; the audited site count stays at 94.

Rebase onto main (#1434 / #1444 / #1462)

main gained #1434 (admit retained relation ownership transitions, reconcile COW ledger exactly once) and #1444 on the same growth paths this branch rewrote, so the #1441 commit conflicted in relation.c and session.c. The merged result keeps both contracts:

The relation_generations_oom target (Linux, -Db_lto=false + --wrap) is the one where allocation-failure injection actually fires; under the default LTO build the wrappers are inert. Verified on the rebased tree: relation_generations_oom, #1434's memory_admission_relation, both PRs' relation/cache/session tests (23 tests), threading_doc (94 sites), clang_tidy_ratchet, ASan/UBSan on the relation, admission and OOM tests, TSan (-Dthreads=posix) on diff_join, tdd_decision_stats, tdd_multiworker with zero warnings; uncrustify --check and git diff --check clean.

@justinjoy

Copy link
Copy Markdown
Collaborator Author

CI repair applied and validated.

TSan (ubuntu-latest / gcc) reports a data race in
wl_columnar_relation_generation_advance from test_diff_join and
test_tdd_decision_stats: the keyed, semijoin and cross-join parallel
fill workers write disjoint row ranges of one fresh output relation
through col_rel_set(), which now advances view_generation on every cell,
so W workers bump the same counter concurrently.

Add col_rel_set_raw() -- the cell write without the shared-view detach
and without the generation publication, valid only for heap-owned,
non-shared relations -- and use it in col_join_write_pair_at() (keyed
fill workers), col_semijoin_fill_worker_fn() and
col_join_cross_fill_worker_fn().  The coordinator already publishes the
view generation exactly once around each parallel fill (nrows
assignment and the error-path reset), which is the intended contract.
Serial appends keep col_rel_set().
Build / windows-latest / msvc fails on this branch:

  - relation.c included <stdatomic.h> and declared the identity counter
    _Atomic with the non-_explicit atomic_* calls; MSVC has no C11
    atomics in its default C mode ("C atomic support is not enabled").
    Use wl_atomic_u64 and the atomic_*_explicit shims that mem_ledger.h
    already provides for MSVC (_Interlocked*) and C11 (<stdatomic.h>),
    with relaxed ordering -- the counter only has to hand out distinct
    values.  docs/THREADING.md rows updated; the site count is unchanged.
  - internal.h placed the col_rel_t layout _Static_asserts at file scope
    unconditionally; MSVC rejects offsetof() there (C2059/C2143 at every
    assert).  Guard them with __STDC_VERSION__ >= 201112L exactly like
    diff_trace.c does for the same kind of check.
@justinjoy
justinjoy force-pushed the issue-1438-generations branch from 6171b0a to ef651c0 Compare September 9, 2026 11:07
The size gate compares libwirelog's .text against tests/baseline_size.txt
with a 5120-byte budget.  Since 86d6314 anchored it at 268199 bytes,
main grew to 269324 bytes on ubuntu-latest/gcc (#1434, #1444, #1462),
and the relation identity/generation instrumentation on this branch
adds 4320 bytes on top of that (273644 bytes measured by CI), so the
combined +5445 trips the gate although the branch's own growth fits the
budget.

Re-anchor at the measured value, as for e88a70a, #1437 and 86d6314.
@justinjoy
justinjoy merged commit f864050 into main Sep 10, 2026
47 of 49 checks passed
@justinjoy
justinjoy deleted the issue-1438-generations branch September 10, 2026 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant