dsmiley commented on a change in pull request #1770:
URL: https://github.com/apache/lucene-solr/pull/1770#discussion_r495230006
##########
File path:
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
##########
@@ -156,36 +155,31 @@ public void submit(final ShardRequest sreq, final String
shard, final Modifiable
return;
}
- // all variables that set inside this listener must be at least volatile
- responseCancellableMap.put(srsp, this.lbClient.asyncReq(lbReq, new
AsyncListener<>() {
- volatile long startTime = System.nanoTime();
-
- @Override
- public void onStart() {
- if (tracer != null && span != null) {
- tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new
SolrRequestCarrier(req));
- }
- SolrRequestInfo requestInfo = SolrRequestInfo.getRequestInfo();
- if (requestInfo != null)
req.setUserPrincipal(requestInfo.getReq().getUserPrincipal());
- }
+ long startTime = System.nanoTime();
+ if (tracer != null && span != null) {
+ tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new
SolrRequestCarrier(req));
+ }
+ SolrRequestInfo requestInfo = SolrRequestInfo.getRequestInfo();
+ if (requestInfo != null)
req.setUserPrincipal(requestInfo.getReq().getUserPrincipal());
- @Override
- public void onSuccess(LBSolrClient.Rsp rsp) {
+ CompletableFuture<LBSolrClient.Rsp> future =
this.lbClient.requestAsync(lbReq);
+ future.whenComplete((rsp, throwable) -> {
+ if (!future.isCompletedExceptionally()) {
ssr.nl = rsp.getResponse();
srsp.setShardAddress(rsp.getServer());
ssr.elapsedTime = TimeUnit.MILLISECONDS.convert(System.nanoTime() -
startTime, TimeUnit.NANOSECONDS);
responses.add(srsp);
- }
-
- public void onFailure(Throwable throwable) {
+ } else if (!future.isCancelled()) {
ssr.elapsedTime = TimeUnit.MILLISECONDS.convert(System.nanoTime() -
startTime, TimeUnit.NANOSECONDS);
srsp.setException(throwable);
if (throwable instanceof SolrException) {
srsp.setResponseCode(((SolrException) throwable).code());
}
responses.add(srsp);
Review comment:
I was going to say: It should now be straight-forward to call
responses.add(srsp) and ssr.elapsedTime = ... in one spot instead of duplicated
for both successful and unsuccessful. BUT... the other scenario is that the
future was cancelled, which won't satisfy either the success/failure
conditions. So we do nothing. Hmmm. I guess it's okay; not sure if you
thought of this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
