[
https://issues.apache.org/jira/browse/SOLR-3284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095301#comment-18095301
]
Serhiy Bzhezytskyy commented on SOLR-3284:
------------------------------------------
Reopening this from a production angle. We use ConcurrentUpdateSolrClient (via
the Builder) to index into Solr, and when a batch fails the only signal is
handleError(Throwable) — the exception, but not which documents didn't make it.
So we can't route the failed docs to a retry queue / DLQ; we just know
something in the batch failed.
[~dsmiley] suggested a Builder-configurable handler here back in 2016. I have
that working locally and it fits our case well:
new ConcurrentUpdateJdkSolrClient.Builder(url, httpClient)
.withErrorHandler((ex, request, collection) -> {
for (SolrInputDocument doc : request.getDocuments()) {
// route the doc that didn't reach the server to retry / DLQ
}
})
.build();
The handler gets the failed UpdateRequest (so, its documents) plus the
collection. The caller reads whatever field is their uniqueKey — the client
doesn't assume one. Default with no handler stays as-is (logs), so it's
backward compatible.
Two things I'd want a committer's call on before I open a PR:
1. Default behavior. I kept it optional + log-by-default for compatibility. The
2016 discussion leaned toward surfacing errors by default (throw if no
handler). Which do you prefer?
2. Error-loop behavior. To pass the request to the handler I had to catch
failures per-update inside the runner loop. A side effect is the runner now
continues to the next batch instead of exiting on error. That seems better (one
bad batch doesn't stall the runner), but it's a behavior change worth flagging.
Side note I hit while testing: RemoteSolrException(host, code, remoteError)
NPEs when remoteError is null (the ctor does Map.of("remoteError",
remoteError), which rejects null values) — happens when the server returns an
error body that doesn't parse. Unrelated to this, probably its own issue.
Is there appetite for this? If so I'll put up a PR scoped to the
Builder-handler approach above.
> ConcurrentUpdateSolrClient swallows exceptions
> ----------------------------------------------
>
> Key: SOLR-3284
> URL: https://issues.apache.org/jira/browse/SOLR-3284
> Project: Solr
> Issue Type: Improvement
> Components: clients - java
> Affects Versions: 3.5, 4.0-ALPHA
> Reporter: Shawn Heisey
> Assignee: Shawn Heisey
> Priority: Major
> Attachments: SOLR-3284.patch
>
>
> StreamingUpdateSolrServer eats exceptions thrown by lower level code, such as
> HttpClient, when doing adds. It may happen with other methods, though I know
> that query and deleteByQuery will throw exceptions. I believe that this is a
> result of the queue/Runner design. That's what makes SUSS perform better,
> but it means you sacrifice the ability to programmatically determine that
> there was a problem with your update. All errors are logged via slf4j, but
> that's not terribly helpful except with determining what went wrong after the
> fact.
> When using CommonsHttpSolrServer, I've been able to rely on getting an
> exception thrown by pretty much any error, letting me use try/catch to detect
> problems.
> There's probably enough dependent code out there that it would not be a good
> idea to change the design of SUSS, unless there were alternate constructors
> or additional methods available to configure new/old behavior. Fixing this
> is probably not trivial, so it's probably a better idea to come up with a new
> server object based on CHSS. This is outside my current skillset.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
