From 2df99bf902002b9de445c0b6b0ca7c41b17825e7 Mon Sep 17 00:00:00 2001 From: Liang Zhang Date: Tue, 17 Sep 2024 12:18:40 +0800 Subject: [PATCH] Add more test cases on StatePersistService (#32911) --- .../mode/manager/ContextManager.java | 3 +- .../persist/service/StatePersistService.java | 12 +++---- .../service/StatePersistServiceTest.java | 31 ++++++++++++++----- .../ral/updatable/UnlockClusterExecutor.java | 2 +- .../impl/ClusterReadWriteLockStrategy.java | 2 +- .../lock/impl/ClusterWriteLockStrategy.java | 2 +- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/ContextManager.java b/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/ContextManager.java index 99f8d4309d056..01925b3208a70 100644 --- a/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/ContextManager.java +++ b/mode/core/src/main/java/org/apache/shardingsphere/mode/manager/ContextManager.java @@ -33,7 +33,6 @@ import org.apache.shardingsphere.infra.metadata.database.schema.builder.GenericSchemaBuilderMaterial; import org.apache.shardingsphere.infra.metadata.database.schema.model.ShardingSphereSchema; import org.apache.shardingsphere.infra.spi.ShardingSphereServiceLoader; -import org.apache.shardingsphere.infra.state.cluster.ClusterState; import org.apache.shardingsphere.mode.manager.listener.ContextManagerLifecycleListener; import org.apache.shardingsphere.mode.metadata.MetaDataContextManager; import org.apache.shardingsphere.mode.metadata.MetaDataContexts; @@ -70,7 +69,7 @@ public ContextManager(final MetaDataContexts metaDataContexts, final ComputeNode this.computeNodeInstanceContext = computeNodeInstanceContext; metaDataContextManager = new MetaDataContextManager(this.metaDataContexts, computeNodeInstanceContext, repository); persistServiceFacade = new PersistServiceFacade(repository, computeNodeInstanceContext.getModeConfiguration(), metaDataContextManager); - stateContext = new StateContext(persistServiceFacade.getStatePersistService().loadClusterState().orElse(ClusterState.OK)); + stateContext = new StateContext(persistServiceFacade.getStatePersistService().load()); executorEngine = ExecutorEngine.createExecutorEngineWithSize(metaDataContexts.getMetaData().getProps().getValue(ConfigurationPropertyKey.KERNEL_EXECUTOR_SIZE)); for (ContextManagerLifecycleListener each : ShardingSphereServiceLoader.getServiceInstances(ContextManagerLifecycleListener.class)) { each.onInitialized(this); diff --git a/mode/core/src/main/java/org/apache/shardingsphere/mode/persist/service/StatePersistService.java b/mode/core/src/main/java/org/apache/shardingsphere/mode/persist/service/StatePersistService.java index 4ab21a6c8c78b..144afcd9556a1 100644 --- a/mode/core/src/main/java/org/apache/shardingsphere/mode/persist/service/StatePersistService.java +++ b/mode/core/src/main/java/org/apache/shardingsphere/mode/persist/service/StatePersistService.java @@ -23,8 +23,6 @@ import org.apache.shardingsphere.metadata.persist.node.ComputeNode; import org.apache.shardingsphere.mode.spi.PersistRepository; -import java.util.Optional; - /** * State persist service. */ @@ -36,19 +34,19 @@ public final class StatePersistService { /** * Update cluster state. * - * @param state cluster state + * @param state to be updated cluster state */ - public void updateClusterState(final ClusterState state) { + public void update(final ClusterState state) { repository.persist(ComputeNode.getClusterStateNodePath(), state.name()); } /** * Load cluster state. * - * @return cluster state + * @return loaded cluster state */ - public Optional loadClusterState() { + public ClusterState load() { String value = repository.query(ComputeNode.getClusterStateNodePath()); - return Strings.isNullOrEmpty(value) ? Optional.empty() : Optional.of(ClusterState.valueOf(value)); + return Strings.isNullOrEmpty(value) ? ClusterState.OK : ClusterState.valueOf(value); } } diff --git a/mode/core/src/test/java/org/apache/shardingsphere/mode/persist/service/StatePersistServiceTest.java b/mode/core/src/test/java/org/apache/shardingsphere/mode/persist/service/StatePersistServiceTest.java index 3805820356044..d4c013d06f113 100644 --- a/mode/core/src/test/java/org/apache/shardingsphere/mode/persist/service/StatePersistServiceTest.java +++ b/mode/core/src/test/java/org/apache/shardingsphere/mode/persist/service/StatePersistServiceTest.java @@ -18,31 +18,46 @@ package org.apache.shardingsphere.mode.persist.service; import org.apache.shardingsphere.infra.state.cluster.ClusterState; -import org.apache.shardingsphere.metadata.persist.node.ComputeNode; import org.apache.shardingsphere.mode.spi.PersistRepository; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class StatePersistServiceTest { + private StatePersistService statePersistService; + @Mock private PersistRepository repository; + @BeforeEach + void setUp() { + statePersistService = new StatePersistService(repository); + } + + @Test + void assertUpdate() { + statePersistService.update(ClusterState.OK); + verify(repository).persist("/nodes/compute_nodes/status", ClusterState.OK.name()); + } + @Test - void assertUpdateClusterStateClusterStateWithoutPath() { - StatePersistService statePersistService = new StatePersistService(repository); - statePersistService.updateClusterState(ClusterState.OK); - verify(repository).persist(ComputeNode.getClusterStateNodePath(), ClusterState.OK.name()); + void assertLoad() { + when(repository.query("/nodes/compute_nodes/status")).thenReturn(ClusterState.READ_ONLY.name()); + assertThat(statePersistService.load(), is(ClusterState.READ_ONLY)); } @Test - void assertLoadClusterStateClusterState() { - new StatePersistService(repository).loadClusterState(); - verify(repository).query(ComputeNode.getClusterStateNodePath()); + void assertLoadWithEmptyState() { + when(repository.query("/nodes/compute_nodes/status")).thenReturn(""); + assertThat(statePersistService.load(), is(ClusterState.OK)); } } diff --git a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/updatable/UnlockClusterExecutor.java b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/updatable/UnlockClusterExecutor.java index 997cb61791342..fb5dab9870ae0 100644 --- a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/updatable/UnlockClusterExecutor.java +++ b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/distsql/ral/updatable/UnlockClusterExecutor.java @@ -43,7 +43,7 @@ public void executeUpdate(final UnlockClusterStatement sqlStatement, final Conte if (lockContext.tryLock(lockDefinition, 3000L)) { try { checkState(contextManager); - contextManager.getPersistServiceFacade().getStatePersistService().updateClusterState(ClusterState.OK); + contextManager.getPersistServiceFacade().getStatePersistService().update(ClusterState.OK); // TODO unlock snapshot info if locked } finally { lockContext.unlock(lockDefinition); diff --git a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterReadWriteLockStrategy.java b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterReadWriteLockStrategy.java index ad9dba4c64036..93820e91e5603 100644 --- a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterReadWriteLockStrategy.java +++ b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterReadWriteLockStrategy.java @@ -28,7 +28,7 @@ public class ClusterReadWriteLockStrategy implements ClusterLockStrategy { @Override public void lock() { - ProxyContext.getInstance().getContextManager().getPersistServiceFacade().getStatePersistService().updateClusterState(ClusterState.UNAVAILABLE); + ProxyContext.getInstance().getContextManager().getPersistServiceFacade().getStatePersistService().update(ClusterState.UNAVAILABLE); } @Override diff --git a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterWriteLockStrategy.java b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterWriteLockStrategy.java index 24da33d6b8652..081f8f6bb0d30 100644 --- a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterWriteLockStrategy.java +++ b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/lock/impl/ClusterWriteLockStrategy.java @@ -28,7 +28,7 @@ public class ClusterWriteLockStrategy implements ClusterLockStrategy { @Override public void lock() { - ProxyContext.getInstance().getContextManager().getPersistServiceFacade().getStatePersistService().updateClusterState(ClusterState.READ_ONLY); + ProxyContext.getInstance().getContextManager().getPersistServiceFacade().getStatePersistService().update(ClusterState.READ_ONLY); } @Override