-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Fix Race in Closing IndicesService.CacheCleaner #42016
Fix Race in Closing IndicesService.CacheCleaner #42016
Conversation
* When close becomes true while the management pool is shut down, we run into an unhandled `EsRejectedExecutionException` that fails tests * Found this while trying to reproduce elastic#32506 * Running the IndexStatsIT in a loop is a way of reproducing this
Pinging @elastic/es-distributed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @original-brownbear.
thanks @dnhatn ! |
* When close becomes true while the management pool is shut down, we run into an unhandled `EsRejectedExecutionException` that fails tests * Found this while trying to reproduce elastic#32506 * Running the IndexStatsIT in a loop is a way of reproducing this
* When close becomes true while the management pool is shut down, we run into an unhandled `EsRejectedExecutionException` that fails tests * Found this while trying to reproduce #32506 * Running the IndexStatsIT in a loop is a way of reproducing this
@@ -1219,7 +1220,13 @@ public void run() { | |||
} | |||
// Reschedule itself to run again if not closed | |||
if (closed.get() == false) { | |||
threadPool.schedule(this, interval, ThreadPool.Names.SAME); | |||
try { | |||
threadPool.schedule(this, interval, ThreadPool.Names.SAME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
easier to use scheduleUnlessShuttingDown
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry! Didn't know about that method, will open cleanup PR in a sec.
* Follow up to elastic#42016
* When close becomes true while the management pool is shut down, we run into an unhandled `EsRejectedExecutionException` that fails tests * Found this while trying to reproduce elastic#32506 * Running the IndexStatsIT in a loop is a way of reproducing this
* Follow up to #42016
Runnable
here is being shut down, we run into an unhandeledEsRejectedExecutionException
that fails testsIndexStatsIT
in a loop is a way of reproducing this