Monitor.match can silently miss a query while Monitor.register is replacing it

A writable Monitor keeps each registered query twice: its clauses as documents in an internal query index, and the same clauses in an in-memory cache keyed by query id + _ + the clause's position. Monitor.register replaces a query by id, but it writes the new version's clauses into the live cache before its new index documents are committed and the searcher is refreshed. A Monitor.match in between pairs the old searcher with the updated cache, so when the edit moves the matching clause to another position, an old index document resolves to a different clause of the new version and the query is silently dropped, although both versions match the document. It is transient (matches that overlap one register call) and limited to the ids in that call, needs no hooks, and reproduces on 12 releases from 8.2.0 to 10.5.2 and on main.

• Monitor: the lucene/monitor module matches each incoming document against a set of registered (standing) queries: register(MonitorQuery) adds or replaces a query by id, match(Document, MatcherFactory) returns the ids of the queries that match.
• Query index and presearcher: the Monitor splits each query into clauses (QueryDecomposer; in clause order since #14885) and indexes one document per clause in an internal index, under the clause's terms. A match first searches that index with the document's terms (the presearcher) to pick candidate clauses.
• Query cache and cache id: the clauses themselves sit in an in-memory map. The key, the cache id, is the query id + _ + the clause's position (QueryCacheEntry.java:56), so every version of q uses q_0, q_1, ... Each candidate document carries its cache id; the matcher looks the clause up in the map and runs it against the document.

1. commitWithoutNotify (:107-133, under the commit lock and the purge read lock) deletes the old documents of the ids (:117-119), then for each new clause puts the cache entry into the live map (:121) and adds its document (:122); writer.commit() (:127) and manager.maybeRefresh() (:128) come last. The searcher is an NRT SearcherManager over the writer (:79): only the refresh makes the deletes and new documents visible.
2. search() (:206-219) takes the current searcher and the same live map together under the purge read lock (:211-217) and uses both for the whole search; the collector resolves each hit with queries.get(cache_id) (:384), and StandardQueryCollector runs that clause (Monitor.java:294-303).
3. Between the first put and the moment the refresh swaps in the new searcher, a match that starts gets the old searcher, which holds the old q_0 document (indexed under apple) while the map says q_0 = f:cherry. The presearcher picks q_0 for a document apple, the matcher runs f:cherry, no match; the new q_1 = f:apple is not in the old searcher. An executed probe shows the matcher running exactly one candidate, f:cherry, with the registering thread parked at the start and at the end of the IndexWriter commit; an independent repro shows it parked inside addDocument and inside Directory.sync. A match that took the old searcher keeps it until it finishes, even if register returns meanwhile (by reading).
4. Matches that captured the map before register started and are still collecting can see the overwrite too (a first fix attempt that moved the put later but still changed the live map kept failing).
5. Only false negatives (every test) and only the ids in the call (by reading, for one re-registration of one id): a hit runs the old or the new version's clause, and the put touches only the cache ids of the queries in the call (in a multi-id register(Iterable), every id of that batch). Not characterized: a long match that spans several re-registrations, and entries above a shorter version's clause count, which stay in the map until a purge.
6. Moving the put after the refresh only reverses the window (new searcher, old map); the map and the searcher have to be published together (by reading).

• Who is hit. Code that calls Monitor.match while another thread calls Monitor.register for an existing id: a percolation or alerting service that keeps matching a document stream while users edit standing queries. Upstream #12801 treats match running concurrently with register/deleteById and purgeCache as a use that must stay fast ("they should be able to execute concurrently"); its demo re-registers existing ids while matching.
• Default configuration. new Monitor(analyzer) with the in-memory directory (MonitorConfiguration.java:41), the default TermFilteredPresearcher (Monitor.java:56-57, :76-77) and decomposer. An on-disk monitor is not the default (setIndexPath or setDirectoryProvider) but shares the code path: the doc's test with setIndexPath and a serializer fails 2 of 3 with assertions on and off, the control passes (fact check). The read-only Monitor has no register path and was not examined.
• What it takes. A document that both versions match and an edit that moves the matching clause to another position: a new leading clause (apple to cherry OR apple), a reordered or shortened disjunction. Before 10.3.0 positions came from hash order, so which edits move a clause depended on the JVM's hash seed.
• Effect. A silent missed match: no exception and nothing in MatchingQueries.getErrors(). Other queries are not affected (by reading). After register returns, the same match reports the query again.
• Window size. One register call: an addDocument per clause, a commit (with fsync on disk directories) and a refresh that rebuilds the presearcher's term filter over every term of the query index (QueryIndex.java:142-156). In a short probe on stock main (in memory, shared host), re-registering one query took a median of about 5-6 ms with 10,000 standing queries and 18-24 ms with 100,000. A bulk register(Iterable) commits once it holds more than 5,000 updates, so a full batch is 5,001 queries (Monitor.java:152-162, MonitorConfiguration.java:35), and the window covers a whole batch (by reading).
• Frequency. No field rate was measured. The 83-96% miss rates of the stress tests come from an updater that is always inside register, not from real traffic; nobody has reported the bug since 8.2.0 (2019).

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: q matched 'apple' during re-register expected:<true> but was:<false>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:120)
       org.apache.lucene.monitor.TestMonitorMatchDuringReRegister.testMatchOnOtherThreadWhileReRegisterCommits(TestMonitorMatchDuringReRegister.java:80)

• The pairing is deliberate. search() takes manager.acquire() and the map together under the purge read lock (WritableQueryIndex.java:211-217). That is the fix of luwak#68 case 2 in the predecessor project (2015, "Get searcher and query cache within a purge lock"). register mutates the shared map before its documents are visible, so the pair no longer belongs to one state.
• Concurrent match and register is an anticipated use (by the design and the upstream history; no javadoc promises it). The implementation note at WritableQueryIndex.java:276-290 arranges the locks so searches do not wait for commits: "...searches waiting on purge waiting on commit which is problematic since commit is slow and search cannot be slow." #12801 (CHANGES: "Remove possible contention on a ReentrantReadWriteLock in Monitor which could result in searches waiting for commits") was about "multi-second latencies in the Monitor.match function when it is called concurrently with Monitor.register/Monitor.deleteById and QueryIndex.purgeCache". purgeCache "is normally called from a background thread" (Monitor.java:131). The module's author wrote in luwak#174: "You should only have a single Monitor object, but you can index and query it from multiple threads".
• Missed matches under concurrent use were fixed as bugs. LUCENE-9791 (#10830) is recorded as "fixing a Luwak/Monitor bug causing registered queries to sometimes fail to match" (lucene/CHANGES.txt:5386-5388). It concerned match calls running concurrently with each other (BytesRefHash#find), not register.
• Replacing by id is the tested update path: TestMonitor.testUpdatesOverwriteOldQueries (TestMonitor.java:71-86) and the delete-by-id loop in commitWithoutNotify.
• No state gives this answer. Both versions match the document, q matches before and after, and every serial order reports q; a stale-but-consistent answer would still report it. This assumes register takes effect at one point (linearizable), the model of the controlled-concurrency oracle and the test; without a documented contract a maintainer could call an in-flight replacement a state in which neither version is guaranteed.
• Origin, not design. The predecessor library (Luwak) used the same put-first order but keyed cache entries by a content hash of the query and its metadata plus _ + the position, so a new version never overwrote an entry the old searcher used. The port (LUCENE-8766) switched to id + position keys when the Monitor began taking Query objects directly and kept the order; its JIRA thread does not discuss matches during updates.

The strongest works-as-designed case (raised by the cross-model reviewers and the devil's-advocate checker): no javadoc says Monitor is thread-safe or that a match sees a snapshot (Monitor.java:38-41, register, match, package-info); ConcurrentQueryLoader.java:37-38 says "You can use multiple threads to parse and index queries before starting matches"; StandardQueryCollector silently skips a missing cache entry (Monitor.java:296); a read-only Monitor returns stale results until it refreshes (TestMonitorReadonly).

Why it fails: the ConcurrentQueryLoader sentence describes that helper's startup use, not a precondition of register, and the lock design and #12801 treat matching during updates as normal, and LUCENE-9791 treated missed matches under concurrency as a bug; the null guard covers a missing entry, while here the entry is present but belongs to the other version; read-only staleness is a consistent old state, while this answer matches no state. What remains is a contract-wording point (the contract is implied by the design, not documented) and a severity point (transient), not a validity point.

Two variants were implemented and validated in a scratch checkout of main; both keep the commit outside the purge write lock, as #12801 requires. A third design and a non-fix are listed for completeness.

+39/-5 in WritableQueryIndex.java. A commit copies the map, adds the new entries (and logs them in the purge's register log as today) and sets this.queries = next instead of mutating the live map. The SearcherFactory (a TermsHashBuilder subclass) binds the current map to each new reader in a ConcurrentHashMap<IndexReader.CacheKey, Map<...>> with a closed listener, the same way termFilters is kept. search() resolves its hits with the map bound to the searcher it acquired. purgeCache rebinds the current reader to the purged map under the commit lock, so no refresh can run in between. A bound map is never changed.

// commitWithoutNotify: copy on write instead of mutating the live map
ConcurrentMap<String, QueryCacheEntry> next = new ConcurrentHashMap<>(this.queries);
for (Indexable update : indexables) {
  next.put(update.queryCacheEntry.cacheId, update.queryCacheEntry);
  writer.addDocument(update.document);
}
this.queries = next;
writer.commit();
manager.maybeRefresh(); // the searcher factory binds this.queries to the new reader

// search(): resolve hits with the map bound to the acquired searcher
searcher = manager.acquire();
queries = cacheByReader.get(searcher.getIndexReader().getReaderCacheHelper().getKey());

Trade-offs

• One map copy per register call (or per batch of about 5,000 in a bulk call). In a short probe (100,000 standing queries, q re-registered 200 times back to back, one matching thread, two runs on a loaded shared host): register median 26-35 ms vs 18-24 ms on stock; match p99 0.10-0.12 ms vs 0.13-0.23 ms on stock. Indicative only, not a benchmark.
• While an old searcher is in use, its map stays alive too (one extra map of pointers per open generation).
• New code: validated by tests and the probe, not by the controlled-concurrency exploration that variant A got. Not tested: failure paths (an addDocument, commit or maybeRefresh that throws), the interaction with LFV-499, and a hook-free run on disk.

What the checks validated

• The new test passes (gradle with assertions on; standalone with assertions on and off, 30 of 30 runs with 10 iterations); the hunt's JUnit, the independent reproducer's window test (parks inside addDocument and Directory.sync) and the hook-free stress test (0 of 34,413 and 0 of 26,373 misses) pass.
• The whole monitor module: 33 suites, 184 tests (181 upstream + 3 new), 0 failures, assertions on and off; the closest suites with nightly tests (29 tests); :lucene:monitor:check -x test (google-java-format, ecjLint, forbiddenApis).
• The independent fact check re-ran the module (184 tests, assertions on and off), check, the new test (standalone, assertions off) and the hook-free stress test (0 of 14,767 misses).

+18/-3 in WritableQueryIndex.java (the hunt's sketch). addDocument and commit stay under the read lock; then, under the write lock that search() takes as a read lock, the commit copies the map, adds the new entries (and logs them during a purge), publishes it and calls maybeRefresh().

Trade-offs

• Searches wait for every refresh, which rebuilds the term filter over every term of the query index (QueryIndex.java:142-156). In the same probe: match p99 21-34 ms (max 74-103 ms) vs 0.13-0.23 ms on stock; register median 27-35 ms. That is the kind of search stall #12801 removed.
• Same map copy per register call as B.
• Failure path (by reading, not tested): this.queries = next is set before maybeRefresh(); if the refresh throws, the old searcher stays current with the new map until the next successful refresh.

What the checks validated

• The new test (standalone with assertions on and off, gradle with them on; no 10-iteration run), the hunt's JUnit, the hook-free stress test (0 of 27,629 misses), the module's 184 tests (assertions on and off), the nightly closest suites and check; the independent reproducer's window test passes with its own sketch, the same code with other comments.
• Controlled-concurrency exploration (Fray) of the real Monitor, on ea57360912ed jars (the monitor module is identical): 0 violations in 420 scenarios (12,200 schedules), 160 with this fix alone and 260 with it plus a mask for the separate LFV-501 race; without the fix the same scenario seeds violate in 27 of 100 (stock) and 21 of 100 (LFV-501 mask only).

Give each version's entries keys that the old searcher's documents cannot collide with, as the predecessor library's content-hash keys did; old entries then live until the next purge.

Trade-offs

On-disk monitors rebuild the cache at open by re-decomposing the stored queries (WritableQueryIndex.java:137-181), so the key must be re-derivable and stable across JVMs (#14886 is the caution), and existing indexes store id_N ids.

Putting the entries after maybeRefresh() reverses the window (a new searcher with the old map). The null guard in StandardQueryCollector does not help: the entry is present but belongs to the other version.

Trade-offs

Both by reading; not executed.

Method: controlled-concurrency exploration with a linearizability oracle (the hunt's "semi-formal" tier), in a hunt unit on Monitor query-index concurrency.

• The plan's lead came from code reading: the cache snapshot and the searcher a search uses should belong to one generation (WritableQueryIndex.java:121-128 versus :211-215).
• Lincheck, the planned model checker, could not run Lucene's indexing under its instrumentation (a tool limitation). The finder used Fray 0.9.0 (POS, PCT and random schedulers) on the stock jars: one writable Monitor, 2-3 threads running random register, match, delete, clear, purge and getQueryIds operations, and an exact linearizability oracle against a sequential model (state = the registered version per id; match(doc) = the ids whose version has a term of doc).
• Results: 19 of 60 register-and-match scenarios violate the oracle on the stock jars, 27 of 100 with purges and getQueryIds, 21 of 100 with every operation (with a fix for the separate delete/clear race masked in); 0 of 420 scenarios (12,200 schedules) with fix variant A. An unscheduled plain run of the same harness also hit it once.
• The finder then wrote a deterministic JUnit (an InfoStream hook parks register at the IndexWriter commit), and an independent reproducer, reviewers from another model family (OpenAI GPT-5.6, reading only), a devil's advocate, an adjudication and an audit checked it.