Skip to content

perf(indexing): soft-commit instead of hard-commit, keeping documents searchable - #196

Merged
epugh merged 1 commit into
apache:mainfrom
adityamparikh:feat/indexing-batch-guidance
Sep 16, 2026
Merged

perf(indexing): soft-commit instead of hard-commit, keeping documents searchable#196
epugh merged 1 commit into
apache:mainfrom
adityamparikh:feat/indexing-batch-guidance

Conversation

@adityamparikh

@adityamparikh adityamparikh commented Sep 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

indexDocuments ended every indexing tool call with solrClient.commit(collection) — a hard commit, which fsyncs the segments, so each call waited on the storage device. All four indexing tools (index-json-documents, index-csv-documents, index-xml-documents, index-markdown-documents) funnel through it, so that one line governed the whole indexing surface.

Solr's own _default configset does the opposite: autoCommit at maxTime 15000 with openSearcher=false — a background hard commit purely to truncate the transaction log — and autoSoftCommit at 3000 for visibility. Forcing a synchronous fsync per tool call fought that design.

The commit is now commit(collection, waitFlush=false, waitSearcher=true, softCommit=true).

  • Searchability is preserved. waitSearcher=true keeps the guarantee that matters to a tool caller: the documents are searchable the moment the call returns. Verified 30/30 with zero delay.
  • Durability is unchanged. The transaction log is written on the add, before any commit, so documents survive a crash regardless of commit mode. A hard commit governs how much tlog must be replayed on recovery, not whether data is lost; that housekeeping stays with autoCommit.

Measurement

20 interleaved reps against Solr, same endpoint and document, only the commit parameter varying:

commit median p90
none 4.05 ms —
soft 8.61 ms 10.65 ms
hard 18.94 ms 41.32 ms

2.2x faster and far tighter — the hard commit's p90 is four times its median, which is fsync variance. Over the MCP tools, 61 single-document calls go from 1853 ms to 585 ms.

Operator note

A custom configset with autoCommit disabled should enable it, or the transaction log grows until something else commits. The _default configset already does.

Tests

IndexingServiceTest.indexDocuments_SoftCommitsSoDocumentsAreSearchableWithoutForcingAnFsync pins the soft-commit overload and asserts the hard-commit overload is never used; the existing stubs and verifications in that class move to the four-argument overload. ./gradlew build: 404 tests, 0 failures.

🤖 Generated with Claude Code

This was referenced Sep 12, 2026
Closed
Open
adityamparikh added a commit to adityamparikh/solr-mcp that referenced this pull request Sep 14, 2026
@adityamparikh @claude
test(indexing): no commit stub in the markdown array test
8f9cde3 
The mock returns null without it, and a strict stub on the one-argument
commit would be flagged unnecessary once apache#196's soft commit lands.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CiUHyyXLTo9ATdgg8eRFZJ
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
@adityamparikh
adityamparikh force-pushed the feat/indexing-batch-guidance branch 2 times, most recently from 4b0815f to 44ea747 Compare September 15, 2026 18:34
@adityamparikh adityamparikh changed the title feat(indexing): advise payload splitting and soft-commit per call perf(indexing): soft-commit instead of hard-commit, keeping documents searchable Sep 15, 2026
@adityamparikh adityamparikh mentioned this pull request Sep 15, 2026
Open
@adityamparikh @claude
perf(indexing): soft-commit instead of hard-commit, keeping documents…
275a1da 
… searchable

Every indexing tool -- JSON, CSV, XML and markdown -- funnels through
indexDocuments, and indexDocuments ended with solrClient.commit(collection).
That is a hard commit: it fsyncs the segments, so each tool call waited on the
storage device.

That is not what Solr's own defaults do. The _default configset ships autoCommit
at maxTime 15000 with openSearcher=false, and autoSoftCommit at 3000: a
background hard commit purely to truncate the transaction log, and soft commits
for visibility. Forcing a synchronous fsync per tool call fought that design.

The commit is now waitFlush=false, waitSearcher=true, softCommit=true.
waitSearcher keeps the guarantee that matters to a tool caller: the documents
are searchable the moment the call returns. Verified 30/30 with zero delay.

Durability is unchanged. The transaction log is written on the add, before any
commit, so documents survive a crash regardless of commit mode; a hard commit
governs how much tlog must be replayed on recovery, not whether data is lost.
That housekeeping stays with autoCommit.

Measured against Solr, 20 interleaved reps, same endpoint and document, only
the commit parameter varying:

  no commit      4.05 ms median
  soft commit    8.61 ms median, p90 10.65
  hard commit   18.94 ms median, p90 41.32

2.2x faster and far tighter -- the hard commit's p90 is four times its median,
which is fsync variance. Over the MCP tools, 61 single-document calls go from
1853 ms to 585 ms.

Operators running a custom configset with autoCommit disabled should enable it,
or the transaction log grows until something else commits.

Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@adityamparikh
adityamparikh force-pushed the feat/indexing-batch-guidance branch from 44ea747 to 275a1da Compare September 16, 2026 13:45
adityamparikh added a commit to adityamparikh/solr-mcp that referenced this pull request Sep 16, 2026
@adityamparikh @claude
perf(indexing): soft-commit the CSV and XML forward as well
8ad6834 
The commit riding on the /update request was a hard one
(setAction(COMMIT, waitFlush=true, waitSearcher=true)), so every CSV or XML
tool call fsynced the segments and waited on the storage device.

That is not what Solr's own defaults do. The _default configset ships autoCommit
at maxTime 15000 with openSearcher=false, and autoSoftCommit at 3000: a
background hard commit purely to truncate the transaction log, and soft commits
for visibility. Forcing a synchronous fsync per tool call fought that design.

The action is now setAction(COMMIT, waitFlush=false, waitSearcher=true,
softCommit=true). waitSearcher keeps the guarantee that matters to a tool
caller: the documents are searchable the moment the call returns. Durability is
unchanged -- the transaction log is written on the add, before any commit, so
documents survive a crash regardless of commit mode; a hard commit governs how
much tlog must be replayed on recovery, not whether data is lost.

Measured against Solr, 20 interleaved reps, same endpoint and document, only
the commit parameter varying:

  no commit      4.05 ms median
  soft commit    8.61 ms median, p90 10.65
  hard commit   18.94 ms median, p90 41.32

2.2x faster and far tighter -- the hard commit's p90 is four times its median,
which is fsync variance. Over the MCP tools, 61 single-document CSV calls go
from 1853 ms to 585 ms.

This is the CSV/XML half of the same change apache#196 makes to indexDocuments, which
covers the JSON and markdown tools. Carrying it here keeps the four indexing
tools consistent whichever of the two PRs merges first.

Operators running a custom configset with autoCommit disabled should enable it,
or the transaction log grows until something else commits.

Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@adityamparikh adityamparikh mentioned this pull request Sep 16, 2026
Merged
@epugh

epugh commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

What happens if you do a softCommit, but the softCommit plumbing isn't set up? Can we confirm that IF you use solrcloud you have soft commit set up.... (or normally do at least!). You may have found a weakness, what happens if you request a soft commit and we haven't set up a trnasaction log etc???/. This might actually be an issue at the SolrJ layer?

@epugh

epugh commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

What happens if you do a softCommit, but the softCommit plumbing isn't set up? Can we confirm that IF you use solrcloud you have soft commit set up.... (or normally do at least!). You may have found a weakness, what happens if you request a soft commit and we haven't set up a trnasaction log etc???/. This might actually be an issue at the SolrJ layer?

actually, turns out according to ref guide that you MUST have tlog set up in solrcloud mode!

epugh
epugh approved these changes Sep 16, 2026
View reviewed changes

@epugh epugh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, talked it through.

@epugh
epugh merged commit e8d678c into apache:main Sep 16, 2026
1 check passed
adityamparikh added a commit to adityamparikh/solr-mcp that referenced this pull request Sep 16, 2026
@adityamparikh @claude
perf(indexing): soft-commit the CSV and XML forward as well
0970747 
The commit riding on the /update request was a hard one
(setAction(COMMIT, waitFlush=true, waitSearcher=true)), so every CSV or XML
tool call fsynced the segments and waited on the storage device.

That is not what Solr's own defaults do. The _default configset ships autoCommit
at maxTime 15000 with openSearcher=false, and autoSoftCommit at 3000: a
background hard commit purely to truncate the transaction log, and soft commits
for visibility. Forcing a synchronous fsync per tool call fought that design.

The action is now setAction(COMMIT, waitFlush=false, waitSearcher=true,
softCommit=true). waitSearcher keeps the guarantee that matters to a tool
caller: the documents are searchable the moment the call returns. Durability is
unchanged -- the transaction log is written on the add, before any commit, so
documents survive a crash regardless of commit mode; a hard commit governs how
much tlog must be replayed on recovery, not whether data is lost.

Measured against Solr, 20 interleaved reps, same endpoint and document, only
the commit parameter varying:

  no commit      4.05 ms median
  soft commit    8.61 ms median, p90 10.65
  hard commit   18.94 ms median, p90 41.32

2.2x faster and far tighter -- the hard commit's p90 is four times its median,
which is fsync variance. Over the MCP tools, 61 single-document CSV calls go
from 1853 ms to 585 ms.

This is the CSV/XML half of the same change apache#196 makes to indexDocuments, which
covers the JSON and markdown tools. Carrying it here keeps the four indexing
tools consistent whichever of the two PRs merges first.

Operators running a custom configset with autoCommit disabled should enable it,
or the transaction log grows until something else commits.

Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
epugh pushed a commit that referenced this pull request Sep 16, 2026
@adityamparikh @claude @junie-agent
refactor(indexing): forward CSV and XML to Solr's own update handlers (…
d119ce6 
…#205)

* refactor(indexing): forward CSV and XML to Solr's own update handlers

Solr already parses CSV and its own update XML. The server's CSV and XML
document creators re-implemented that, and the XML one did it with a
convention of its own: a generic <shows><show> mapping that prefixed
every field with the record element (show_title) and dropped the id, so
XML was the one format whose documents did not match the same data in
JSON, CSV or Markdown.

Both creators are removed, along with the orchestrator's CSV/XML paths
and the commons-csv dependency. The two tools now forward the payload,
as given, to Solr's /update handler:

- index-csv-documents sends the CSV with header=true. Solr reads the
  header for the field names, so column names are used as given.
  Repeated column names are multi-valued fields and empty cells are
  skipped.
- index-xml-documents takes Solr update XML, <add><doc><field
  name="...">, the format every Solr user already has.

Solr's update XML grammar is a command language: <delete>, <commit>,
<optimize> and <rollback> go to the same endpoint as <add>, so a tool
that forwarded blindly would let an indexing call delete a collection.
SolrUpdateXml.requireAddBlock reads the payload with a hardened StAX
parser (DTD off, external entities off) only as far as the root element
and rejects a DOCTYPE or any root but <add>; Solr parses the rest.

Solr's update response carries a status and a QTime but no document
count, so the two tools report that Solr accepted and committed the
payload instead of an invented "indexed N of N", and the index-data
prompt points at the health check for the count.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJSxr89SRAa7BTC8Jg27Rj
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>

* test(data): add CSV and XML show sample datasets and cross-format integration tests

Port shows.csv, shows.xml, and ShowsSampleDataIntegrationTest from PR #201.
The integration test verifies that JSON, CSV, and XML datasets index the
exact same 61 documents into Solr.

Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
Co-authored-by: Junie <junie@jetbrains.com>

* refactor(indexing): use SolrJ and Spring constants, drop one Solr round trip

Follow-up cleanup on the CSV/XML pass-through, no behaviour change beyond
the commit folding below.

Reuse:
- The XML content type was spelled out as a literal; SolrJ ships it as
  ClientUtils.TEXT_XML with exactly that value. (The CSV side keeps its
  literal: ContentStreamBase.TEXT_CSV is private.)
- SolrUpdateXml configured an XMLInputFactory by hand. Spring's
  StaxUtils.createDefensiveInputFactory() sets the same two properties and
  additionally installs a no-op XMLResolver, so it is strictly more
  hardened. It is now a static field: XMLInputFactory.newFactory() runs a
  ServiceLoader scan of the whole classpath on every call, and the factory
  is never reconfigured after construction, which is the sharing contract
  StAX requires.
- IndexingServiceIntegrationTest is a @SpringBootTest that overwrote its
  own @Autowired beans with hand-built ones, under a comment claiming it
  is not a Spring Boot test. IndexingService is @observed, so the test was
  exercising an unproxied object rather than the one the application runs.

Efficiency:
- forward() posted the payload and then posted a separate commit. The
  commit now rides on the same request via setAction(ACTION.COMMIT), which
  removes a round trip per CSV/XML call and makes the status and QTime the
  message reports actually cover the commit it claims. The two mock tests
  that pinned the second call now assert commit=true on the request.

Simplification:
- describeIndexedFields/describeFieldNames was split when three tools
  reported field names; only the JSON tool does now, so it is one method
  again. The indexDocuments javadoc that had drifted onto a constant is
  reattached.
- SolrUpdateXml's ClosingReader record existed only to make one
  XMLStreamReader try-with-resources-able; the source is an in-memory
  StringReader and XMLStreamReader.close() does not close it.
- Removed the emptied "Apache Commons" heading left by dropping
  commons-csv.

Docs that still described the deleted parsers: IndexingDocumentCreator's
class javadoc, the test tree in dev-docs/ARCHITECTURE.md, and the FAQ's
claim that CSV and XML get field sanitization and 10 MB guards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014XuwC2kdJ6Q1dDDZMHDCee
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>

* refactor(native): register test fixtures via TestRuntimeHintsRegistrar

The sample data was made readable from the native test binary with a raw
hosted option in the Gradle args list:

    -H:IncludeResources=shows\.(csv|xml)$

That is the wrong seam and the wrong scope. dev-docs/graalvm-native-image.md
says to add a targeted registration through a hints registrar and prefer it
over other mechanisms "so the rule is explicit and reviewed", so the next
person adding a fixture does not find the precedent where the docs point.
And the rule being encoded is not the name of one dataset.

Replaced with a TestRuntimeHintsRegistrar wired through
src/test/resources/META-INF/spring/aot.factories, which spring-test invokes
for every test class. It lives in test sources deliberately: SolrNativeHints
is the production equivalent and must not ship test fixture names in the
application image.

The patterns name the three fixtures rather than matching by extension.
registerPattern compiles * to .*, which crosses /, so a *.xml here would
embed all 124 XML resources present on this project's 210-jar test
classpath, declaring every dependency's XML to be a test fixture.

Registering shows.json alongside its siblings is not redundant: Spring AOT
registers .*\.json globally, so the JSON fixture was covered by the
framework while the CSV and XML ones were not. Covering all three in one
place makes the dataset intentional rather than two-thirds accidental.

Verified with ./gradlew nativeTest -Pnative: 244 passed, 0 failed, 142
skipped (skips unchanged from baseline), with
ShowsSampleDataIntegrationTest executing natively. A negative control with
the registrar unregistered fails that test alone, on
"NullPointerException: missing test resource /shows.csv", confirming the
registration is load-bearing and not a no-op replacing an unnecessary flag.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014XuwC2kdJ6Q1dDDZMHDCee
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>

* perf(indexing): soft-commit the CSV and XML forward as well

The commit riding on the /update request was a hard one
(setAction(COMMIT, waitFlush=true, waitSearcher=true)), so every CSV or XML
tool call fsynced the segments and waited on the storage device.

That is not what Solr's own defaults do. The _default configset ships autoCommit
at maxTime 15000 with openSearcher=false, and autoSoftCommit at 3000: a
background hard commit purely to truncate the transaction log, and soft commits
for visibility. Forcing a synchronous fsync per tool call fought that design.

The action is now setAction(COMMIT, waitFlush=false, waitSearcher=true,
softCommit=true). waitSearcher keeps the guarantee that matters to a tool
caller: the documents are searchable the moment the call returns. Durability is
unchanged -- the transaction log is written on the add, before any commit, so
documents survive a crash regardless of commit mode; a hard commit governs how
much tlog must be replayed on recovery, not whether data is lost.

Measured against Solr, 20 interleaved reps, same endpoint and document, only
the commit parameter varying:

  no commit      4.05 ms median
  soft commit    8.61 ms median, p90 10.65
  hard commit   18.94 ms median, p90 41.32

2.2x faster and far tighter -- the hard commit's p90 is four times its median,
which is fsync variance. Over the MCP tools, 61 single-document CSV calls go
from 1853 ms to 585 ms.

This is the CSV/XML half of the same change #196 makes to indexDocuments, which
covers the JSON and markdown tools. Carrying it here keeps the four indexing
tools consistent whichever of the two PRs merges first.

Operators running a custom configset with autoCommit disabled should enable it,
or the transaction log grows until something else commits.

Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Junie <junie@jetbrains.com>
adityamparikh added a commit to adityamparikh/solr-mcp that referenced this pull request Sep 16, 2026
@adityamparikh @claude
Merge upstream/main (apache#196 apache#198 apache#187 apache#202 #98 #…
8e16303 
…103 #104 apache#205) into sb4

Resolves the conflicts against main and ports what merged cleanly but
would not compile on the Boot 4 / Jackson 3 line:

- gradle/libs.versions.toml: keep the Boot 4 test starters and the
  mcp-sdk pin, add spring-security-test (apache#187), drop commons-csv (apache#205).
- JsonDocumentCreator: take apache#202's flatten()/toDocument() split with
  Jackson 3 imports and JacksonException.
- CollectionServiceIntegrationTest / IndexingServiceIntegrationTest:
  take main's typed-documents and autowired-creator setup.
- TestDocuments, JsonDocumentCreatorTest (new on main): com.fasterxml ->
  tools.jackson, JsonMapper.builder().build(), JacksonException.
- IndexingServiceIntegrationTest: add the assertThrows static import
  that sb4's explicit-import expansion left out for apache#205's new XML test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@epugh epugh epugh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@adityamparikh @epugh