gerlowskija commented on code in PR #2461:
URL: https://github.com/apache/solr/pull/2461#discussion_r1613504683
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java:
##########
@@ -566,24 +576,28 @@ public NamedList<Object> request(final SolrRequest<?>
request, String collection
if (!success) {
// stall prevention
int currentQueueSize = queue.size();
+ long currentTime = System.nanoTime();
+ long processed = processedCount.sum();
Review Comment:
[Q] Should we still be comparing queue-sizes in L581 below? As you've
highlighted in the JIRA ticket and this PR description - queue size isn't a
pretty ambivalent indicator of whether the queue processing is stalled or not.
We might see queue size change even when processing is stalled (as new queue
elements are added). And we might see queue size stay the same even when
processing is going just fine (as heavy indexing will keep the bounded queue
almost continually at its max).
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java:
##########
@@ -566,24 +576,28 @@ public NamedList<Object> request(final SolrRequest<?>
request, String collection
if (!success) {
// stall prevention
int currentQueueSize = queue.size();
+ long currentTime = System.nanoTime();
+ long processed = processedCount.sum();
if (currentQueueSize != lastQueueSize) {
// there's still some progress in processing the queue - not
stalled
lastQueueSize = currentQueueSize;
+ lastProcessedCount = processed;
+ lastCheckTime = currentTime;
lastStallTime = -1;
} else {
- if (lastStallTime == -1) {
- // mark a stall but keep trying
- lastStallTime = System.nanoTime();
- } else {
- long currentStallTime =
- TimeUnit.NANOSECONDS.toMillis(System.nanoTime() -
lastStallTime);
- if (currentStallTime > stallTimeMillis) {
+ long timeElapsed = TimeUnit.NANOSECONDS.toMillis(currentTime -
lastCheckTime);
+ if (timeElapsed > stallTimeMillis) {
Review Comment:
+1 to what AB said - ideally we'd find some way to remove a lot of this code
duplication, but at a minimum we'll need to update the other copies of the
logic to use the new "processedCount" instead of queue size.
--
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]
