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;