This is an automated email from the ASF dual-hosted git repository.

sanpwc pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git


The following commit(s) were added to refs/heads/main by this push:
     new 16bac8d2e4 IGNITE-23661 Eliminate race in onBeforeApply within 
StandaloneMetaStorageManager (#4723)
16bac8d2e4 is described below

commit 16bac8d2e4ef7a544a5ea54310fe5e298327c228
Author: Alexander Lapin <[email protected]>
AuthorDate: Fri Nov 15 09:07:55 2024 +0200

    IGNITE-23661 Eliminate race in onBeforeApply within 
StandaloneMetaStorageManager (#4723)
---
 .../ItIgniteDistributionZoneManagerNodeRestartTest.java  |  4 ----
 .../distributionzones/DistributionZoneManager.java       |  2 ++
 .../rebalance/DistributionZoneRebalanceEngine.java       |  2 +-
 .../internal/metastorage/server/WatchProcessor.java      |  1 +
 .../metastorage/impl/StandaloneMetaStorageManager.java   | 16 +++++++++++-----
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git 
a/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/distributionzones/ItIgniteDistributionZoneManagerNodeRestartTest.java
 
b/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/distributionzones/ItIgniteDistributionZoneManagerNodeRestartTest.java
index 518b054875..de8e43f8b3 100644
--- 
a/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/distributionzones/ItIgniteDistributionZoneManagerNodeRestartTest.java
+++ 
b/modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/distributionzones/ItIgniteDistributionZoneManagerNodeRestartTest.java
@@ -132,7 +132,6 @@ import 
org.apache.ignite.internal.worker.fixtures.NoOpCriticalWorkerRegistry;
 import org.apache.ignite.network.NetworkAddress;
 import org.jetbrains.annotations.Nullable;
 import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -441,7 +440,6 @@ public class ItIgniteDistributionZoneManagerNodeRestartTest 
extends BaseIgniteRe
     }
 
     @Test
-    @Disabled("https://issues.apache.org/jira/browse/IGNITE-23661";)
     public void testLogicalTopologyRestoredAfterRestart() throws Exception {
         PartialNode node = startPartialNode(0);
 
@@ -545,7 +543,6 @@ public class ItIgniteDistributionZoneManagerNodeRestartTest 
extends BaseIgniteRe
     }
 
     @Test
-    @Disabled("https://issues.apache.org/jira/browse/IGNITE-23661";)
     public void 
testFirstLogicalTopologyUpdateInterruptedEventRestoredAfterRestart() throws 
Exception {
         PartialNode node = startPartialNode(0);
 
@@ -712,7 +709,6 @@ public class ItIgniteDistributionZoneManagerNodeRestartTest 
extends BaseIgniteRe
 
     @ParameterizedTest(name = "defaultZone={0}")
     @ValueSource(booleans = {true, false})
-    @Disabled("https://issues.apache.org/jira/browse/IGNITE-23660";)
     public void testScaleUpTimerIsRestoredAfterRestart(boolean defaultZone) 
throws Exception {
         PartialNode node = startPartialNode(0);
 
diff --git 
a/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java
 
b/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java
index ca0a305b1b..764d001cf0 100644
--- 
a/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java
+++ 
b/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java
@@ -310,6 +310,8 @@ public class DistributionZoneManager implements 
IgniteComponent {
 
         busyLock.block();
 
+        zonesState.values().forEach(ZoneState::stopTimers);
+
         rebalanceEngine.stop();
 
         logicalTopologyService.removeEventListener(topologyEventListener);
diff --git 
a/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java
 
b/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java
index 777afd919b..c92f85e2b0 100644
--- 
a/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java
+++ 
b/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java
@@ -92,7 +92,7 @@ public class DistributionZoneRebalanceEngine {
     public static final boolean ENABLED = getBoolean(FEATURE_FLAG_NAME, false);
 
     /** Special flag to skip rebalance on node recovery for tests. */
-    // TODO: IGNITE-23466 Remove it
+    // TODO: IGNITE-23561 Remove it
     @TestOnly
     public static final String SKIP_REBALANCE_TRIGGERS_RECOVERY = 
"IGNITE_SKIP_REBALANCE_TRIGGERS_RECOVERY";
 
diff --git 
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/WatchProcessor.java
 
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/WatchProcessor.java
index eee92a89e9..b6e0975cfc 100644
--- 
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/WatchProcessor.java
+++ 
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/WatchProcessor.java
@@ -177,6 +177,7 @@ public class WatchProcessor implements ManuallyCloseable {
     public CompletableFuture<Void> notifyWatches(List<Entry> updatedEntries, 
HybridTimestamp time) {
         assert time != null;
 
+        // TODO https://issues.apache.org/jira/browse/IGNITE-23675
         CompletableFuture<Void> newFuture = notificationFuture
                 .thenComposeAsync(v -> {
                     // Revision must be the same for all entries.
diff --git 
a/modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/impl/StandaloneMetaStorageManager.java
 
b/modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/impl/StandaloneMetaStorageManager.java
index dac763e142..3c007c7bcc 100644
--- 
a/modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/impl/StandaloneMetaStorageManager.java
+++ 
b/modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/impl/StandaloneMetaStorageManager.java
@@ -247,14 +247,20 @@ public class StandaloneMetaStorageManager extends 
MetaStorageManagerImpl {
         }
 
         when(raftGroupService.run(any())).thenAnswer(invocation -> {
-            Command command = invocation.getArgument(0);
             RaftGroupListener listener = listenerCaptor.getValue();
+            // Both onBeforeApply and command processing within listener 
should be thread-safe.
+            // onBeforeApply is guarded by group specific monitor, precisely 
synchronized (groupIdSyncMonitor(request.groupId())).
+            // See ActionRequestProcessor.handleRequestInternal for more 
details.
+            // Command processing on its turn is expected to be processed 
under raft umbrella, meaning in single-thread environment.
+            synchronized (listener) {
+                Command command = invocation.getArgument(0);
+
+                if (listener instanceof BeforeApplyHandler && command 
instanceof WriteCommand) {
+                    ((BeforeApplyHandler) listener).onBeforeApply(command);
+                }
 
-            if (listener instanceof BeforeApplyHandler) {
-                ((BeforeApplyHandler) listener).onBeforeApply(command);
+                return runCommand(command, listener);
             }
-
-            return runCommand(command, listener);
         });
 
         return raftManager;

Reply via email to