[GitHub] [shardingsphere] sandynz commented on a change in pull request #14991: For #14722：Support only one scaling job could be executed at the same time for one schema

[GitBox <gi...@apache.org>]

sandynz commented on a change in pull request #14991:
URL: https://github.com/apache/shardingsphere/pull/14991#discussion_r789636393



##########
File path: shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/cluster/coordinator/registry/cache/subscriber/ScalingRegistrySubscriber.java
##########
@@ -75,8 +81,12 @@ public void ruleConfigurationCached(final RuleConfigurationCachedEvent event) {
         String sourceRule = repository.get(SchemaMetaDataNode.getRulePath(event.getSchemaName()));
         String targetRule = registryCacheManager.loadCache(SchemaMetaDataNode.getRulePath(event.getSchemaName()), event.getCacheId());
         String ruleCacheId = event.getCacheId();
-        StartScalingEvent startScalingEvent = new StartScalingEvent(event.getSchemaName(), sourceDataSource, sourceRule, targetRule, ruleCacheId);
-        ShardingSphereEventBus.getInstance().post(startScalingEvent);
+        lockStatus = lockRegistryService.tryLock(event.getSchemaName(), 1000);
+        if (lockStatus) {
+            log.info("start scaling job, locked the schema name, event={}", event);
+            StartScalingEvent startScalingEvent = new StartScalingEvent(event.getSchemaName(), sourceDataSource, sourceRule, targetRule, ruleCacheId);
+            ShardingSphereEventBus.getInstance().post(startScalingEvent);
+        }

Review comment:
       Could be
   ```
   if (!lockStatus) {
       return;
   }
   ```
   and keep original code unchanged.

##########
File path: shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/cluster/coordinator/registry/cache/subscriber/ScalingRegistrySubscriber.java
##########
@@ -98,6 +108,10 @@ public void scalingTaskFinished(final ScalingTaskFinishedEvent event) {
             log.info("start to delete cache, ruleCacheId={}", ruleCacheId);
             registryCacheManager.deleteCache(SchemaMetaDataNode.getRulePath(event.getTargetSchemaName()), ruleCacheId);
         }
+        if (lockStatus) {

Review comment:
       From code, seems `ScalingTaskFinishedEvent` is not sent in every case, e.g. target DB broken and scaling job failed. The lock might be not released properly.

##########
File path: shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/cluster/coordinator/registry/cache/subscriber/ScalingRegistrySubscriber.java
##########
@@ -56,11 +57,16 @@
     
     private final RegistryCacheManager registryCacheManager;
     
+    private final LockRegistryService lockRegistryService;
+    
+    private boolean lockStatus;

Review comment:
       Questions:
   1. Is `voilatile` necessary?
   2. Should `lockStatus` be isolated by every schema name?
   3. Could `lockStatus` name be `locked` like?

##########
File path: shardingsphere-mode/shardingsphere-mode-type/shardingsphere-cluster-mode/shardingsphere-cluster-mode-core/src/main/java/org/apache/shardingsphere/mode/manager/cluster/coordinator/registry/cache/subscriber/ScalingRegistrySubscriber.java
##########
@@ -98,6 +108,10 @@ public void scalingTaskFinished(final ScalingTaskFinishedEvent event) {
             log.info("start to delete cache, ruleCacheId={}", ruleCacheId);
             registryCacheManager.deleteCache(SchemaMetaDataNode.getRulePath(event.getTargetSchemaName()), ruleCacheId);
         }
+        if (lockStatus) {
+            log.info("scaling job finished, release schema name lock, event = {}", event);
+            lockRegistryService.releaseLock(event.getTargetSchemaName());

Review comment:
       Should the lock name be decorated? e.g. scaling-schemaName.




-- 
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-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org