Hold mutation leases across operator appends and cell writes (residual #2037 per-row lease cost)
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 35/100
- Issue type
- Refactor
- Clarity
- Mostly clear
- Activity status
- Active
- Tech stack
- c
- Domain
- backend, performance, testing
Research direction
Start by tracing the mutation-lease implementation from relation.c, then inspect the listed operator call sites in join.c, ops.c, inline.c, eval.c, kfusion.c, and session.c; compare with the held-lease batch appender used by filter.c. Run the sanitizer debug session test pinned to 2 CPUs and inspect mutation-set acquisition counts using wl_columnar_relation_test_mutation_nonce_peek. Done means converted operator tests reject per-row or per-cell acquisitions, outputs remain bit-identical, and the specified denial and renewal-failure cases cleanly destroy their outputs.
Written by the indexing model from the issue text.
Description
Discovery
Found while tracing the main sanitizer CI timeouts (Sanitizers / ubuntu-latest / gcc: tdd_recursive 30 s and session 180 s on 2-vCPU hosted runners) to the #2037 mutation-lease series. Since that series, the public single-relation mutators acquire, validate and finish a full mutation set on every call:
col_rel_append_row(relation.c): one set per appended row.col_rel_set(relation.c, fromdf9c7be8"Guard single-relation payload mutation with leases"): one set per cell.
The fix for the filter operator adds a held-lease batch appender (col_rel_append_batch_begin/row/end) and moves every filter.c fill onto it. Pinned to 2 CPUs, sanitizer debug:
| test | a8b5b5dd (pre-#2037) | main 04638f77 | with the filter fix |
|---|---|---|---|
tdd_recursive |
8.8 s | 26.0 s | 10.9 s |
session |
61.6 s | 187.0 s | 104.7 s |
session is still about 1.7x its baseline. With the filter fix applied, perf on session still shows col_rel_mutation_set_acquire at 12.7 % and col_rel_mutation_lease_validate at 13.9 %. 12.5 % is reached through wl_columnar_join_diff_op, whose col_join_append_pair writes each output cell with col_rel_set.
Remaining per-row and per-cell lease callers on operator paths
At origin/main 04638f77:
col_rel_set, once per cell:join.ccol_join_append_pair(1133, 1141, 1146);ops.caggregation (796–873);inline.c136, 362.col_rel_append_row, once per row:join.c1166 (col_join_append_pairgrowth fallback), 2478 (antijoin), 2932 (semijoin);ops.c443, 902, 1096;eval.c819, 2468;kfusion.c167;session.c3998, 5386.
Scope
Move the operator paths among these onto a held lease. Use the batch appender for appends. For per-cell writes, add a leased equivalent of col_rel_set, or write each row under the batch. Keep the lease contract intact, keep results bit-identical, and leave memory admission unchanged. Treat a hosted CI session time above about 150 s as the trigger to do this sooner.
Acceptance criteria
sessionpinned to 2 CPUs in the sanitizer debug build is back near its pre-#2037 time, measured againsta8b5b5dd.- Each converted operator has a test that counts mutation-set acquisitions per call (
wl_columnar_relation_test_mutation_nonce_peek) and fails on per-row or per-cell acquisition. - A mid-batch denial test, raised in review of the filter fix: a governor budget that refuses the second growth inside
wl_columnar_filter_op(timestamped and slow paths) andfill_filtered_rel. It asserts ENOSPC,sess->memory_budget_denied,col_rel_destroy_checked(out) == 0and no leak under ASan. Also one lease-renewal failure (nonce seeded near exhaustion) leaving the batch inactive with a clean destroy.
- Dominant language
- C
- Stars
- 18
- Forks
- 2
- Avg merge
- 10h 9m
- Merged PRs (30d)
- 173
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from semantic-reasoning/wirelog
-
Difficulty 2/5 1-3 hours Newbie friendliness 83/100
semantic-reasoning/wirelog#2229 ·
Maintainers usually reply within 1 day
-
Doc: insert executor and incremental consolidation leave sorted runs, not globally sorted headsOpen
Difficulty 1/5 Under an hour Newbie friendliness 83/100
semantic-reasoning/wirelog#2133 ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100
semantic-reasoning/wirelog#2042 ·
Maintainers usually reply within 1 day
-
p2-medium
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
semantic-reasoning/wirelog#1914 ·
Maintainers usually reply within 1 day
-
Difficulty 3/5 1-2 days Newbie friendliness 58/100
semantic-reasoning/wirelog#2262 ·
Maintainers usually reply within 1 day
All issues in semantic-reasoning/wirelog
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 1 day
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
-
build help wanted new feature
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
AOMediaCodec/SVT-AV1#2516 · 2 comments ·
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 77/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 63/100
Maintainers usually reply within 4 days