Hi all, I'd like feedback on letting soft commits run without DirectUpdateHandler2 <https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java>'s updateLock, before opening a JIRA.
*Background* Today, commit() holds updateLock across preSoftCommit -> getSearcher -> postSoftCommit because the tlog's realtime-get maps are rotated and cleared during this window. A comment in UpdateLog.postSoftCommit points to the alternative: "record what old maps were created and only remove those". I prototyped this approach. Since Lucene is already MVCC-like (immutable segments, snapshot searchers), this narrows the gap on the update-log side. *Summary of Changes (Opt-in classes; default behavior is unchanged)* - SeqNoUpdateLog: Tags each realtime-get entry with a per-core sequence number. preSoftCommit records a watermark, and postSoftCommit trims only entries at or below it, preventing concurrent commits and updates from interfering. (Lucene's seqNos are per IndexWriter, so they cannot be used directly.) - SeqNoDirectUpdateHandler: Runs plain soft commits without updateLock. A small separate lock keeps reordered replica adds (addAndDelete) atomic relative to a soft commit. - Hard commits, prepareCommit, optimize, and expungeDeletes retain the stock path. *Correctness (Solr 9.7.0 / Lucene 9.11.1)* - Targeted Tests: New tests force specific interleavings with pause points inside DirectUpdateHandler2, UpdateLog, and lookupVersion: soft commit inside a DBQ, reordered adds vs. commits, overlapping soft commits, updates between watermark and searcher open, IndexWriter replacement, reload, recovery, and PeerSync. All pass as on stock Solr. - Linearizability: Realtime get vs. /select showed 0 violations and 0 deadlocks over 29M reads in a 40-iteration soak. - Removing any one of the 8 safety mechanisms causes the test suite to fail. - TestStressVersions and TestStressReorder pass. *Performance Evaluation* (Single node, 1M docs, 2,000 docs/s, 1 DBQ/s) Share of time updateLock is held by soft commits: - 1 s soft commit: Stock 3.6% | SeqNo 0% - 5 s soft commit: Stock 0.62% | SeqNo 0% - 30 s soft commit: Stock 0.07% | SeqNo 0% Key Observations: - Commit duration is unchanged (this removes lock contention, not work). Added cost per commit: trim p50 0.06–0.21 ms, max 3.4 ms. - At 1 s soft commits, updateLock wait drops from ~1.7 s per 5-minute run to 0. Add/DBQ median latency shifted -6% to -9% (within run-to-run noise). At 5 s and 30 s, there is no measurable difference. - Big documents (300 words, 30 s soft commit, DBQ every second; 7 stock vs. 6 seqno runs): DBQ p99 dropped from 678 ms to 537 ms, DBQ max dropped from 2,058 ms to 795 ms, and add max dropped from 2,333 ms to 828 ms. Stock showed a >= 2 s DBQ in 6 of 7 runs; seqno showed 0 of 6. (Treating as suggestive, as these spikes are not fully explained.) - DBQ bursts (every 250 ms) and overload performed identically within noise limits. Throughput gain is minor at typical soft-commit intervals. The primary benefit is removing a lock from the commit path—which matters most for short soft-commit intervals—making the locking model easier to reason about. *Questions* 1. Is removing updateLock from soft commits a change the project would support? 2. Would this be preferred as new opt-in classes, or as an update to UpdateLog's map rotation mechanism directly? 3. Is there existing history (a JIRA or mailing list discussion) explaining why the lock spans the full soft commit that I should review? Happy to open a JIRA and draft PR against main. Please let us know. Thanks. Neeraj Chandraprakash Shukla, Slack (Salesforce)
