[ 
https://issues.apache.org/jira/browse/SOLR-17348?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17860019#comment-17860019
 ] 

Michael Gibney edited comment on SOLR-17348 at 6/26/24 6:28 PM:
----------------------------------------------------------------

In order to naively check for possible deadlocks if the parallelism of 
zkCallback executor were modified, I changed it to 
{{ExecutorUtil.newMDCAwareSingleThreadExecutor()}}. Most of the tests were fine 
(I did not run nightlies); there were only two tests that failed, one of which 
({{ZkSolrClientTest.testMultipleWatchesAsync}}) is not really worth mentioning 
because iiuc it explicitly tests for parallel callback execution, so it should 
be expected to fail.

The only other failure (which is more concerning) seems to involve leader 
election (with recovery?), and can be reproduced via:
{code:sh}
./gradlew :solr:core:test --tests 
"org.apache.solr.cloud.TestCloudConsistency.testOutOfSyncReplicasCannotBecomeLeader"
 -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:+UseParallelGC 
-XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m" 
-Ptests.seed=56FF65ADA5A59077 -Ptests.file.encoding=ISO-8859-1
{code}

I'm not sure what the deadlock here is, but I'd like to know whether it means:
# full parallelism of zk callbacks is really required, and this just won't work
# with a few changes, zk callbacks would _not_ require parallelism

Note also that I wouldn't assume that a passing test suite would mean 
everything is fine. I don't plan to pursue this further atm, but I wanted to 
drop my thoughts and experiments into this issue in case someone feels inclined 
to follow up on it. It's possible that a deep dive here might even improve 
tangential things (e.g., perhaps leader elections?) in unanticipated ways ...


was (Author: mgibney):
In order to naively check for possible deadlocks if the parallelism of 
zkCallback executor were modified, I changed it to 
{{ExecutorUtil.newMDCAwareSingleThreadExecutor()}}. Most of the tests were fine 
(I did not run nightlies); there were only two tests that failed, one of which 
({{ZkSolrClientTest.testMultipleWatchesAsync}}) is not really worth mentioning 
because iiuc it explicitly tests for parallel callback execution, so it should 
be expected to fail.

The only other failure (which is more concerning) can be reproduced via:
{code:sh}
./gradlew :solr:core:test --tests 
"org.apache.solr.cloud.TestCloudConsistency.testOutOfSyncReplicasCannotBecomeLeader"
 -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:+UseParallelGC 
-XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m" 
-Ptests.seed=56FF65ADA5A59077 -Ptests.file.encoding=ISO-8859-1
{code}

I'm not sure what the deadlock here is, but I'd like to know whether it means:
# full parallelism of zk callbacks is really required, and this just won't work
# with a few changes, zk callbacks would _not_ require parallelism

Note also that I wouldn't assume that a passing test suite would mean 
everything is fine. I don't plan to pursue this further atm, but I wanted to 
drop my thoughts and experiments into this issue in case someone feels inclined 
to follow up on it. It's possible that a deep dive here might even improve 
tangential things (e.g., perhaps leader elections?) in unanticipated ways ...

> Mitigate extreme parallelism of zkCallback executor
> ---------------------------------------------------
>
>                 Key: SOLR-17348
>                 URL: https://issues.apache.org/jira/browse/SOLR-17348
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Michael Gibney
>            Priority: Minor
>
> zkCallback executor is [currently an unbounded thread pool of core size 
> 0|https://github.com/apache/solr/blob/709a1ee27df23b419d09fe8f67c3276409131a4a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java#L91-L92],
>  using a SynchronousQueue. Thus, a flood of zkCallback events (as might be 
> triggered by a cluster restart, e.g.) can result in spinning up a very large 
> number of threads. In practice we have encountered as many as 35k threads 
> created in some such cases, even after the impact of this situation was 
> reduced by the fix for SOLR-11535.
> Inspired by [~cpoerschke]'s recent [closer look at thread pool 
> behavior|https://issues.apache.org/jira/browse/SOLR-13350?focusedCommentId=17853178#comment-17853178],
>  I wondered if we might be able to employ a bounded queue to alleviate some 
> of the pressure from bursty zk callbacks.
> The new config might look something like: {{corePoolSize=1024, 
> maximumPoolSize=Integer.MAX_VALUE, allowCoreThreadTimeout=true, workQueue=new 
> LinkedBlockingQueue<>(1024)}}. This would allow the pool to grow up to (and 
> shrink from) corePoolSize in the same manner it currently does, but once 
> exceeding corePoolSize (e.g. during a cluster restart or other callback flood 
> event), tasks would be queued (up to some fixed limit). If the queue limit is 
> exceeded, new threads would still be created, but we would have avoided the 
> current “always create a thread” behavior, and by so doing hopefully reduce 
> task execution time and improve overall throughput.
> From the ThreadPoolExecutor javadocs:
> {quote}Direct handoffs. A good default choice for a work queue is a 
> SynchronousQueue that hands off tasks to threads without otherwise holding 
> them. Here, an attempt to queue a task will fail if no threads are 
> immediately available to run it, so a new thread will be constructed. This 
> policy avoids lockups when handling sets of requests that might have internal 
> dependencies. Direct handoffs generally require unbounded maximumPoolSizes to 
> avoid rejection of new submitted tasks. This in turn admits the possibility 
> of unbounded thread growth when commands continue to arrive on average faster 
> than they can be processed.{quote}
> So afaict SynchronousQueue mainly makes sense if there exists the possibility 
> of deadlock due to dependencies among tasks, and I think this should ideally 
> _not_ be the case with zk callbacks (though in practice I'm not sure this is 
> the case).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to