Skip to content

fix(#1022): _update_by_query pages the match set; exact sig-text/enrich counts - #1023

Merged
xerj-org merged 9 commits into
mainfrom
fix/issue-1022-update-by-query-paging
Sep 26, 2026
Merged

xerj-org merged 9 commits into
mainfrom
fix/issue-1022-update-by-query-paging

Conversation

@xerj-org

Copy link
Copy Markdown
Owner

#1022 - three one-shot size:10000 truncations, all found by #1019's verify pass. All three silently did a third (or less) of the requested work while reporting success.

What was broken (fail-before, red on the starting tree)

  1. _update_by_query - ONE {query, size:10000, from:0} search, update whatever came back; total was the page length, batches hardcoded 1. Past 10,000 matches it silently under-updated. max_docs / scroll_size were not implemented at all.
  2. significant_text profiler - the profile debug counters were computed from the first 10,000 hydrated hits; extract_count froze at 10,000 on any larger corpus.
  3. enrich policy _execute - the .enrich-* index was materialised from only the first 10,000 source docs; records reported the page length.

Fail-before verbatim: update_by_query_updates_entire_index.rs 0/6 passed (total must be the exact match count, not the page length: left 10000, right 25000), significant_text_profile_counts_every_doc.rs 1/3, enrich_execute_copies_entire_index.rs 1/2.

Fix

All three reuse #1019's machinery; nothing external:

  • SITE 1 run_update_by_query rewritten on the delete runner's shape: flush precondition, scroll_size (default 1000, clamped 1..=10000), max_docs (ES semantics: total = processed count), real batches. Scripted match_all/ids selectors take the single-pass matching_ids_sorted arm; everything else _id-keyset search_after pages with the ids_only projection. Mid-run rewrite hazard handled: a batch that mutates docs repopulates the memtable and _id-keyset paging is only correct over flushed segments - every mutating batch flushes before the next page (failed flush aborts rather than risk applying a script twice). Pinned by the 2500-doc scroll-100 exactly-once test.
  • SITE 2 exact counting without materialisation: SigTextAccumulator (order-independent sums + per-group token sets) fed by a new single-pass Index::fold_match_sources walker - memtable walk + per-segment stored decode under the existing single-flight caches, version-map liveness filter, tombstone handling. A fold, not keyset paging: a read endpoint cannot flush, and order-independent counters need no ordering.
  • SITE 3 enrich _execute adopts the _reindex keyset loop: flush each source, 1000-doc _id:asc pages with search_after, records = exact materialised count.

A/B - synthetic corpus, 30,000 docs (stated as synthetic; private ports, throwaway dirs)

_update_by_query match_all + script:

before after
total / updated 10000 / 30000 30000 / 30000
batches 1 (hardcoded) 30 (real)
touched docs 10000 30000

enrich _execute: records 10000 -> 30000. sig-text extract_count 10000 -> 30000 (same wall time - the fold replaces the search+hydrate path entirely).

"before" is faster because it does a third of the work and reports the page length as the total. Per-doc amortised update cost 24 -> 33 us/doc (completeness' extra writes + 30 flushes).

Gates

cargo fmt --check clean; clippy -p xerj-api -p xerj-engine --all-targets 0 warnings; cargo test --profile ci-test -p xerj-api -p xerj-engine 1898 passed / 0 failed; ES-YAML conformance 1380 passed / 0 failed / 3 skipped (starting tree 1378/0/3; +2 new update_by_query cases). 11 new HTTP tests + 2 ES-YAML cases, all fail-before on the starting tree.

Reference-code: none retrieved - the adapted pattern is XERJ's own, established in-tree by #1019. ES consulted only for wire semantics.

Stacked on #1021 (merge that first; #1020 before it).

Fixes #1022.

Root cause, two defects, both present in the HTTP runner
(es_compat.rs run_delete_by_query) and the engine loop
(Index::delete_by_query):

1. ONE search, `{"query":…,"size":10000,"from":0}`, then delete whatever
   came back. Beyond 10 000 matching docs the call silently under-deleted
   while reporting success: `total` was the page length, `batches` was
   hardcoded 1. The autoindex client works around it by looping up to
   1 000 delete passes, each re-paying the search (esclient.rs:1658-1669).
2. That search built a full `Hit { source: Value }` per match — 10 100
   hydrated `Value` trees per call to read nothing but `hit.id`.
   `_source: false` does not avoid this: the engine keeps the raw source
   for `fields`/highlight resolution, and the unsorted scan Value-parses
   every stored document anyway. This was the residual RSS transient
   #950 recorded as follow-up headroom.

Fail-before (both red on main, verbatim):

  engine/crates/xerj-api/tests/delete_by_query_purges_entire_index.rs
    delete_by_query_purges_past_ten_thousand_in_one_call
      total must be the exact match count, not the page length:
      {"total":10000,"deleted":10000,"batches":1,…}
        left: Number(10000)  right: Number(25000)
    delete_by_query_honors_max_docs
      with max_docs set, total is the processed count
        left: Number(20)  right: Number(7)
    delete_by_query_reports_real_batch_count
      10 docs at scroll_size 3 = 4 batches
        left: Number(1)  right: Number(4)
    delete_by_query_paginates_selective_queries
      20 matches at scroll_size 4 = 5 batches
        left: Number(1)  right: Number(5)
    test result: FAILED. 0 passed; 4 failed

  engine/crates/xerj-engine/tests/integration.rs
    test_delete_by_query_purges_past_ten_thousand
      every matching doc deleted, not just the first 10k page
        left: 10000  right: 12000
    test result: FAILED. 1 passed; 1 failed

Design.

* Internal ids-only projection. `SearchRequest::ids_only`
  (xerj-query/src/ast.rs) is `serde(skip)` and hard-set `false` in
  `parse_request`, so it can never arrive from the wire (same contract as
  `savings`). `search_inner` honours it only when the request provably
  needs nothing beyond ids: sort non-empty and confined to `_id`/`_doc`,
  no `fields`/`script_fields`/`highlight`/`explain`/`collapse`/`rescore`/
  `aggs`/`min_score`. Every other shape runs exactly as before. At the
  three admission sites (stored-section scan + both hydration arms) the
  stored `_id` is extracted by a borrowed prefix scan
  (`extract_stored_id_str`; escape-bearing layouts fall back to the full
  parse, which still admits source-Null) and the hit carries
  `source: Value::Null` with the scorer skipped. The version-map liveness
  filter is kept verbatim. A precomputed cursor `(id, ascending)` declines
  docs at/below `search_after` with one memcmp BEFORE the liveness lookup
  and every allocation.

* Keyset paged arm for arbitrary queries, modeled on the `_reindex` loop:
  flush the member first (paging and the id collection below are only
  correct over on-disk segments), then pull `scroll_size` pages sorted
  `_id: asc` with a `search_after` cursor. ES semantics: `scroll_size`
  default 1 000 clamped 1..=10_000, `max_docs` truncates processing and
  `total` then reports the processed count, `batches` is the real page
  count, `total` otherwise the exact page-1 match count,
  `noops`/`version_conflicts` 0, script resource failures fail-closed
  per batch before any id of that batch is deleted.

* Single-pass arm for `match_all`/`ids` selectors. Measurement-driven:
  the paged arm alone re-scans every stored section once per page —
  O(N x pages). On the 252k-doc corpus the default-params purge measured
  43.5 s, 28.9 s after the cursor fast-decline; still quadratic, because
  every page must consider every doc. `Index::matching_ids_sorted` now
  collects the whole live match set in ONE pass over #950's cached
  `id_pos_map_for` id maps plus the version map (one `String` per live
  match, ~48 B), sorts it, and the runner deletes it in `scroll_size`
  batches. Observable semantics unchanged (`batches` = ceil(n/scroll_size));
  like ES's by-query, the match set is the start-of-run snapshot and a
  concurrently-vanished id is tolerated. Any other query shape (or a
  segment without a complete id index) takes the paged arm; `None` falls
  back, never errors.

Reference-code retrieval, honestly: `xc.py` was run this session and
FAILED for this task's domains — the quickwit and elasticsearch corpora
are not in the sandbox index, and building them was outside this task's
hard constraints (the shared corpus server/index must not be touched).
The design is XERJ's own. The one adapted approach — resolving document
ids from an id map instead of the document store — was recorded at
commit d1e8056 (#950): meilisearch `external_documents_ids()`
(crates/meilisearch/src/routes/indexes/documents.rs:2284, MIT, approach
only) and quickwit `warm_up_terms` (quickwit-search/src/leaf.rs:475,
Apache-2.0); those file:line references are from the prior session and
approximate. #1019 builds on that #950 machinery. No Elasticsearch (AGPL/
SSPL/Elastic) or sonic (GPL) code was read for or copied into this change;
ES was consulted only for wire semantics (`scroll_size`, `max_docs`,
`total`, `batches`).

Measurement — synthetic corpus, this session, private ports + throwaway
data dirs, wall via curl time_total, RSS via /proc/<pid>/status sampled
at 250 ms: 252 000 docs / 74.5 MB NDJSON ingested into one index
(16 segments). before = main 62fa237 release binary, after = this
branch's release binary.

  single _delete_by_query, default params (match_all):
    before        wall 0.54 s   deleted 10 000 / 252 000  (defect 1)
                 peak RSS 1 234 216 kB   VmHWM 1 256 268 kB
    before, client workaround loop until _count==0
                 wall 3.78 s   deleted 252 000
                 peak RSS 1 285 416 kB   VmHWM 1 354 104 kB
    after         wall 1.10 s   deleted 252 000 / 252 000
                 total 252 000   batches 252   failures []
                 peak RSS   466 100 kB   VmHWM   471 764 kB

  3.4x faster than the pre-fix workaround loop AND complete, with ~62%
  lower peak RSS. Intermediates on the same corpus (all this session):
  ids-only + keyset paging alone 43.5 s default / 9.1 s at scroll 10k;
  + cursor fast-decline 28.9 s / 7.4 s; + single-pass arm 1.10 s. The
  ~900 MB RSS transient common to both before runs is NOT the delete
  search itself: a single plain wire-level sorted search returned having
  added ~220 MB and grew to HWM 1.21 GB three seconds AFTER the response
  — background post-search merge/cache hydration over the 16-segment
  corpus (the #950 follow-up area, out of #1019's scope). The single-pass
  arm never runs that search, so its peak is the delete path's own.

Gates (this branch, final code): cargo fmt --check clean; clippy
-p xerj-engine -p xerj-api -p xerj-query --all-targets clean; full
xerj-engine suite green; full xerj-api suite green (68 test binaries);
ES-YAML conformance suite 1378 passed / 0 failed / 3 skipped against a
private port with a throwaway data dir (main: 1376/0/3; +2 new
delete_by_query cases). New coverage: 4 HTTP-level tests, 1 engine
integration test, 1 engine lib test pinning `source: Null` + gap-free
dupe-free keyset pages, 2 ES-YAML cases, all fail-before on main.

Fixes #1019.
…ch counts

#1019's verify pass found three more one-shot `size:10000` truncations.
All three silently did a third (or less) of the requested work while
reporting success; this commit pages or folds all three to completion.

Root cause, three sites, same shape:

1. `_update_by_query` (es_compat.rs run_update_by_query): ONE
   `{"query":…,"size":10000,"from":0}` search, update whatever came
   back, `total` was the page length, `batches` hardcoded 1. Past
   10 000 matches the call silently under-updated. `max_docs` and
   `scroll_size` were not implemented at all.

2. significant_text profiler fast path (compute_sig_text_debug): the
   profile `debug` counters were computed from the first 10 000
   hydrated hits, so `extract_count`/`values_fetched`/`chars_fetched`/
   `collect_analyzed_count` froze at 10 000 on any larger corpus.

3. enrich policy `_execute`: same one-shot copy — the `.enrich-*`
   index was materialised from only the first 10 000 source docs and
   `records` reported the page length.

Fail-before (red on the starting tree 64c698e, verbatim):

  xerj-api/tests/update_by_query_updates_entire_index.rs  0 passed; 6 failed
    update_by_query_scripts_past_ten_thousand_in_one_call
      total must be the exact match count, not the page length:
      {"total":10000,"updated":10000,"batches":1,…}
        left: Number(10000)  right: Number(25000)
    update_by_query_reindexes_past_ten_thousand_in_one_call
        left: Number(10000)  right: Number(25000)
    update_by_query_honors_max_docs
      with max_docs set, total is the processed count
        left: Number(20)  right: Number(7)
    update_by_query_reports_real_batch_count
      10 docs at scroll_size 3 = 4 batches
        left: Number(1)  right: Number(4)
    update_by_query_paginates_selective_queries
      20 matches at scroll_size 4 = 5 batches
        left: Number(1)  right: Number(5)
    update_by_query_applies_script_exactly_once_per_doc_when_paged
      2500 docs at scroll_size 100 = 25 batches
        left: Number(1)  right: Number(25)

  xerj-api/tests/significant_text_profile_counts_every_doc.rs  2 passed; 1 failed
    significant_text_profile_counts_past_ten_thousand
      extract_count must count every matched doc:
      {"values_fetched":10000,"chars_fetched":110000,
       "extract_count":10000,"collect_analyzed_count":20000}
        left: Number(10000)  right: Number(10500)

  xerj-api/tests/enrich_execute_copies_entire_index.rs  1 passed; 1 failed
    enrich_execute_copies_past_ten_thousand
      records must be the exact source count, not the page length:
      {"enrich_index":".enrich-pol-big","records":10000}
        left: Number(10000)  right: Number(10500)

Design — all three reuse #1019's machinery; nothing external.

* SITE 1, run_update_by_query rewritten on the delete runner's shape
  (es_compat.rs:26792): flush precondition; the query is validated once
  through the same `{"query":…,"size":scroll_size,"sort":[{"_id":"asc"}]}`
  body; `scroll_size` defaults 1000 clamped 1..=10_000, `max_docs`
  truncates processing and `total` then reports the processed count,
  `batches` is the real page count. The 30 s SCRIPTED_UPDATE_BUDGET
  deadline and fault capture now span the whole paged loop (the runner
  is Box::pin'd — the inline state machine overflowed the 2 MiB
  debug-profile test stack). Script mode + match_all/ids selectors take
  #1019's single-pass `matching_ids_sorted` arm (one O(N) id
  materialisation, transformed per id in scroll_size chunks);
  everything else takes `_id`-keyset `search_after` pages with the
  ids_only projection (#1019). Per-id work goes through
  `transform_document_serialized` inside `transform_one`
  (es_compat.rs:27132): a doc that vanished mid-run is skipped
  silently, an update error lands in `failures` per ES semantics.
  The no-script arm (reindex-in-place, pipelines) is preserved verbatim
  but paged with `_source:true` pages.

  Mid-run rewrite hazard, unique to update: a batch that mutates docs
  repopulates the memtable, and `_id`-keyset paging is only correct
  over flushed segments (the invariant documented at the reindex loop
  and run_delete_by_query) — an unflushed next page could resurface a
  doc at/below the cursor and apply the script TWICE. So every batch
  that mutated >= 1 doc flushes the index before the next page is
  searched; a failed flush aborts the run rather than risk it. The
  2500-doc scroll_size-100 regression test pins exactly-once semantics.

* SITE 2, exact counting without materialisation: the significant_text
  debug counters are now an incremental accumulator
  (SigTextAccumulator, es_compat.rs:24897 — sums and per-group token
  sets, i.e. order-independent) fed by a new single-pass engine walker
  `Index::fold_match_sources` (index.rs:17891): memtable walk plus
  per-segment stored-section decode under the existing
  decoded_stored_cache/stored_slices_cache single-flight locks, with
  the version-map liveness filter, tombstone/ghost-window handling and
  seen-set dedup, matching every document with the same resolution
  pipeline and `doc_matches_query_typed` matcher the memtable scan arm
  of search uses (#396: buffered answers must not change at flush).
  A fold, not keyset paging: a read endpoint cannot flush (that would
  be a write side effect from _search), and without a flush the
  keyset-page invariant is unavailable — order-independent counters
  need no ordering at all, so one pass over a consistent segment
  snapshot is exact. The debug json shape is unchanged (pinned by the
  small-corpus test and the existing significant_text.yml pins).

* SITE 3, enrich `_execute` adopts the `_reindex` keyset loop
  (es_compat.rs:32344): flush each source, then 1 000-doc pages sorted
  `_id: asc` with a `search_after` cursor and `_source:true`; first
  write error fails the request; `records` is the exact materialised
  count; response shape unchanged. No new wire params.

Measurement — synthetic corpus (30 000 generated docs, ~250 B each,
stated as synthetic), this session, private ports 9622/9623 +
throwaway data dirs, wall via curl time_total, RSS via
/proc/<pid>/status sampled at 250 ms. before = base 64c698e release
binary, after = this branch's release binary:

  _update_by_query, match_all + `ctx._source.touched = 1`:
                          before            after
    total / updated       10 000 / 30 000   30 000 / 30 000
    batches               1 (hardcoded)     30 (real, scroll 1000)
    wall                  0.24 s            1.00 s
    touched docs (_count) 10 000            30 000
    last doc in _id order no touched field   touched = 1
    peak RSS (window)     384 MB            554 MB
    timed_out             false             false

  enrich _execute (one source index):
    records / .enrich-*    10 000 / 30 000   30 000 / 30 000
    wall                   0.24 s            2.89 s

  significant_text profile counters (match_all over the body field):
    extract_count          10 000            30 000
    chars_fetched          1 800 000         5 400 000
    collect_analyzed_count 180 000           540 000
    wall                   0.34 s            0.33 s

  before is "faster" because it does a third of the work and reports
  the page length as the total. Per-doc amortised update cost moved
  24 -> 33 us/doc for completeness' extra writes + 30 flushes. The
  sig-text fold counts 3x the documents in the same wall time because
  it replaces the search+hydrate path entirely.

Gates (final tree): cargo fmt --check clean; clippy -p xerj-api
-p xerj-engine --all-targets 0 warnings; cargo test --profile ci-test
-p xerj-api -p xerj-engine 146 test binaries ok, 1898 passed, 0
failed (ci-test is CI's profile; the dev-profile run additionally
passes all xerj-api binaries but aborts in the pre-existing
hybrid_fused_order_is_stable debug stack overflow, verified present
on the starting tree 64c698e before any of this branch's changes);
ES-YAML conformance 1380 passed / 0 failed / 3 skipped against a
private port with a throwaway data dir (starting tree: 1378/0/3;
+2 new update_by_query cases). New coverage: 6 update_by_query HTTP
tests, 2 enrich HTTP tests, 3 sig-text HTTP tests, 2 ES-YAML cases —
all fail-before on the starting tree.

Reference-code: none retrieved — the adapted pattern is XERJ's own,
established in-tree by #1019 (d1e8056 lineage); CLAUDE.md's
retrieval mandate does not bind on changes confined to this
repository's own code. No Elasticsearch (AGPL/SSPL/Elastic) or sonic
(GPL) code was read or copied; ES was consulted only for wire
semantics (max_docs, scroll_size, total, batches).

stacked on fix/issue-1019-delete-by-query-ids-only (#1021); merge that first

Fixes #1022.
@cla-bot cla-bot Bot added the cla-signed label Sep 21, 2026
@xerj-org

Copy link
Copy Markdown
Owner Author

CI caught a real regression in this branch's head commit — fix in progress.

Build + Test failed on 427e2f47: concurrent_scripted_update_by_query_preserves_every_increment — 16 barrier-released _update_by_query(match_all, ctx._source.n += 1) on one doc, every task reports failures: [], final n == 1 (expected 16). Deterministic locally (0.05 s) and on the runner.

Bisect: green at 64c698ec (the #1019 commit this stacks on), green on main, red only at 427e2f47 — so PR #1021 is unaffected and may merge on its own merits; this PR waits for the fixup commit.

Leading suspect (under confirmation): the new entry-flush precondition (idx.flush() at the top of run_update_by_query) racing the per-doc guarded read-modify-write — the old one-shot path never flushed. Pushing the root cause + fix here shortly.

CI on this branch caught a concurrency regression the fail-before suite
could not see, because it needs sixteen runners racing one document:

    es_compat::scripted_update_publication_tests::
    concurrent_scripted_update_by_query_preserves_every_increment
    assertion `left == right` failed
      left: Number(1)
     right: 16

Sixteen concurrent `_update_by_query` calls over a one-doc index left
n == 1: fifteen increments were silently lost while every runner
reported success (failures:[]).

Root cause (instrumented, two independent runs, identical shape): the
increments were never attempted — they vanished at match-set collection.
run_update_by_query's new entry flush makes all 16 tasks call flush()
concurrently; exactly one shard task wins the memtable drain, and from
that instant the doc is in neither the memtable nor a published segment
until the flush's Phase-2 publish completes. The flush writer bracket
spans exactly that drain→publish window (guard begins before the drain,
commits in the finalize worker after the snapshot publish and version-map
repoint). The other 15 flushes drained nothing and returned ~100x
faster; each then called Index::matching_ids_sorted, which enumerated
ONLY store.snapshot().segments via the per-segment _id maps — no
memtable leg, and decisively no CollectionPublication reader admission —
so all 15 collected ids=[] inside the winner's window, looped over
nothing, and returned total:0/updated:0/failures:[]. Instrumentation
grep: 15x ids=[] + 1x ids=["counter"]; exactly one transform ran
(0→1). flush() itself is sound: the guarded write survived and was
served afterwards; get_document and search are already admission-bracketed
and retry on WriterActive, which is why they were never wrong. The defect
is that the #1019/#1022 id-enumeration API was not linearized against
flush publications — run_delete_by_query has the identical hole.

Fix, engine-side, minimal and idiomatic (get_document's own retry shape,
collection_publication.rs):

- Index::matching_ids_sorted is now async and wraps its enumeration in
  the same reader-admission bracket get_document uses (index.rs):
  tokio::pin!(notified()) + enable(), try_admit_reader, enumerate,
  validate_reader, retry on WriterActive/generation change. On Poisoned
  it returns None, preserving the "unsupported shape → paged arm"
  contract — the paged arm's search() surfaces the poison as an error
  instead of a silent no-op.
- fold_match_sources (#1022's sig-text walk) gets the narrow variant of
  the same bracket: its memtable capture and segment snapshot are now
  taken inside ONE admission and validated BEFORE the walk — a doc
  drained after the capture and unpublished at the snapshot was silently
  missed. Only the captures are retried (f has side effects; the walk
  itself is never re-run).
- .await added at the three call sites: run_update_by_query's
  single-pass arm, run_delete_by_query's single-pass arm (es_compat.rs),
  and the engine's own Index::delete_by_query.

Nothing else changes: the entry flush stays (the paged arm needs it),
and the flush/memtable/write paths are untouched because they are
correct — the 32-writer sister test passes unchanged.

Fail-before, reproduced by reverting just the admission wrapper on this
tree (single unguarded snapshot pass = pre-fix semantics):

  concurrent_scripted_update_by_query_preserves_every_increment
    FAILED — left: Number(1), right: 16 (the CI signature)
  test_matching_ids_sorted_linearized_against_concurrent_flush (new)
    FAILED — left: [], right: ["counter"]

After:

  concurrent_scripted_update_by_query_preserves_every_increment
    20/20 consecutive runs green (--profile ci-test, --exact)
  test_matching_ids_sorted_linearized_against_concurrent_flush   ok
  test_concurrent_delete_by_query_purges_the_document (new)      ok
  concurrent_delete_by_query_purges_the_document (new, api twin) ok
  update_by_query_updates_entire_index (paging/exactly-once)     6/6
  xerj-engine --lib          725 passed, 0 failed
  xerj-api --lib             273 passed, 0 failed
  xerj-engine --test integration  143 passed, 0 failed (1 ignored)
  cargo fmt --check / clippy (touched crates)  clean
  ES-YAML conformance (private :9598, throwaway dir)
    1380 passed · 0 failed · 3 skipped

New tests: engine race test (8 tasks flush+collect over one doc, every
collector must see it) + its delete_by_query twin at engine and api
level (no swallowed failures, doc actually purged; the deterministic
lost-work proof is the update twin — a runner collecting after another's
delete legitimately reports total:0, ES's start-of-run collection
semantics).

Refs #1022.
@xerj-org

Copy link
Copy Markdown
Owner Author

CI regression root-caused and fixed — cec66fa0 on this branch.

What CI caught — concurrent_scripted_update_by_query_preserves_every_increment: 16 concurrent _update_by_query calls over one doc (ctx._source.n += 1) left n == 1, assertion left: Number(1) right: 16, while every runner reported failures: [].

Root cause (instrumented; 2 runs, identical shape): the 15 increments were never attempted — they vanished at match-set collection. The new entry flush makes all 16 tasks flush() concurrently; exactly one shard task wins the memtable drain, and until its Phase-2 publish lands the doc is in neither the memtable nor a published segment (the flush writer bracket spans exactly that drain→publish window). The 15 empty-drain flushes return ~100× faster and call Index::matching_ids_sorted, which enumerated only store.snapshot().segments with no CollectionPublication reader admission — so all 15 collected ids=[] inside the winner's window and returned total:0 / updated:0 / failures:[]. Grep proof: 15× ids=[] + 1× ids=["counter"], exactly one transform (0→1). flush() itself is sound — the guarded write survived and was served after (32-writer sister test passes); get_document/search were never wrong because they already take reader admission. The defect is that the #1019/#1022 id-enumeration API was not linearized against flush publications; _delete_by_query shares the hole.

Fix (engine, minimal — get_document's own retry idiom): matching_ids_sorted is now async and wraps its enumeration in the same admission bracket (wait out any active publication bracket, enumerate, validate, retry); on Poisoned it returns None so the paged arm's search() surfaces the error. fold_match_sources (sig-text) got the narrow variant: memtable capture + segment snapshot inside one admission, validated before the walk (only captures are retried). .await at the three call sites. Flush/memtable/write paths untouched.

Numbers — fail-before (wrapper reverted): main test left:1 right:16, new engine race test left:[] right:["counter"]. After: main test 20/20 consecutive runs green; paging/exactly-once suite 6/6; xerj-engine lib 725/0, xerj-api lib 273/0, integration 143/0; fmt+clippy clean; ES-YAML 1380 passed · 0 failed · 3 skipped (private :9598, throwaway dir). New tests: engine flush-vs-collector race, plus delete twins at engine and api level.

@xerj-org

Copy link
Copy Markdown
Owner Author

Second CI run on cec66fa0: same test, left: 2 (was 1) — the linearization fix improved the runner outcome but did not close it. Not reproducible locally: 20/20 isolated, 5/5 taskset -c 0-3, 3× full-suite under taskset -c 0-3 (273 tests, all green). The residual loss is runner-shape-only (slower fsync stretching the drain→publish window, oversubscribed vCPUs).

Next: auditing the admission retry loop for a partial/give-up path and the write-flush interaction under 16 concurrent guarded writes; if the audit + local stress don't pin it, pushing a temporary instrumented diagnostic commit so the runner itself prints the interleaving (this PR is already red — a diagnostic run costs nothing), then the real fix. Merge stays held.

… BE REVERTED

Adds [DIAG2] eprintln probes to pin down the runner-only left:2 loss in
concurrent_scripted_update_by_query_preserves_every_increment (CI shape:
4 vCPU, --test-threads=2; 280+ local executions never reproduce):

- es_compat run_update_by_query: entry-flush start/done
- es_compat call sites (ubq + dbq): matching_ids_sorted result count +
  first ids, plus the None -> ped-arm fallthrough
- es_compat transform_one: read_n/write_n around the serialized transform,
  and the Ok(Ok(None)) VANISHED (get_document None) arm
- engine matching_ids_sorted: admission wait-writer / poisoned, per-attempt
  segs/ids/memtbl_docs, validate retry
- engine do_flush_shard: drain-empty, drained docs+ids, commit seg id,
  cancel+restore

All output is stderr, tagged [DIAG2], keyed by data_dir so lines can be
attributed to a test instance. No behaviour change; compile-checked and
the target test still passes locally (16/16 increments).

REVERT ON NEXT COMMIT.
…, WILL BE REVERTED

PR #1023 went merge-dirty against main (es_compat.rs changed on both sides
when #1009/#1021 landed), so pushes to the branch stopped creating
pull_request CI runs entirely - GitHub cannot build refs/pull/1023/merge and
silently creates no run (observed: two pushes, only the CLA and code-scanning
workflows fired). The diagnostic run therefore cannot reach Build + Test the
normal way.

Two temporary triggers, both on the branch ref (our exact tree, no merge):

- diag1023.yml: minimal capture job replicating Build + Test's runner,
  toolchain pin, and rust-cache key - runs the xerj-api lib suite at
  --test-threads=2, then 40 isolated --nocapture iterations of the target
  test. [DIAG2] eprintln fires from tokio worker threads, so it reaches the
  job log whether the test passes or fails.
- ci.yml on:workflow_dispatch: so the full authoritative Build + Test shape
  can also be dispatched on the branch. PR-context steps are if:-gated on
  pull_request and simply skip.

Both are reverted with the instrumentation.
…AG2] run, WILL BE REVERTED"

This reverts commit cbdf138.
…only lost increments)

CI kept failing PR #1023's concurrent update_by_query test after the
cec66fa admission fix: sixteen barrier-released run_update_by_query
calls on one doc still ended n=2 on the 4-vCPU runner (left:2
right:16) while every local shape stayed green. Two instrumented
runner cycles (run 35693271959 / job 106634587318, and the one before
it) captured the interleaving and DISCONFIRMED the drain-race theory:
the admission bracket held on the runner (10 "mis admit wait-writer",
2x "segs=1 ids=1", and every "ids=0" landing only after a tombstone).
Both cycles instead failed the SISTER test,
test_concurrent_delete_by_query_purges_the_document, at its own
assert_eq!(deleted, total) — left:0 right:1.

Root cause — the enumeration is CONSISTENT but not COMPLETE.
matching_ids_sorted enumerated only store.snapshot().segments id maps
(plus version-map liveness). At any reader-admitted instant where a
doc's live copy is still memtable-only — before the first flush drain,
or after a failed finalize restored a drained doc — every segment id
map is silent about it, so the function answered Some([]): a perfectly
consistent EMPTY match set. The by-query arms convert that into a
silent no-op (total:0 / failures:[]), which is exactly the shape of
the runner's left:2: increments vanish while failures stays [].

Why local shapes could not see it: correctness relied on a TIMING
argument. The by-query arms flush on entry, so the old reasoning was
"the first shard task to reach its flush brackets the doc into a
segment before any other task's collect." On a fast local box the
drain→publish→repoint chain completes in microseconds, so every
collect lands after the first publish and the memtable-only instant is
unreachable. On CI's 4-vCPU / --test-threads=2 / slow-fsync runner the
schedule stretches: a collect can be admitted while the doc is still
memtable-only, collect nothing, and no-op — and because the whole
capture is consistent, nothing errors, nothing conflicts, nothing
reports a failure. Forty-plus isolated runs, three full api-lib runs
under taskset -c 0-3 with RUST_TEST_THREADS=2, and pinned-core
full-suite runs never reproduced it locally.

The fix: give the enumeration a memtable leg, captured inside the SAME
validated admission bracket (fold_match_sources' capture shape) —
memtable.all_doc_ids() and store.snapshot() taken as one pair between
try_admit_reader and validate_reader. A drain happens only inside a
publication bracket whose commit follows the snapshot publish, so a
validated pair cannot straddle a drain→publish window: every live doc
is in exactly one leg, and the existing sort+dedup collapses an id a
concurrent update left in both. Memtable ids pass the same Ids
membership filter and the same version-map tombstone skip the segment
leg applies. From this, a doc is enumerated from its memtable copy
from its write until a flush's publish bracket commits, and from its
segment's id map from then on — never from neither. The entry-flush
precondition in the by-query arms is now a performance hint, not a
correctness requirement.

The twice-captured runner failure in the delete twin was our own
too-strict assertion, not lost work: a runner that collected the doc
can lose the delete race to another runner's delete of the same id
(the winner's delete lands between the loser's collect and its
delete_document). The loser's delete is a clean no-op — ES charges
that to version conflicts — and delete_by_query already reports
total.max(deleted). Relaxed to deleted<=total plus
at-least-one-matched / at-least-one-deleted; the final get proving the
purge happened is unchanged.

Tests: new deterministic twin
test_matching_ids_sorted_enumerates_memtable_only_documents (no race,
no flush) fails before the memtable leg with left:[] right:["counter"]
and passes after for match_all, the Ids selector, and a post-flush
exactly-once check.

Gates (ci-test profile, all green):
- fail-before proof: left:[] right:["counter"] on the new twin
- isolated twin x20 (default threads): 20/20
- taskset -c 0-3 x5: 5/5; delete twin taskset -c 0-3 x10: 10/10
- RUST_TEST_THREADS=2 taskset -c 0-3 x20: 20/20
- FULL -p xerj-api --lib taskset -c 0-3 x3: 273 passed x3 (includes
  concurrent_scripted_update_by_query_preserves_every_increment)
- -p xerj-engine --lib: 725 passed; -p xerj-engine --test integration:
  144 passed, 1 ignored (cec66fa's tests green)
- cargo fmt --check: clean
- clippy --profile ci-test -p xerj-engine -p xerj-api --lib --tests
  -- -D warnings: clean
- ES-YAML conformance on a private port with a throwaway data dir:
  1380 passed / 0 failed / 3 skipped

Refs #1022.
@xerj-org

Copy link
Copy Markdown
Owner Author

Real root cause of the runner-only left:2 right:16 — fixed in a750983

What CI showed. After cec66fa (reader admission), the 4-vCPU runner still ended the 16-way concurrent _update_by_query test at n=2 — 14 increments silently vanished with failures: []. Two instrumented runner cycles (e.g. run 35693271959) disproved the drain-race theory: the admission bracket held on the runner (10 mis admit wait-writer, 2× segs=1 ids=1, and every ids=0 landing only after a tombstone). Both cycles instead failed the sister test test_concurrent_delete_by_query_purges_the_document at its own assert_eq!(deleted, total) — a too-strict assertion of ours, not lost work.

Why the first fix was not enough. Admission made every matching_ids_sorted capture consistent but left it incomplete: it enumerated only segment id maps + version-map liveness. At any validated instant where a doc's live copy is still memtable-only (before the first flush drain, or after a failed finalize restored a drained doc), the segment maps say nothing about it — so it answered Some([]), a perfectly consistent EMPTY match set, which the by-query arms turn into a silent no-op (total:0 / failures:[]). That is exactly the left:2 shape.

Why local never saw it. The old argument was a timing one: the by-query arms flush on entry, so "the first task's flush brackets the doc before anyone collects." On a fast box the drain→publish→repoint chain finishes in µs and the memtable-only instant is unreachable; on the runner (4 vCPU, --test-threads=2, slow fsync) a collect can be admitted in that window, collect nothing, and no-op — no error, no conflict, no failure.

The fix (matching_ids_sorted): a memtable leg captured inside the same validated admission bracket (the fold_match_sources capture shape) — memtable.all_doc_ids() + store.snapshot() as one pair. A doc is enumerated from its memtable copy until a flush's publish bracket commits, and from its segment id map thereafter — never from neither. The entry-flush precondition is now a performance hint, not a correctness requirement. cec66fa's admission design is untouched.

The twice-captured delete-twin failure was our own assertion: a runner that collected the doc can lose the delete race to another runner's delete of the same id — a clean no-op ES charges to version conflicts (delete_by_query already reports total.max(deleted)). Relaxed to deleted <= total + at-least-one-matched/deleted; the final purge get is unchanged.

Proof + gates (ci-test profile, all green):

  • New deterministic twin test_matching_ids_sorted_enumerates_memtable_only_documents: fails before the leg with left:[] right:["counter"], green after (match_all, Ids, post-flush exactly-once).
  • Isolated ×20 (default threads) 20/20; taskset -c 0-3 ×5 5/5; delete twin ×10 10/10; RUST_TEST_THREADS=2 taskset -c 0-3 ×20 20/20.
  • FULL -p xerj-api --lib ×3 under taskset -c 0-3: 273 passed each (includes the update_by_query test).
  • -p xerj-engine --lib: 725 passed. -p xerj-engine --test integration: 144 passed, 1 ignored.
  • cargo fmt --check clean; clippy -p xerj-engine -p xerj-api --lib --tests -- -D warnings clean.
  • ES-YAML conformance (private port, throwaway dir): 1380 passed / 0 failed / 3 skipped.

Refs #1022.

@xerj-org
xerj-org merged commit b63debb into main Sep 26, 2026
23 checks passed
xerj-org added a commit that referenced this pull request Sep 26, 2026
Twelve PRs have merged since the rc.77 tag; the [Unreleased] section
carried only the #874 gauges entry. The release-notes gate
(.github/scripts/release-notes-gate.sh, issue #474) fails an rc.78
release PR whose notes do not cite every PR in the previous-tag..head
window, so each of these would have surfaced as a gate failure at cut
time instead of a review comment now.

Entries added (with PR links so the gate's coverage check resolves):
- Fixed: #1015 flush-drain freeze (PR #1018), #950 id-position maps +
  streamed reassembly (PR #1017), #1019 _delete_by_query paging (PR
  #1021), #1022 _update_by_query paging (PR #1023); the existing #874
  entry now cites its PR (#1020).
- Added: POST /{index}/_cache/clear (PR #1009), the systemone
  email-labelling benchmark answering discussion #1012 (PR #1026) with
  its measured numbers (1.000 templated / 0.625 at 0.902 confidence on
  the hard tier).
- Performance: request-cache seen-set lazy allocation, idle 206 -> 64
  kB/idx (PRs #1025, #1034).
- Documentation: README Jev section (PR #1010), the /_decide field
  report (PR #1027), llms.txt status catch-up (PR #1033, closing #1028).

Numbers are quoted only from the PR bodies' own verified runs.
xerj-org added a commit that referenced this pull request Sep 26, 2026
…-09-26)

rc.75-rc.77 shipped rerank, share links, mail ingest, S3-as-index-home and
the #1002/#975/#1023 measurements, and the 2026-09-26 post-merge audit
(58 confirmed findings) found nearly every public surface still denying or
misstating them. llms.txt had been fixed by #1033; this sweep is the rest.

- llms-full.txt: NOT-IMPLEMENTED block rolled to the rc.77 reality (object
  storage removed from it; remaining rows re-stamped 2026-09-26 after
  re-reading the code); the #1017/#1009 dates corrected to each commit's
  own git timestamp; the ellipsis results-URL expanded to a real link.
- ZERO_TOKEN_DIRECTION.md: rerank run pointers now name the actual
  benchmark dirs (RERANK.md carries only the pilot); the autoindex
  resilience row rolled to "fixed in rc.75 (#934)"; object-storage row
  already said rc.77 - kept.
- XERJ_VS_LUCENE.md: _update_by_query "hard-capped at 10,000 hits"
  replaced with the post-#1023 _id-keyset search_after paging.
- docs/RERANK.md: the "min_score means the same thing on every query"
  claim and the min_score example replaced with the measured calibration
  (ECE 0.10-0.31; rank by the probability, never threshold it); the
  egress inventory now records that storage.backend = "s3" writes index
  bundles since rc.77.
- docs/recipes/mail-takeout-mbox.md and the takeout answer: pre-#1002
  memory numbers reframed as dated before-state with the #1002 fix and
  the #1032 residual named.
- answers/compare pages: "no email handler / no mbox handler" blanket
  claims replaced everywhere (mbox/.eml shipped rc.75; PST/Maildir still
  unreadable); capabilities tables and FAQs updated to match.
- does-xerj-support-s3-alerting-plugins: the page llms.txt:50 cites as
  proof of the S3 status still said the opposite - rolled to the rc.77
  reality (both directions work, one remaining startup refusal: an
  unnamed s3_bucket), with the correction stated in the open.
- docs/SCRIPTING.md: every code anchor re-pinned after #1023 and the
  index.rs refactors (script query 26502->43775, script_score 33162->
  51918, script_fields es_compat 11051->14069, fault capture 977->997).
- Landing twins, both compare/answers hubs and the sitemap regenerated
  via scripts/seo/build_articles.py --write; landing/llms.txt article
  index regenerated (title change propagation, 88 files).

Verification: per-group adversarial review (12 groups), then a final
gate - landing-constants-guard all checks passed; build_articles.py
--check ok (189 outputs, 92 articles); gen_sitemap.py --check ok (178
URLs); stale-phrase greps ("no email handler", "local-directory
simulation" as present tense, "memory is the limit today", ...) return
zero hits; the diff touches only docs/, content/ and landing/ - no
CHANGELOG, ROADMAP or engine changes. CHANGELOG entry follows after
rebase on #1035 (single-writer rule on that file).
Amakurai pushed a commit to Amakurai/xerj that referenced this pull request Sep 27, 2026
…, rc.78 queue

The 2026-09-21 review of this file was stale within hours: xerj-org#950 closed
19:33, xerj-org#941 19:38, xerj-org#1015 20:49 (all 2026-09-21), xerj-org#874 2026-09-22, and the
stage-1 gating trio xerj-org#937/xerj-org#938/xerj-org#939 closed the same evening — every one
recorded here as open or "under way". This pass re-checks every status
claim against live tracker state and rolls the file forward:

- Next release: the two "in flight" items (xerj-org#1015, xerj-org#950) are closed with
  fixes on main riding to rc.78 — the section now records what actually
  landed since the rc.77 tag (xerj-org#1009, xerj-org#1017, xerj-org#1018, xerj-org#1020, xerj-org#1021, xerj-org#1023,
  xerj-org#1025, xerj-org#1026, xerj-org#1033, xerj-org#1034) with PR links, including xerj-org#874's budget met
  at ~3x margin (206 -> 64 kB per idle index) and discussion xerj-org#1012's
  email-labelling measurement (1.000 templated tier / 0.625 at 0.902
  confidence on the hard tier — wrong-and-confident).
- Open defects: now xerj-org#1031 and xerj-org#1032 only; xerj-org#1015/xerj-org#950 removed (closed);
  trackers sentence corrected (xerj-org#941, xerj-org#874 closed — only xerj-org#298 remains by
  design).
- GA gate: "Close the CHANGELOG gap" marked closed 2026-09-26 (the
  rc.19-rc.70 backfill, PR xerj-org#1035); the record-complete note replaces the
  gap warning under "Shipping today".
- Zero-token: stage-1 gating trio recorded as fixed (xerj-org#991, xerj-org#995, xerj-org#979)
  with the old measured costs kept as the before-state; the xerj-org#940
  hash-seed spread annotated as a pre-fix measurement; stage-2 object
  storage marked done (xerj-org#965 wired in rc.77 — the old bullet contradicted
  this file's own "Shipping today" section); mail ingest "unreleased"
  corrected to "shipped in rc.75"; three stale "In flight:" labels on
  landed stage-1 items relabelled "History:".
- Mail-ingest memory line: xerj-org#948's runaway is fixed (xerj-org#1002) — the line
  now carries the fixed numbers and points at xerj-org#1032 for the residual.

Review line updated to 2026-09-26 (machine-checked format), scoped
honestly as a desk review. The milestones line is true again: rc.78
milestone created, all four open issues triaged (xerj-org#1030/xerj-org#1031/xerj-org#1032 ->
rc.78, xerj-org#298 -> GA).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_update_by_query still one-shots size:10_000 (and two more single-shot by-query sites) — the #1019 truncation class lives on in the siblings

1 participant