You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by me...@apache.org on 2022/06/17 09:23:07 UTC

[shardingsphere] branch master updated: Optimize lock judge engine (#18411)

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

menghaoran pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git


The following commit(s) were added to refs/heads/master by this push:
     new c904ff88940 Optimize lock judge engine (#18411)
c904ff88940 is described below

commit c904ff889406caffecb1f2ee99a4af64d0cd86a4
Author: gin <ja...@163.com>
AuthorDate: Fri Jun 17 17:23:00 2022 +0800

    Optimize lock judge engine (#18411)
    
    * Optimize lock judge engine.
    
    * Simplify code
    
    * Simplify code
---
 .../infra/instance/InstanceContext.java            |  9 +----
 ...dgeEngine.java => AbstractLockJudgeEngine.java} | 26 ++++---------
 .../lock/ShardingSphereLockJudgeEngine.java        | 45 +---------------------
 .../lock/DistributedLockContextTest.java           |  8 ++--
 4 files changed, 15 insertions(+), 73 deletions(-)

diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/instance/InstanceContext.java b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/instance/InstanceContext.java
index a17fa712026..3154f31f73f 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/instance/InstanceContext.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/instance/InstanceContext.java
@@ -52,8 +52,8 @@ public final class InstanceContext {
         this.workerIdGenerator = workerIdGenerator;
         this.modeConfiguration = modeConfiguration;
         this.lockContext = lockContext;
-        initLockContext();
         getWorkerId();
+        lockContext.initLockState(this);
     }
     
     /**
@@ -194,13 +194,6 @@ public final class InstanceContext {
         return computeNodeInstances.stream().filter(each -> instanceId.equals(each.getCurrentInstanceId())).findFirst();
     }
     
-    /**
-     * Init lock context.
-     */
-    public void initLockContext() {
-        lockContext.initLockState(this);
-    }
-    
     /**
      * Is cluster instance or not.
      * 
diff --git a/shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/ShardingSphereLockJudgeEngine.java b/shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/AbstractLockJudgeEngine.java
similarity index 76%
copy from shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/ShardingSphereLockJudgeEngine.java
copy to shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/AbstractLockJudgeEngine.java
index 598f8e4a15c..8825d882c7e 100644
--- a/shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/ShardingSphereLockJudgeEngine.java
+++ b/shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/AbstractLockJudgeEngine.java
@@ -17,8 +17,7 @@
 
 package org.apache.shardingsphere.mode.manager.lock;
 
-import lombok.RequiredArgsConstructor;
-import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
+import lombok.Getter;
 import org.apache.shardingsphere.infra.lock.LockContext;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.SQLStatement;
 import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DDLStatement;
@@ -30,13 +29,13 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
- * Lock judge engine for ShardingSphere.
+ * Abstract lock judge engine.
  */
-@RequiredArgsConstructor
-public final class ShardingSphereLockJudgeEngine implements LockJudgeEngine {
+public abstract class AbstractLockJudgeEngine implements LockJudgeEngine {
     
     private static final Set<Class<? extends SQLStatement>> IGNORABLE_SQL_STATEMENT_CLASSES_STOP_WRITING = Collections.newSetFromMap(new ConcurrentHashMap<>());
     
+    @Getter
     private LockContext lockContext;
     
     @Override
@@ -45,21 +44,12 @@ public final class ShardingSphereLockJudgeEngine implements LockJudgeEngine {
     }
     
     /**
-     * Is locked.
+     * Is write statement.
      *
-     * @param databaseName database name
-     * @param sqlStatementContext sql statement context
-     * @return is locked or not
+     * @param sqlStatement sql statement
+     * @return is write statement or not
      */
-    @Override
-    public boolean isLocked(final String databaseName, final SQLStatementContext<?> sqlStatementContext) {
-        if (isWriteStatement(sqlStatementContext.getSqlStatement())) {
-            return lockContext.isLocked(databaseName);
-        }
-        return false;
-    }
-    
-    private boolean isWriteStatement(final SQLStatement sqlStatement) {
+    protected boolean isWriteStatement(final SQLStatement sqlStatement) {
         Class<? extends SQLStatement> sqlStatementClass = sqlStatement.getClass();
         if (IGNORABLE_SQL_STATEMENT_CLASSES_STOP_WRITING.contains(sqlStatementClass)) {
             return false;
diff --git a/shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/ShardingSphereLockJudgeEngine.java b/shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/ShardingSphereLockJudgeEngine.java
index 598f8e4a15c..a0ab6728759 100644
--- a/shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/ShardingSphereLockJudgeEngine.java
+++ b/shardingsphere-mode/shardingsphere-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/lock/ShardingSphereLockJudgeEngine.java
@@ -19,30 +19,12 @@ package org.apache.shardingsphere.mode.manager.lock;
 
 import lombok.RequiredArgsConstructor;
 import org.apache.shardingsphere.infra.binder.statement.SQLStatementContext;
-import org.apache.shardingsphere.infra.lock.LockContext;
-import org.apache.shardingsphere.sql.parser.sql.common.statement.SQLStatement;
-import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DDLStatement;
-import org.apache.shardingsphere.sql.parser.sql.common.statement.dml.DMLStatement;
-import org.apache.shardingsphere.sql.parser.sql.common.statement.dml.SelectStatement;
-
-import java.util.Collections;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Lock judge engine for ShardingSphere.
  */
 @RequiredArgsConstructor
-public final class ShardingSphereLockJudgeEngine implements LockJudgeEngine {
-    
-    private static final Set<Class<? extends SQLStatement>> IGNORABLE_SQL_STATEMENT_CLASSES_STOP_WRITING = Collections.newSetFromMap(new ConcurrentHashMap<>());
-    
-    private LockContext lockContext;
-    
-    @Override
-    public void init(final LockContext lockContext) {
-        this.lockContext = lockContext;
-    }
+public final class ShardingSphereLockJudgeEngine extends AbstractLockJudgeEngine {
     
     /**
      * Is locked.
@@ -54,31 +36,8 @@ public final class ShardingSphereLockJudgeEngine implements LockJudgeEngine {
     @Override
     public boolean isLocked(final String databaseName, final SQLStatementContext<?> sqlStatementContext) {
         if (isWriteStatement(sqlStatementContext.getSqlStatement())) {
-            return lockContext.isLocked(databaseName);
+            return getLockContext().isLocked(databaseName);
         }
         return false;
     }
-    
-    private boolean isWriteStatement(final SQLStatement sqlStatement) {
-        Class<? extends SQLStatement> sqlStatementClass = sqlStatement.getClass();
-        if (IGNORABLE_SQL_STATEMENT_CLASSES_STOP_WRITING.contains(sqlStatementClass)) {
-            return false;
-        }
-        if (sqlStatement instanceof SelectStatement) {
-            catchIgnorable(sqlStatementClass);
-            return false;
-        }
-        if (sqlStatement instanceof DMLStatement) {
-            return true;
-        }
-        if (sqlStatement instanceof DDLStatement) {
-            return true;
-        }
-        catchIgnorable(sqlStatementClass);
-        return false;
-    }
-    
-    private void catchIgnorable(final Class<? extends SQLStatement> sqlStatementClass) {
-        IGNORABLE_SQL_STATEMENT_CLASSES_STOP_WRITING.add(sqlStatementClass);
-    }
 }
diff --git a/shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/test/java/org/apache/shardingsphere/mode/manager/cluster/coordinator/lock/DistributedLockContextTest.java b/shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/test/java/org/apache/shardingsphere/mode/manager/cluster/coordinator/lock/DistributedLockContextTest.java
index 12ccbb65bd9..6c184612fa5 100644
--- a/shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/test/java/org/apache/shardingsphere/mode/manager/cluster/coordinator/lock/DistributedLockContextTest.java
+++ b/shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/test/java/org/apache/shardingsphere/mode/manager/cluster/coordinator/lock/DistributedLockContextTest.java
@@ -39,7 +39,7 @@ public final class DistributedLockContextTest {
     public void assertGetDistributedLock() {
         DistributedLockContext distributedLockContext = new DistributedLockContext(mock(ClusterPersistRepository.class));
         ComputeNodeInstance currentInstance = new ComputeNodeInstance(new InstanceDefinition(InstanceType.PROXY, "127.0.0.1@3307"));
-        new InstanceContext(currentInstance, mock(WorkerIdGenerator.class), mock(ModeConfiguration.class), distributedLockContext).initLockContext();
+        new InstanceContext(currentInstance, mock(WorkerIdGenerator.class), mock(ModeConfiguration.class), distributedLockContext);
         assertThat(distributedLockContext.getLock(), instanceOf(ShardingSphereLock.class));
     }
     
@@ -47,7 +47,7 @@ public final class DistributedLockContextTest {
     public void assertTryLock() {
         ComputeNodeInstance currentInstance = new ComputeNodeInstance(new InstanceDefinition(InstanceType.PROXY, "127.0.0.1@3307"));
         DistributedLockContext distributedLockContext = new DistributedLockContext(mock(ClusterPersistRepository.class));
-        new InstanceContext(currentInstance, mock(WorkerIdGenerator.class), mock(ModeConfiguration.class), distributedLockContext).initLockContext();
+        new InstanceContext(currentInstance, mock(WorkerIdGenerator.class), mock(ModeConfiguration.class), distributedLockContext);
         assertNotNull(distributedLockContext.getLock());
     }
     
@@ -55,7 +55,7 @@ public final class DistributedLockContextTest {
     public void assertReleaseLock() {
         ComputeNodeInstance currentInstance = new ComputeNodeInstance(new InstanceDefinition(InstanceType.PROXY, "127.0.0.1@3307"));
         DistributedLockContext distributedLockContext = new DistributedLockContext(mock(ClusterPersistRepository.class));
-        new InstanceContext(currentInstance, mock(WorkerIdGenerator.class), mock(ModeConfiguration.class), distributedLockContext).initLockContext();
+        new InstanceContext(currentInstance, mock(WorkerIdGenerator.class), mock(ModeConfiguration.class), distributedLockContext);
         distributedLockContext.releaseLock("database");
     }
     
@@ -63,7 +63,7 @@ public final class DistributedLockContextTest {
     public void assertIsLockedDatabase() {
         ComputeNodeInstance currentInstance = new ComputeNodeInstance(new InstanceDefinition(InstanceType.PROXY, "127.0.0.1@3307"));
         DistributedLockContext distributedLockContext = new DistributedLockContext(mock(ClusterPersistRepository.class));
-        new InstanceContext(currentInstance, mock(WorkerIdGenerator.class), mock(ModeConfiguration.class), distributedLockContext).initLockContext();
+        new InstanceContext(currentInstance, mock(WorkerIdGenerator.class), mock(ModeConfiguration.class), distributedLockContext);
         assertFalse(distributedLockContext.isLocked("database"));
     }
 }