pvcnt commented on code in PR #2313:
URL: https://github.com/apache/solr/pull/2313#discussion_r1571172153
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -1017,19 +1023,21 @@ public String getBaseURL() {
}
private static class AsyncTracker {
- private static final int MAX_OUTSTANDING_REQUESTS = 1000;
+ private static final int DEFAULT_MAX_OUTSTANDING_REQUESTS = 1000;
// wait for async requests
private final Phaser phaser;
+ private final int maxOutstandingRequests;
// maximum outstanding requests left
private final Semaphore available;
private final Request.QueuedListener queuedListener;
private final Response.CompleteListener completeListener;
- AsyncTracker() {
+ AsyncTracker(int maxOutstandingRequests) {
// TODO: what about shared instances?
+ this.maxOutstandingRequests = maxOutstandingRequests;
phaser = new Phaser(1);
- available = new Semaphore(MAX_OUTSTANDING_REQUESTS, false);
+ available = new Semaphore(maxOutstandingRequests, false);
Review Comment:
I have already observed in production exceptions thrown because the internal
queue of the HTTP2 client was full. It should not have been the case if the
semaphore was working correctly, should it? When I was debugging the code, I
found out that the `queuedListener` is called after the request had a change to
get rejected, which means that the semaphore does not have a chance to
protected this from happening. Or am I missing the point of this semaphore?
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
