Introduce necessary executors and implement sleepAsync - #3
Conversation
634c8dd to
85b1c3d
Compare
JAVA-6240
JAVA-6240
| * All {@link Throwable}s are logged.</li> | ||
| * </ul> | ||
| */ | ||
| public final class MongoThreadPoolExecutor extends ThreadPoolExecutor { |
There was a problem hiding this comment.
VAKOTODO Update the description of https://jira.mongodb.org/browse/JAVA-6109: mention that all other executor implementations / single threads should be replaces either with virtual threads (https://jira.mongodb.org/browse/JAVA-4930 - distant future), or MongoThreadPoolExecutor/MongoScheduledThreadPoolExecutor (this includes the executors created in AsynchronousTlsChannelGroup, NettyStreamFactoryFactory (NioEventLoopGroup)), unless it's an IO executor supplied by an application.
Also mention in that ticket to document that the executors supplied by applications should themselves make sure uncaught Throwables are not swallowed, potentially the same way MongoThreadPoolExecutor/MongoScheduledThreadPoolExecutor do it.
There was a problem hiding this comment.
This is about updating Jira, so not blocking merge.
So consider it resolved from standpoint of merging this PR.
|
LGTM! |
| withLock(closeLock, () -> { | ||
| scheduledTasks.add(scheduledTask); | ||
| if (closed) { | ||
| throw createClosedException(); |
There was a problem hiding this comment.
I'm not clear why this both adds scheduledTask to scheduledTasks and also throws here.
There was a problem hiding this comment.
Discussed over Zoom. Leaving a comment for completeness.
We throw an exception here just to catch it below and call scheduledTask.reject:
catch (RejectedExecutionException rejectionCause) {
scheduledTask.reject(rejectionCause);
}ScheduledRejectableRunnable.run/reject are implemented mutually exclusive by means of if (scheduledTasks.remove(this)). Thus, if we don't do scheduledTasks.add(scheduledTask), then scheduledTask.reject does nothing.
An alternative is: introduce another reject that does not do if (scheduledTasks.remove(this)) and is dedicated for tasks that are guaranteed not be have been scheduled. This complicates the code by introducing non-uniformity.
| * @see AsyncClientExecutor | ||
| */ | ||
| // VAKOTODO decide what to do with https://jira.mongodb.org/browse/JAVA-6279. | ||
| public final class CommonExecutor { |
…s `getExecutor` returns the actual I/O executor
| * | ||
| * @see AsyncClientExecutor | ||
| */ | ||
| // TODO-BACKPRESSURE Valentin decide what to do with https://jira.mongodb.org/browse/JAVA-6279. |
There was a problem hiding this comment.
This is a documented (see the description of mongodb#1918) way of leaving TODOs in the backpressure feature branch.
There was a problem hiding this comment.
The refactoring done to this and related classes in 66182bc addresses the VAKOTODO code comment that was left previously in this PR.
3 participants
AI usage
AI was used only to review and to suggest ways to deal with the serious bug it discovered (see below).
AI identified a serious bug with
CommonExecutoroffloading scheduled tasks to anotherExecutor, which I failed to think about on my own. AI also expressed ideas on how one may deal with that problem. One of them I manually implemented inDefaultAsyncClientExecutor.JAVA-6240