adityamparikh opened a new pull request, #233:
URL: https://github.com/apache/solr-mcp/pull/233
## What
Two changes to `indexDocuments`, which backs `index-json-documents` and
`index-markdown-documents`, plus one CPU fix in `JsonDocumentCreator`.
### 1. Per-document retry only when Solr rejects the documents (400)
Today any failed batch add is retried one document at a time:
`SolrServerException`, `IOException` or any `RuntimeException`. A missing
collection (404), an auth failure (401/403), a 5xx or an unreachable Solr fails
every retry the same way:
- **Round trips:** up to N + 2 per 1,000-document batch (the batch add, N
single adds, then the commit).
- **Outage cost:** when Solr is down, each single add waits out its own
connect timeout.
- **Wrong result:** if the commit then succeeds, the tool reports
`Successfully indexed 0 of N` and the real error is lost.
The fallback now runs only for `SolrException` with `code() ==
ErrorCode.BAD_REQUEST.code`, which covers unknown-field and type-mismatch
errors: a bad document, where going one at a time can save the valid ones.
Every other failure propagates unchanged, so MCP clients see the real cause.
**Behaviour change:** a transport or server error during a batch is now
reported as an error, not swallowed as a partial success.
### 2. Commit on the last batch's request
JSON and Markdown sent their adds and then a separate `commit`. CSV and XML
already attach the commit to the update request itself (`forward()`). The last
batch now uses the same approach: one `UpdateRequest` carrying the documents
plus `setAction(COMMIT, false, true, true)`. The commit semantics are
unchanged: soft commit with `waitSearcher=true`, so documents are searchable
when the call returns.
| Call | Before | After |
|---|---|---|
| Markdown (always 1 doc) | 2 | 1 |
| JSON, ≤ 1,000 docs | 2 | 1 |
| JSON, 2,500 docs | 4 | 3 |
| Batch hits a 404 / 5xx / outage | N + 2 | 1, with the error surfaced |
If the last batch falls back to per-document retry, Solr never ran its
commit, so an explicit commit follows on that path only. An empty list now
makes no request; the tools can't send one anyway.
### 3. Sanitize each field name once per call
`addAllFieldsFlat` ran `FieldNameSanitizer` (lower-casing plus three regex
passes) for every field of every document. A `HashMap` memo that lives for one
`flatten()` call now sanitizes each distinct raw name once. It is not a static
cache, because the keys are user input. Output is identical.
## Tests
- `IndexingServiceTest`:
- Fallback tests now use a 400 `RemoteSolrException`.
- A `RuntimeException` or `SolrServerException` now propagates with no
single-document adds. The old `…ShouldPropagateException` test actually
asserted the error was swallowed.
- New test: a 404 propagates after exactly one request.
- Commit assertions now capture the `UpdateRequest` and check `commit`,
`softCommit` and `waitSearcher`, as the CSV test does.
- `IndexingServiceIntegrationTest`, `MarkdownIndexingTest` and
`JsonDocumentCreatorTest` are unchanged and pass.
- `./gradlew build`: 422 tests, 0 failures, 0 skipped.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
