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

Close RemoteStorePinnedTimestampService on Node.close() #16228

Conversation

sachinpkale
Copy link
Member

Description

  • RemoteStorePinnedTimestampService internally manages an async scheduler.
  • If not closed properly, integ tests sometimes fail with following exception:
com.carrotsearch.randomizedtesting.UncaughtExceptionError: Captured an uncaught exception in thread: Thread[id=6639, name=opensearch[node_t1][scheduler][T#1], state=RUNNABLE, group=TGRP-RemoteStorePinnedTimestampsGarbageCollectionIT]
	at __randomizedtesting.SeedInfo.seed([401E5CF6CD0D4727:9C0BCAD1CF20D2F6]:0)
Caused by: org.opensearch.core.concurrency.OpenSearchRejectedExecutionException: rejected execution of java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask@143262f2[Not completed, task = java.util.concurrent.Executors$RunnableAdapter@2866748[Wrapped task = org.opensearch.node.remotestore.RemoteStorePinnedTimestampService$AsyncUpdatePinnedTimestampTask@7794b6fc]] on org.opensearch.threadpool.Scheduler$SafeScheduledThreadPoolExecutor@16dc01db[Shutting down, pool size = 1, active threads = 1, queued tasks = 0, completed tasks = 124]
	at __randomizedtesting.SeedInfo.seed([401E5CF6CD0D4727]:0)
  • Even though this does not have direct impact on live clusters as Node.close() closes threadpool, integ tests became flaky due to this.
  • In this PR, we explicitly close RemoteStorePinnedTimestampService on Node.close()

Related Issues

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

Copy link
Contributor

github-actions bot commented Oct 8, 2024

✅ Gradle check result for 4940a0e: SUCCESS

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.95%. Comparing base (a81b868) to head (4940a0e).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #16228   +/-   ##
=========================================
  Coverage     71.94%   71.95%           
- Complexity    64612    64633   +21     
=========================================
  Files          5298     5299    +1     
  Lines        301952   301965   +13     
  Branches      43627    43630    +3     
=========================================
+ Hits         217247   217267   +20     
+ Misses        66884    66828   -56     
- Partials      17821    17870   +49     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gbbafna gbbafna merged commit 96082f7 into opensearch-project:main Oct 8, 2024
58 of 59 checks passed
@gbbafna gbbafna added the backport 2.x Backport to 2.x branch label Oct 8, 2024
@gbbafna gbbafna deleted the RemoteStorePinnedTimestampsGarbageCollectionIT-flaky-fix branch October 8, 2024 09:46
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 8, 2024
Signed-off-by: Sachin Kale <[email protected]>
(cherry picked from commit 96082f7)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
gbbafna pushed a commit that referenced this pull request Oct 8, 2024
)

(cherry picked from commit 96082f7)

Signed-off-by: Sachin Kale <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.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 skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants