rpuch commented on code in PR #6199:
URL: https://github.com/apache/ignite-3/pull/6199#discussion_r2191698962
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/WatchProcessor.java:
##########
@@ -224,36 +227,34 @@ public CompletableFuture<Void> notifyWatches(long
newRevision, List<Entry> updat
* @return Updated value of {@link #notificationFuture}.
*/
@VisibleForTesting
- CompletableFuture<Void> enqueue(Supplier<CompletableFuture<Void>>
asyncAction, Supplier<String> additionalInfoSupplier) {
- while (true) {
- CompletableFuture<Void> chainingFuture = new CompletableFuture<>();
-
- CompletableFuture<Void> newNotificationFuture = chainingFuture
+ CompletableFuture<Void> enqueue(
+ Supplier<CompletableFuture<Void>> asyncAction,
+ Consumer<CompletableFuture<Void>> afterEnqueuing,
+ Supplier<String> additionalInfoSupplier
+ ) {
+ synchronized (notificationFutureMutex) {
Review Comment:
This method is called from 2 places:
1. On start, from 'join-0' thread (this is the thread that starts most
components)
2. During normal operation, from MS state machine (always from the same
thread)
Also, `notificationFuture` is read on stop from the thread that does the
stop; but the stop happens once and it not important in this context (the only
issue is to ensure visibility, which `synchronized` solves).
1 and 2 do not overlap at all, so there never is any contention. After the
node started, this method is always called from the same thread, so it's not
important for performance whether the algorithm is blocking or not. But code
with `synchronized` is simpler; also, it makes it easier to ensure atomicity of
several updates.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
