fix(#1441): formalize relation generation publication - #1442
Merged
Conversation
This was referenced Sep 8, 2026
justinjoy
force-pushed
the
issue-1438-generations
branch
3 times, most recently
from
September 9, 2026 03:10
b96d2ec to
e9cb9b0
Compare
Collaborator
Author
|
CI repair applied and validated.
|
justinjoy
force-pushed
the
issue-1438-generations
branch
2 times, most recently
from
September 9, 2026 10:14
a3ab64f to
6171b0a
Compare
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
force-pushed
the
issue-1438-generations
branch
from
September 9, 2026 11:07
6171b0a to
ef651c0
Compare
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.
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.
Summary
col_rel_tprefix layout and document the identity allocator atomic sitesRebase onto
main(6fac60f)The two stacked commits this branch used to carry (#1384
pin primary arrangements during joins, #1435protect materialization cache lifetimes) are onmainas 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) inrelation.c,eval.candeval_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:col_rel_append_rowgrowth keeps this branch's prepare → fill →col_rel_publish_resize→touch_storageflow and folds Admit retained EDB relation growth under the memory budget #1433's admission around it: the governor reservation is taken before any storage is allocated, committed (col_rel_publish_retained_reservation) immediately before the storage swap, and rolled back together with the private buffers on any failure, so the relation and its previous token stay untouched on the error path. Admit retained EDB relation growth under the memory budget #1433'scol_rel_grow_heap_admittedis superseded by this and removed.col_rel_set_schemapublishes the retained reservation (Admit retained EDB relation growth under the memory budget #1433) and then bumps the view generation (this branch).col_rel_enable_timestamps(); thetouch_storagethis branch added at those call sites now lives inside that helper on both allocation paths.tests/test_simd_row_cmp.c'scol_rel_tmirror gained the fieldsmainadded betweenmem_ledgerandrow_scratch(Admit retained EDB relation growth under the memory budget #1433'smemory_governor/retained_reservation/retained_reserved_bytes, Instrument TDD memory usage and establish baseline #1380'sledger_ts_bytes); without them the generation suffix this branch relies on sat at the wrong offsets and test 9 failed.The #1438 commit applied cleanly.
CI on the rebased head (7a6c5fa) and fixes
Sanitizers / macos-latest / clang→relation_generationsFAIL. The four allocation-failure cases needld --wrap, whichtests/meson.buildonly enables on Linux; on other hosts the test counted them as failures ("allocator-failure wrappers unavailable"). They now report as skipped and the generation contract cases still run, so the test passes on macOS and Windows. Folded into the Define relation generation contract and mutation instrumentation #1441 commit.Build / ubuntu-latest / gcc→Check binary size: +11,635 B over the 262,305 B baseline.mainitself has grown to 268,199 B since ci: ratchet text baseline for memory admission #1437 set the baseline (Define arrangement and cache lifetime/rebuild policy #1384, Admit retained EDB relation growth under the memory budget #1433, Add pin-aware lifetime and generation validation for cached relation views #1435), and this branch adds 5,741 B on top (273,940 B measured by CI). Addedci: ratchet text baseline for relation generationsre-anchoring at 273,940 B, following e88a70a and ci: ratchet text baseline for memory admission #1437.Validation (after rebase)
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 passthreading_doc: 94/94 atomic sites;clang_tidy_ratchet: passtdd_multiworker) cleanuncrustify --checkandgit diff --check: cleanScope
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 ismainitself (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 onmainat 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 / gccreported a data race inwl_columnar_relation_generation_advancefromdiff_joinandtdd_decision_stats: the keyed, semijoin and cross-join parallel fill workers write disjoint row ranges of one fresh output throughcol_rel_set(), which now bumpsview_generationper cell, so W workers raced on the same counter.dcf77187addscol_rel_set_raw()(cell write without the shared-view detach and without generation publication; heap-owned, non-shared relations only) and uses it incol_join_write_pair_at(),col_semijoin_fill_worker_fn()andcol_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_multiworkerrun with zero TSan warnings.Build / windows-latest / msvcfailed twice over:relation.cincluded<stdatomic.h>and used_Atomicwith the non-_explicitcalls (MSVC has no C11 atomics in its default C mode), andinternal.hplaced thecol_rel_tlayout_Static_assert(offsetof(...))checks at file scope unconditionally (C2059/C2143 on every assert).65d597d2moves the identity allocator ontowl_atomic_u64and theatomic_*_explicitshimsmem_ledger.halready provides for MSVC and C11 (relaxed ordering; the counter only has to hand out distinct values), and guards the asserts with__STDC_VERSION__ >= 201112Lexactly asdiff_trace.cdoes.docs/THREADING.mdrows updated; the audited site count stays at 94.Rebase onto
main(#1434 / #1444 / #1462)maingained #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 inrelation.candsession.c. The merged result keeps both contracts:col_rel_append_row,col_rel_append_all) is this branch's transaction -- prepare private buffers, thencol_rel_publish_resize-- with Admit retained relation ownership transitions for COW and arena promotion #1434's admission folded in for retained relations: the governor reservation is taken before any buffer is allocated, committed immediately before publication, and rolled back together with the private buffers on any failure. Admit retained relation ownership transitions for COW and arena promotion #1434'scol_rel_grow_heap_admittedis superseded by this and removed.col_rel_grow_owned_transition, which now publishes the storage generation on success;col_rel_promote_arena_admitted(thesession.cside of the conflict) inherits that.col_rel_cow_unshareeverywhere (it publishes one storage epoch when it privatizes). It stays exported becausecol_rel_set()and the tests use it. The admission-free_rawvariant is gone:col_rel_radix_sortnow detaches through the admitted path and, if the sort cannot allocate, rolls the detach back completely (columns, ownership flags, ledger and both generations); the sort wrappers no longer pre-detach, which is what made that rollback impossible.col_rel_compactshrinks (and, for a shared view, privatizes) in one prepare/publish transaction; Admit retained relation ownership transitions for COW and arena promotion #1434's retained-reservation handling around it is unchanged.col_rel_append_allkeeps Admit retained relation ownership transitions for COW and arena promotion #1434's arena-preserving growth for unmanaged arena relations and this branch's transactional type metadata (prepared before growth, installed after growth and COW; every failure releases it).col_rel_install_shared_view: a destination that already tracks timestamps keeps a zeroed array sized like the borrowed columns when the source has none; Admit retained relation ownership transitions for COW and arena promotion #1434'smemory_admission_relationtest relies on that tracking surviving installation and the later growth..textagainst the 268,199 B baseline (+5,445 B, budget 5,120 B).mainitself is at 269,324 B (+1,125 B since the baseline: Admit retained relation ownership transitions for COW and arena promotion #1434, fix(#1434): admit retained relation ownership transitions #1444, CI: abi shell gates run against meson's 30s default and threading_doc now times out on Windows #1462) and this branch adds 4,320 B, inside the budget on its own; the last commit re-anchors the baseline at the measured value, following e88a70a, ci: ratchet text baseline for memory admission #1437 and 86d6314.The
relation_generations_oomtarget (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'smemory_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) ondiff_join,tdd_decision_stats,tdd_multiworkerwith zero warnings;uncrustify --checkandgit diff --checkclean.