Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving get snapshot requests to listener based async calls #8377

Merged

Conversation

indrajohn7
Copy link
Contributor

@indrajohn7 indrajohn7 commented Jun 30, 2023

Description

This PR is to discuss the optimisation changes for listener based get_snapshot calls handled through async based model.

The TransportGetSnapshotAction shouldn't block wait on the
repositoriesService.getRepositoryData and move to async processing

Due to this, pending tasks were stuck for hours on master.

Reproduction:

  • 700 shards.
  • Concurrent create_snapshot() calls and PUT mapping requests.
  • 140 concurrent get_snapshot calls
  • It keeps all the generic threadpool busy and the pending_tasks queue piles up.
"opensearch[c007f9bc9cbee0de09eb93767897e305][generic][T#24]" #131865 daemon prio=5 os_prio=0 cpu=1319.41ms elapsed=20.31s tid=0x00007effe406a2b0 nid=0x1d2f waiting on condition  [0x00007effa9b92000]
   java.lang.Thread.State: WAITING (parking)
        at jdk.internal.misc.Unsafe.park(java.base@11.0.18/Native Method)
        - parking to wait for  <0x00000006ac9752d8> (a org.opensearch.common.util.concurrent.BaseFuture$Sync)
        at java.util.concurrent.locks.LockSupport.park(java.base@11.0.18/LockSupport.java:194)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(java.base@11.0.18/AbstractQueuedSynchronizer.java:885)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(java.base@11.0.18/AbstractQueuedSynchronizer.java:1039)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(java.base@11.0.18/AbstractQueuedSynchronizer.java:1345)
        at org.opensearch.common.util.concurrent.BaseFuture$Sync.get(BaseFuture.java:272)
        at org.opensearch.common.util.concurrent.BaseFuture.get(BaseFuture.java:104)
        at org.opensearch.common.util.concurrent.FutureUtils.get(FutureUtils.java:74)
        at org.opensearch.action.support.AdapterActionFuture.actionGet(AdapterActionFuture.java:55)
        at org.opensearch.action.support.PlainActionFuture.get(PlainActionFuture.java:51)
        at org.opensearch.action.admin.cluster.snapshots.get.TransportGetSnapshotsAction.clusterManagerOperation(TransportGetSnapshotsAction.java:143)
        at org.opensearch.action.admin.cluster.snapshots.get.TransportGetSnapshotsAction.clusterManagerOperation(TransportGetSnapshotsAction.java:82)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction.masterOperation(TransportClusterManagerNodeAction.java:144)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction.clusterManagerOperation(TransportClusterManagerNodeAction.java:153)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction$AsyncSingleAction.lambda$doStart$3(TransportClusterManagerNodeAction.java:269)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction$AsyncSingleAction$$Lambda$5035/0x00000008015ff440.accept(Unknown Source)
        at org.opensearch.action.ActionRunnable$2.doRun(ActionRunnable.java:88)
        at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:815)
        at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:52)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@11.0.18/ThreadPoolExecutor.java:1128)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@11.0.18/ThreadPoolExecutor.java:628)
        at java.lang.Thread.run(java.base@11.0.18/Thread.java:829)
        
  • Too many pending tasks queue:
5252  1.1d NORMAL update snapshot after shards started [false] or node configuration changed [true]
10377   12h NORMAL cluster_reroute(reroute after starting shards)
13056  7.2h NORMAL cluster_reroute(reroute after starting shards)
11170 10.3h NORMAL cluster_reroute(reroute after starting shards)
15289 11.7m HIGH   shard-failed
15283 11.7m HIGH   shard-failed
15269   28m HIGH   shard-failed
9039 14.3h NORMAL cluster_reroute(reroute after starting shards)

% curl localhost:9200/_cat/pending_tasks | wc -l
 % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                Dload  Upload   Total   Spent    Left  Speed
100  411k  100  411k    0     0  1674k      0 --:--:-- --:--:-- --:--:-- 1680k
**10022**
  • Pending tasks queue piles up to > 10K.

With Fix:

  • With the similar cluster configurations:
  • The pending task queue never piles up for more than 10 - 20 count with similar number of concurrent create/ get snapshot calls and PUT mapping requests.
  • All the generic threadpools are also not blocked with the get_snapshot() calls.
% grep -irn "getRepositoryData" ./output.jstk | wc -l
0
...

% grep -irn "getRepositoryData" ./output.jstk| wc -l 
2
...

% grep -irn "getRepositoryData" ./output.jstk| wc -l 
6

Use of SNAPSHOT Threadpool

  • Use SNAPSHOT threadpool in stead of GENERIC threadpool for the get_snapshot calls.
  • The create snapshot calls use the SNAPSHOTS threadpool, the same can be expedited here as well unblocking the generic threadpool.

Observations:

  • This has a ripple effect on Latency. With too many get_snapshot concurrent calls, it was observed that there is always a single entry in the thread dump for the SNAPSHOT threadpool, processing processing the get_snapshot request.
    • For 10 concurrent GET calls, where the generic threadpool response time is ~1 secs, with listener based async model the SNAPSHOT threadpool is being latent, where the latency is ~100 - ~110 secs, which is a real impact to the performance metrics.
      *get_snapshot response:
        % tail -f nohup.out         
          % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                         Dload  Upload   Total   Spent    Left  Speed
          0     0    0     0    0     0      0      0 --:--:--  0:01:49 --:--:--     02023-06-13t08-50-54.5a5cb21d-8563-46ca-9779-4ee58443fa91     SUCCESS 1686646257 08:50:57 1686646259 08:50:59  1.2s   2    2 0    2
        2023-06-13t09-21-44.d9879b36-162b-7c62-f194-240b355be809     SUCCESS 1686648104 09:21:44 1686648104 09:21:44 200ms   2    2 0    2
        2023-06-13t10-21-44.bd67e503-9d29-d63e-0efd-c4f3763891fe     SUCCESS 1686651704 10:21:44 1686651711 10:21:51  6.8s  34  322 0  322
        2023-06-13t11-21-44.16b7bd58-7ca0-de6a-6537-cdd99236cc17
  • This is because the number of SNAPSHOT threadpools are lesser compared to the generic one.
final int genericThreadPoolMax = boundedBy(4 * allocatedProcessors, 128, 512);
builders.put(Names.GENERIC, new ScalingExecutorBuilder(Names.GENERIC, 4, genericThreadPoolMax, TimeValue.timeValueSeconds(30)));
builders.put(Names.SNAPSHOT, new ScalingExecutorBuilder(Names.SNAPSHOT, 1, halfProcMaxAt5, TimeValue.timeValueMinutes(5)));

  • Hence use of SNAPSHOT threadpool would not be recommended here as it may block the create snapshot calls as well if its shared across get_snapshot requests.

Related Issues

Resolves #1788

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: INDRAJIT BANERJEE <indrajohn7@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Merging #8377 (0d7e7fb) into main (bb78930) will decrease coverage by 0.08%.
Report is 4 commits behind head on main.
The diff coverage is 74.54%.

@@             Coverage Diff              @@
##               main    #8377      +/-   ##
============================================
- Coverage     71.06%   70.99%   -0.08%     
+ Complexity    57314    57288      -26     
============================================
  Files          4766     4765       -1     
  Lines        270453   270485      +32     
  Branches      39555    39555              
============================================
- Hits         192197   192028     -169     
- Misses        62045    62309     +264     
+ Partials      16211    16148      -63     
Files Changed Coverage Δ
.../opensearch/client/indices/CreateIndexRequest.java 76.66% <0.00%> (ø)
...opensearch/client/slm/SnapshotLifecyclePolicy.java 0.00% <ø> (ø)
...ch/client/slm/SnapshotLifecyclePolicyMetadata.java 0.00% <ø> (ø)
.../opensearch/client/slm/SnapshotLifecycleStats.java 0.00% <ø> (ø)
...rch/client/slm/SnapshotRetentionConfiguration.java 0.00% <ø> (ø)
...h/script/mustache/MultiSearchTemplateResponse.java 59.01% <ø> (ø)
...rg/opensearch/index/rankeval/RankEvalResponse.java 91.42% <ø> (+1.42%) ⬆️
...va/org/opensearch/index/rankeval/RankEvalSpec.java 97.36% <ø> (ø)
...a/org/opensearch/index/rankeval/RatedDocument.java 81.03% <0.00%> (ø)
...va/org/opensearch/index/rankeval/RatedRequest.java 91.79% <ø> (ø)
... and 135 more

... and 438 files with indirect coverage changes

@shwetathareja
Copy link
Member

Porting the comment from old PR here

For 10 concurrent GET calls, where the generic threadpool response time is ~1 secs, with listener based async model the SNAPSHOT threadpool is being latent, where the latency is ~100 - ~110 secs, which is a real impact to the performance metrics.

@indrajohn7 This is because SNAPSHOT threadpool can scale up to only 5 threads

builders.put(Names.SNAPSHOT, new ScalingExecutorBuilder(Names.SNAPSHOT, 1, halfProcMaxAt5, TimeValue.timeValueMinutes(5)));

where as GENERIC threadpool can scale upto 512 depending on no. of cores.

final int genericThreadPoolMax = boundedBy(4 * allocatedProcessors, 128, 512);
builders.put(Names.GENERIC, new ScalingExecutorBuilder(Names.GENERIC, 4, genericThreadPoolMax, TimeValue.timeValueSeconds(30)));

Signed-off-by: Indrajit Banerjee <banerind@amazon.com>
Signed-off-by: Indrajit Banerjee <banerind@amazon.com>
@indrajohn7 indrajohn7 requested a review from sohami as a code owner August 4, 2023 13:11
@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/performance-analyzer.git]
Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git]

BUILD SUCCESSFUL in 27m 11s

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Indrajit Banerjee <banerind@amazon.com>
@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:


> Task :checkCompatibility
Checking compatibility for: https://github.com/opensearch-project/reporting.git with ref: main
Incompatible components: [https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/asynchronous-search.git]
Compatible components: [https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git]

BUILD SUCCESSFUL in 24m 9s

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2023

Gradle Check (Jenkins) Run Completed with:

@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/security-analytics.git]
Compatible components: [https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

BUILD SUCCESSFUL in 26m 43s

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

Gradle Check (Jenkins) Run Completed with:

@shwetathareja shwetathareja merged commit 2de1c24 into opensearch-project:main Aug 8, 2023
10 checks passed
@shwetathareja shwetathareja added the backport 2.x Backport to 2.x branch label Aug 8, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-8377-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2de1c24e8947e15d6faebd448517e9d524a1b788
# Push it to GitHub
git push --set-upstream origin backport/backport-8377-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-8377-to-2.x.

@indrajohn7 indrajohn7 mentioned this pull request Aug 8, 2023
6 tasks
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
…h-project#8377)

* Moving get snapshot requests to listener based async calls
---------

Signed-off-by: INDRAJIT BANERJEE <indrajohn7@gmail.com>
Signed-off-by: Indrajit Banerjee <banerind@amazon.com>
Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…h-project#8377)

* Moving get snapshot requests to listener based async calls
---------

Signed-off-by: INDRAJIT BANERJEE <indrajohn7@gmail.com>
Signed-off-by: Indrajit Banerjee <banerind@amazon.com>
Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…h-project#8377)

* Moving get snapshot requests to listener based async calls
---------

Signed-off-by: INDRAJIT BANERJEE <indrajohn7@gmail.com>
Signed-off-by: Indrajit Banerjee <banerind@amazon.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Too many get snapshot calls causing generic threadpool to be busy completely
4 participants