You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by az...@apache.org on 2023/04/03 01:58:20 UTC

[shardingsphere] branch master updated: Improve part of pipeline code follow spotbugs (#24954)

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

azexin 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 f4e59930228 Improve part of pipeline code follow spotbugs (#24954)
f4e59930228 is described below

commit f4e59930228301849068d1a32ec51f9419661b90
Author: Hongsheng Zhong <zh...@apache.org>
AuthorDate: Mon Apr 3 09:58:08 2023 +0800

    Improve part of pipeline code follow spotbugs (#24954)
    
    * Improve ManualBitSet IS2_INCONSISTENT_SYNC
    
    * Improve PipelineAPIFactory JLM_JSR166_UTILCONCURRENT_MONITORENTER
    
    * Simplify code of data consistency check
    
    * Update spotbugs.xml
---
 .../data/pipeline/core/api/PipelineAPIFactory.java | 24 +++++++++++++---------
 ...SingleTableInventoryDataConsistencyChecker.java | 13 +++---------
 ...DataMatchDataConsistencyCalculateAlgorithm.java |  8 +-------
 .../core/ingest/channel/memory/ManualBitSet.java   | 12 +++++------
 src/resources/spotbugs.xml                         | 10 ++++-----
 5 files changed, 27 insertions(+), 40 deletions(-)

diff --git a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/api/PipelineAPIFactory.java b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/api/PipelineAPIFactory.java
index d1c536b9dc9..c666aea828d 100644
--- a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/api/PipelineAPIFactory.java
+++ b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/api/PipelineAPIFactory.java
@@ -18,7 +18,6 @@
 package org.apache.shardingsphere.data.pipeline.core.api;
 
 import lombok.AccessLevel;
-import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.SneakyThrows;
 import org.apache.commons.lang3.concurrent.ConcurrentException;
@@ -76,7 +75,7 @@ public final class PipelineAPIFactory {
      * @return job statistics API
      */
     public static JobStatisticsAPI getJobStatisticsAPI(final PipelineContextKey contextKey) {
-        return ElasticJobAPIHolder.getInstance(contextKey).getJobStatisticsAPI();
+        return ElasticJobAPIHolder.getInstance(contextKey).jobStatisticsAPI;
     }
     
     /**
@@ -86,7 +85,7 @@ public final class PipelineAPIFactory {
      * @return job configuration API
      */
     public static JobConfigurationAPI getJobConfigurationAPI(final PipelineContextKey contextKey) {
-        return ElasticJobAPIHolder.getInstance(contextKey).getJobConfigurationAPI();
+        return ElasticJobAPIHolder.getInstance(contextKey).jobConfigurationAPI;
     }
     
     /**
@@ -96,7 +95,7 @@ public final class PipelineAPIFactory {
      * @return job operate API
      */
     public static JobOperateAPI getJobOperateAPI(final PipelineContextKey contextKey) {
-        return ElasticJobAPIHolder.getInstance(contextKey).getJobOperateAPI();
+        return ElasticJobAPIHolder.getInstance(contextKey).jobOperateAPI;
     }
     
     /**
@@ -106,10 +105,9 @@ public final class PipelineAPIFactory {
      * @return Coordinator registry center
      */
     public static CoordinatorRegistryCenter getRegistryCenter(final PipelineContextKey contextKey) {
-        return RegistryCenterHolder.getInstance(contextKey);
+        return RegistryCenterHolder.getInstance(contextKey).registryCenter;
     }
     
-    @Getter
     private static final class ElasticJobAPIHolder {
         
         private static final Map<PipelineContextKey, ElasticJobAPIHolder> INSTANCE_MAP = new ConcurrentHashMap<>();
@@ -134,13 +132,15 @@ public final class PipelineAPIFactory {
     
     private static final class RegistryCenterHolder {
         
-        private static final Map<PipelineContextKey, CoordinatorRegistryCenter> INSTANCE_MAP = new ConcurrentHashMap<>();
+        private static final Map<PipelineContextKey, RegistryCenterHolder> INSTANCE_MAP = new ConcurrentHashMap<>();
         
-        public static CoordinatorRegistryCenter getInstance(final PipelineContextKey contextKey) {
-            return INSTANCE_MAP.computeIfAbsent(contextKey, key -> createRegistryCenter(contextKey));
+        private final CoordinatorRegistryCenter registryCenter;
+        
+        private RegistryCenterHolder(final PipelineContextKey contextKey) {
+            registryCenter = createRegistryCenter(contextKey);
         }
         
-        private static CoordinatorRegistryCenter createRegistryCenter(final PipelineContextKey contextKey) {
+        private CoordinatorRegistryCenter createRegistryCenter(final PipelineContextKey contextKey) {
             CoordinatorRegistryCenterInitializer registryCenterInitializer = new CoordinatorRegistryCenterInitializer();
             PipelineContext pipelineContext = PipelineContextManager.getContext(contextKey);
             ModeConfiguration modeConfig = pipelineContext.getModeConfig();
@@ -152,5 +152,9 @@ public final class PipelineAPIFactory {
                 throw new IllegalArgumentException("Unsupported cluster type: " + clusterType);
             }
         }
+        
+        public static RegistryCenterHolder getInstance(final PipelineContextKey contextKey) {
+            return INSTANCE_MAP.computeIfAbsent(contextKey, key -> new RegistryCenterHolder(contextKey));
+        }
     }
 }
diff --git a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/SingleTableInventoryDataConsistencyChecker.java b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/SingleTableInventoryDataConsistencyChecker.java
index 465cce01383..6a36be7d52a 100644
--- a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/SingleTableInventoryDataConsistencyChecker.java
+++ b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/SingleTableInventoryDataConsistencyChecker.java
@@ -105,16 +105,9 @@ public final class SingleTableInventoryDataConsistencyChecker {
         Iterator<DataConsistencyCalculatedResult> targetCalculatedResults = waitFuture(executor.submit(() -> calculateAlgorithm.calculate(targetParam))).iterator();
         try {
             return check0(sourceCalculatedResults, targetCalculatedResults, executor);
-            // CHECKSTYLE:OFF
-        } catch (final RuntimeException ex) {
-            // CHECKSTYLE:ON
-            if (null != sourceParam.getCalculationContext()) {
-                CloseUtils.closeQuietly(sourceParam.getCalculationContext());
-            }
-            if (null != targetParam.getCalculationContext()) {
-                CloseUtils.closeQuietly(targetParam.getCalculationContext());
-            }
-            throw ex;
+        } finally {
+            CloseUtils.closeQuietly(sourceParam.getCalculationContext());
+            CloseUtils.closeQuietly(targetParam.getCalculationContext());
         }
     }
     
diff --git a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/algorithm/DataMatchDataConsistencyCalculateAlgorithm.java b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/algorithm/DataMatchDataConsistencyCalculateAlgorithm.java
index 3278b6f3b0e..4b8339c8a23 100644
--- a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/algorithm/DataMatchDataConsistencyCalculateAlgorithm.java
+++ b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/algorithm/DataMatchDataConsistencyCalculateAlgorithm.java
@@ -132,17 +132,11 @@ public final class DataMatchDataConsistencyCalculateAlgorithm extends AbstractSt
         }
         try {
             result = createCalculationContext(param);
-            // CHECKSTYLE:OFF
-        } catch (final Exception ex) {
-            // CHECKSTYLE:ON
-            throw new PipelineTableDataConsistencyCheckLoadingFailedException(param.getSchemaName(), param.getLogicTableName(), ex);
-        }
-        try {
             fulfillCalculationContext(result, param);
             // CHECKSTYLE:OFF
         } catch (final Exception ex) {
             // CHECKSTYLE:ON
-            result.close();
+            CloseUtils.closeQuietly(result);
             throw new PipelineTableDataConsistencyCheckLoadingFailedException(param.getSchemaName(), param.getLogicTableName(), ex);
         }
         return result;
diff --git a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/ingest/channel/memory/ManualBitSet.java b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/ingest/channel/memory/ManualBitSet.java
index c3b3b49ef08..7ee7134dcdd 100644
--- a/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/ingest/channel/memory/ManualBitSet.java
+++ b/kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/ingest/channel/memory/ManualBitSet.java
@@ -110,14 +110,12 @@ public final class ManualBitSet {
      *
      * @param bitIndex retain bit index
      */
-    public void clear(final long bitIndex) {
+    public synchronized void clear(final long bitIndex) {
         if ((bitIndex - startIndex) > BIT_SET_SIZE) {
-            synchronized (this) {
-                int count = Math.min(bitSets.size(), (int) ((bitIndex - startIndex) / BIT_SET_SIZE));
-                if (count > 0) {
-                    bitSets.subList(0, count).clear();
-                    startIndex += (long) count * BIT_SET_SIZE;
-                }
+            int count = Math.min(bitSets.size(), (int) ((bitIndex - startIndex) / BIT_SET_SIZE));
+            if (count > 0) {
+                bitSets.subList(0, count).clear();
+                startIndex += (long) count * BIT_SET_SIZE;
             }
         }
     }
diff --git a/src/resources/spotbugs.xml b/src/resources/spotbugs.xml
index 22a6461089d..5eff3a404c5 100644
--- a/src/resources/spotbugs.xml
+++ b/src/resources/spotbugs.xml
@@ -97,20 +97,18 @@
         <Class name="org.apache.shardingsphere.data.pipeline.core.util.JDBCStreamQueryUtils" />
         <Bug code="OBL" />
     </Match>
-    <!-- TODO hongsheng fix the ignored bug -->
     <Match>
         <Class name="org.apache.shardingsphere.data.pipeline.core.check.consistency.algorithm.DataMatchDataConsistencyCalculateAlgorithm" />
         <Bug code="OBL" />
     </Match>
-    <!-- TODO hongsheng fix the ignored bug -->
     <Match>
-        <Class name="org.apache.shardingsphere.data.pipeline.core.ingest.channel.memory.ManualBitSet" />
+        <Class name="org.apache.shardingsphere.data.pipeline.mysql.ingest.client.MySQLClient" />
         <Bug code="IS" />
     </Match>
-    <!-- TODO hongsheng fix the ignored bug -->
+    <!-- TODO chuxin fix the ignored bug -->
     <Match>
-        <Class name="org.apache.shardingsphere.data.pipeline.mysql.ingest.client.MySQLClient" />
-        <Bug code="IS" />
+        <Class name="org.apache.shardingsphere.data.pipeline.core.execute.ShardingSphereDataJobWorker" />
+        <Bug code="JLM" />
     </Match>
     <!-- TODO zhengqiang fix the ignored bug -->
     <Match>