nick-boss-tech opened a new pull request, #5075: URL: https://github.com/apache/solr/pull/5075
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-5939 ## What happens today **When a merged stream fails, the error is recorded only against the first request sent to that node.** A streaming client merges queued updates into one request stream. The client is built with the first request sent to its node, and a failed stream is recorded against that request. The distributor then judges retries and failure counts on the wrong request. For example, a delete-by-query that fails in a stream with a retriable add leaves the add with no error record, and the add is not resubmitted. ## What this change does **Each request in a failed stream gets its own error, and a successful stream forgets its requests.** - The streaming clients report each update they write into a stream. The Jetty and JDK clients both do this ([ConcurrentUpdateJettySolrClient.java](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/ConcurrentUpdateJettySolrClient.java#L92), [ConcurrentUpdateJdkSolrClient.java](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateJdkSolrClient.java#L40)), through `recordStreamedUpdate` ([ConcurrentUpdateBaseSolrClient.java](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateBaseSolrClient.java#L268-L273)). - A failed stream calls a new `handleError` callback with its requests. The Solr client records one error per request, and the errors share the exception ([StreamingSolrClients.java](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java#L155-L169)). - A successful stream removes its requests from the lookup map ([StreamingSolrClients.java](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java#L193-L201)). Before, the map kept them until the update request ended. - In the tolerant update path, each distinct remote exception is counted once, even when several errors share it ([TolerantUpdateProcessor.java](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/core/src/java/org/apache/solr/update/processor/TolerantUpdateProcessor.java#L238-L246)). - Three SolrJ additions, and no existing SolrJ signature changes: `protected final recordStreamedUpdate(Update)` ([L268-L273](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateBaseSolrClient.java#L268-L273)), `protected handleError(Throwable, List<UpdateRequest>, List<String>, String)` ([L675-L678](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateBaseSolrClient.java#L675-L678)), and `public onSuccess(Object, InputStream, List<UpdateRequest>)` ([L724-L727](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateBaseSolrClient.java#L724-L727)). The last two delegate to the older methods by default. ## A choice to check **Per-request attribution is a design choice. Maintainers should judge it.** The server returns one response for the whole stream, so the client cannot tell which update it rejected. This branch records the failure once per request in the stream, sharing the exception. Two other routes were considered: - Pass per-document identity through the callback, so an error names the document the server rejected. This needs server-side changes, or one request per send, which gives up stream merging. - Fail the whole batch, with one error and no per-request retry decision. Simpler, but coarser. Was per-request attribution the right middle call? ## Proof **The focused tests pass with SOLR-5754 combined in, and the new error-attribution tests fail on base production.** - The combined run (verified 2026-10-08) passed 26 of 26 focused tests on the merge commit `1ddbf36202d`, whose tree is `f2e33340f0ea`, of this head with the SOLR-5754 branch. This head `f8d4bdbea518` is the live tip and the gated head. The 26 tests are: - StreamingSolrClientsErrorAttributionTest 2 of 2 - SolrCmdDistributorTest 1 of 1 - TestTolerantUpdateProcessorCloud 19 of 19 - TestTolerantUpdateProcessorRandomCloud 2 of 2 - StreamingSolrClientsTest 2 of 2. This class comes from SOLR-5754 and is not in this branch alone. - [StreamingSolrClientsErrorAttributionTest](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/core/src/test/org/apache/solr/update/StreamingSolrClientsErrorAttributionTest.java#L104-L152) has two tests. `testMergedStreamErrorIsAttributedToEveryRequestInTheStream` expects error document ids `1` and `2`. `testDeleteByQueryRetryRuleIsJudgedOnItsOwnRequest` expects two errors. On base production, this class fails 2 of 2. ## Limits **Several effects of this change are stated here, not narrowed.** - Client-visible text: a failed merged stream now has one entry per request in it. `DistributedUpdatesAsyncException` lists each entry's message, so the same message can repeat ([DistributedUpdateProcessor.java](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java#L1267-L1282)). - Failure accounting: when a stream fails and the retry rule says no, each request in the stream is recorded as failed, not only the first ([StreamingSolrClients.java](https://github.com/nick-boss-tech/solr/blob/f8d4bdbea518901e255ae119f3e5c43e7804a9bf/solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java#L180-L183)). Requests that were fine can be recorded as failed and can appear in the client's error list. - Retries: within a failed stream, each request gets its own retry decision. A request that would have succeeded on its own can be resent. - Success credit is unchanged. A successful merged stream still credits only the request the client was built with. - A stream implementation that does not report its members names only the first request in that stream. - The tolerant-path change has no new test in this branch. - The registry release is checked by reading. No test measures memory use. - The proof run used the merged tree with SOLR-5754, not this branch alone. Changelog: `changelog/unreleased/SOLR-5939.yml` (type: fixed). ### AI assistance AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution. -- 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]
