pvcnt commented on code in PR #2313:
URL: https://github.com/apache/solr/pull/2313#discussion_r1535540950
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -1278,6 +1287,15 @@ public Builder withMaxConnectionsPerHost(int max) {
return this;
}
+ /**
+ * Specify the maximum number of concurrent asynchronous requests the
client can send. The
+ * default value is 1000
+ */
+ public Builder withMaxOutstandingRequests(int max) {
Review Comment:
If I'm not mistaken it's actually a maximum amount per host. Could we make
it explicit, e.g., `maxOutstandingRequestsPerHost`?
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -1065,6 +1073,7 @@ public static class Builder {
private Long connectionTimeoutMillis;
private Long requestTimeoutMillis;
private Integer maxConnectionsPerHost;
+ private Integer maxOutstandingAsyncRequests;
Review Comment:
`maxOutstandingAsyncRequests` is inconsistent with
`withMaxOutstandingRequests` (about the async part). I think we can drop the
async part, since even synchronous requests use the threshold returned by
`getMaxRequestsQueuedPerDestination`.
##########
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:
Not related to your review, but this semaphore is not working at all.
`queuedListener` is called after the request may have been rejected, which
essentially makes it useless.
--
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
