You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by du...@apache.org on 2023/05/12 06:49:21 UTC

[shardingsphere] branch master updated: Fix sonar issue for SnowflakeKeyGenerateAlgorithm (#25602)

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

duanzhengqiang 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 c5462510039 Fix sonar issue for SnowflakeKeyGenerateAlgorithm (#25602)
c5462510039 is described below

commit c5462510039d064b7c47468b8f72c3d211927b68
Author: Liang Zhang <zh...@apache.org>
AuthorDate: Fri May 12 14:49:14 2023 +0800

    Fix sonar issue for SnowflakeKeyGenerateAlgorithm (#25602)
---
 .../keygen/SnowflakeKeyGenerateAlgorithm.java      | 81 ++++++++++++----------
 .../keygen/SnowflakeClockMoveBackException.java    |  4 +-
 .../keygen/SnowflakeKeyGenerateAlgorithmTest.java  | 13 ++--
 3 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java
index d6125d23b6f..abd453950e2 100644
--- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java
+++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithm.java
@@ -28,6 +28,9 @@ import org.apache.shardingsphere.sharding.spi.KeyGenerateAlgorithm;
 
 import java.util.Calendar;
 import java.util.Properties;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Snowflake key generate algorithm.
@@ -46,13 +49,13 @@ public final class SnowflakeKeyGenerateAlgorithm implements KeyGenerateAlgorithm
     
     private static final String MAX_VIBRATION_OFFSET_KEY = "max-vibration-offset";
     
-    private static final String MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY = "max-tolerate-time-difference-milliseconds";
+    private static final String MAX_TOLERATE_TIME_DIFFERENCE_MILLIS_KEY = "max-tolerate-time-difference-milliseconds";
     
     private static final long SEQUENCE_BITS = 12L;
     
     private static final long WORKER_ID_BITS = 10L;
     
-    private static final long SEQUENCE_MASK = (1 << SEQUENCE_BITS) - 1;
+    private static final long SEQUENCE_MASK = (1 << SEQUENCE_BITS) - 1L;
     
     private static final long WORKER_ID_LEFT_SHIFT_BITS = SEQUENCE_BITS;
     
@@ -60,26 +63,26 @@ public final class SnowflakeKeyGenerateAlgorithm implements KeyGenerateAlgorithm
     
     private static final int DEFAULT_VIBRATION_VALUE = 1;
     
-    private static final int MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS = 10;
+    private static final int MAX_TOLERATE_TIME_DIFFERENCE_MILLIS = 10;
     
     private static final int DEFAULT_WORKER_ID = 0;
     
     @Setter
     private static TimeService timeService = new TimeService();
     
-    private Properties props;
+    private final AtomicReference<InstanceContext> instanceContext = new AtomicReference<>();
     
-    private int maxVibrationOffset;
+    private final AtomicInteger sequenceOffset = new AtomicInteger(-1);
     
-    private int maxTolerateTimeDifferenceMilliseconds;
+    private final AtomicLong sequence = new AtomicLong();
     
-    private volatile int sequenceOffset = -1;
+    private final AtomicLong lastMillis = new AtomicLong();
     
-    private volatile long sequence;
+    private Properties props;
     
-    private volatile long lastMilliseconds;
+    private int maxVibrationOffset;
     
-    private volatile InstanceContext instanceContext;
+    private int maxTolerateTimeDifferenceMillis;
     
     static {
         Calendar calendar = Calendar.getInstance();
@@ -95,15 +98,7 @@ public final class SnowflakeKeyGenerateAlgorithm implements KeyGenerateAlgorithm
     public void init(final Properties props) {
         this.props = props;
         maxVibrationOffset = getMaxVibrationOffset(props);
-        maxTolerateTimeDifferenceMilliseconds = getMaxTolerateTimeDifferenceMilliseconds(props);
-    }
-    
-    @Override
-    public void setInstanceContext(final InstanceContext instanceContext) {
-        this.instanceContext = instanceContext;
-        if (null != instanceContext) {
-            instanceContext.generateWorkerId(props);
-        }
+        maxTolerateTimeDifferenceMillis = getMaxTolerateTimeDifferenceMillis(props);
     }
     
     private int getMaxVibrationOffset(final Properties props) {
@@ -112,38 +107,47 @@ public final class SnowflakeKeyGenerateAlgorithm implements KeyGenerateAlgorithm
         return result;
     }
     
-    private int getMaxTolerateTimeDifferenceMilliseconds(final Properties props) {
-        int result = Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLISECONDS).toString());
+    private int getMaxTolerateTimeDifferenceMillis(final Properties props) {
+        int result = Integer.parseInt(props.getOrDefault(MAX_TOLERATE_TIME_DIFFERENCE_MILLIS_KEY, MAX_TOLERATE_TIME_DIFFERENCE_MILLIS).toString());
         ShardingSpherePreconditions.checkState(result >= 0, () -> new KeyGenerateAlgorithmInitializationException(getType(), "Illegal max tolerate time difference milliseconds."));
         return result;
     }
     
+    @Override
+    public void setInstanceContext(final InstanceContext instanceContext) {
+        this.instanceContext.set(instanceContext);
+        if (null != instanceContext) {
+            instanceContext.generateWorkerId(props);
+        }
+    }
+    
     @Override
     public synchronized Long generateKey() {
-        long currentMilliseconds = timeService.getCurrentMillis();
-        if (waitTolerateTimeDifferenceIfNeed(currentMilliseconds)) {
-            currentMilliseconds = timeService.getCurrentMillis();
+        long currentMillis = timeService.getCurrentMillis();
+        if (waitTolerateTimeDifferenceIfNeed(currentMillis)) {
+            currentMillis = timeService.getCurrentMillis();
         }
-        if (lastMilliseconds == currentMilliseconds) {
-            if (0L == (sequence = (sequence + 1) & SEQUENCE_MASK)) {
-                currentMilliseconds = waitUntilNextTime(currentMilliseconds);
+        if (lastMillis.get() == currentMillis) {
+            sequence.set(sequence.incrementAndGet() & SEQUENCE_MASK);
+            if (0L == sequence.get()) {
+                currentMillis = waitUntilNextTime(currentMillis);
             }
         } else {
             vibrateSequenceOffset();
-            sequence = sequenceOffset;
+            sequence.set(sequenceOffset.get());
         }
-        lastMilliseconds = currentMilliseconds;
-        return ((currentMilliseconds - EPOCH) << TIMESTAMP_LEFT_SHIFT_BITS) | ((long) getWorkerId() << WORKER_ID_LEFT_SHIFT_BITS) | sequence;
+        lastMillis.set(currentMillis);
+        return ((currentMillis - EPOCH) << TIMESTAMP_LEFT_SHIFT_BITS) | ((long) getWorkerId() << WORKER_ID_LEFT_SHIFT_BITS) | sequence.get();
     }
     
     @SneakyThrows(InterruptedException.class)
-    private boolean waitTolerateTimeDifferenceIfNeed(final long currentMilliseconds) {
-        if (lastMilliseconds <= currentMilliseconds) {
+    private boolean waitTolerateTimeDifferenceIfNeed(final long currentMillis) {
+        if (lastMillis.get() <= currentMillis) {
             return false;
         }
-        long timeDifferenceMilliseconds = lastMilliseconds - currentMilliseconds;
-        ShardingSpherePreconditions.checkState(timeDifferenceMilliseconds < maxTolerateTimeDifferenceMilliseconds, () -> new SnowflakeClockMoveBackException(lastMilliseconds, currentMilliseconds));
-        Thread.sleep(timeDifferenceMilliseconds);
+        long timeDifferenceMillis = lastMillis.get() - currentMillis;
+        ShardingSpherePreconditions.checkState(timeDifferenceMillis < maxTolerateTimeDifferenceMillis, () -> new SnowflakeClockMoveBackException(lastMillis.get(), currentMillis));
+        Thread.sleep(timeDifferenceMillis);
         return true;
     }
     
@@ -155,13 +159,14 @@ public final class SnowflakeKeyGenerateAlgorithm implements KeyGenerateAlgorithm
         return result;
     }
     
-    @SuppressWarnings("NonAtomicOperationOnVolatileField")
     private void vibrateSequenceOffset() {
-        sequenceOffset = sequenceOffset >= maxVibrationOffset ? 0 : sequenceOffset + 1;
+        if (!sequenceOffset.compareAndSet(maxVibrationOffset, 0)) {
+            sequenceOffset.incrementAndGet();
+        }
     }
     
     private int getWorkerId() {
-        return null == instanceContext ? DEFAULT_WORKER_ID : instanceContext.getWorkerId();
+        return null == instanceContext.get() ? DEFAULT_WORKER_ID : instanceContext.get().getWorkerId();
     }
     
     @Override
diff --git a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/exception/algorithm/keygen/SnowflakeClockMoveBackException.java b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/exception/algorithm/keygen/SnowflakeClockMoveBackException.java
index 6580470afdc..cfc56220842 100644
--- a/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/exception/algorithm/keygen/SnowflakeClockMoveBackException.java
+++ b/features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/exception/algorithm/keygen/SnowflakeClockMoveBackException.java
@@ -27,7 +27,7 @@ public final class SnowflakeClockMoveBackException extends ShardingSQLException
     
     private static final long serialVersionUID = -2435731376659956566L;
     
-    public SnowflakeClockMoveBackException(final long lastMilliseconds, final long currentMilliseconds) {
-        super(XOpenSQLState.GENERAL_ERROR, 92, "Clock is moving backwards, last time is %d milliseconds, current time is %d milliseconds.", lastMilliseconds, currentMilliseconds);
+    public SnowflakeClockMoveBackException(final long lastMillis, final long currentMillis) {
+        super(XOpenSQLState.GENERAL_ERROR, 92, "Clock is moving backwards, last time is %d milliseconds, current time is %d milliseconds.", lastMillis, currentMillis);
     }
 }
diff --git a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithmTest.java b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithmTest.java
index 4f480898fc7..08d7e4e55f9 100644
--- a/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithmTest.java
+++ b/features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/algorithm/keygen/SnowflakeKeyGenerateAlgorithmTest.java
@@ -47,6 +47,7 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicLong;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -144,7 +145,7 @@ class SnowflakeKeyGenerateAlgorithmTest {
         if (algorithm instanceof InstanceContextAware) {
             ((InstanceContextAware) algorithm).setInstanceContext(INSTANCE);
         }
-        setLastMilliseconds(algorithm, timeService.getCurrentMillis() + 2);
+        setLastMillis(algorithm, timeService.getCurrentMillis() + 2);
         List<Comparable<?>> expected = Arrays.asList(4194304L, 8388609L, 8388610L, 12582912L, 12582913L, 16777217L, 16777218L, 20971520L, 20971521L, 25165825L);
         List<Comparable<?>> actual = new ArrayList<>(DEFAULT_KEY_AMOUNT);
         for (int i = 0; i < DEFAULT_KEY_AMOUNT; i++) {
@@ -161,7 +162,7 @@ class SnowflakeKeyGenerateAlgorithmTest {
         if (algorithm instanceof InstanceContextAware) {
             ((InstanceContextAware) algorithm).setInstanceContext(INSTANCE);
         }
-        setLastMilliseconds(algorithm, timeService.getCurrentMillis() + 2);
+        setLastMillis(algorithm, timeService.getCurrentMillis() + 2);
         assertThrows(SnowflakeClockMoveBackException.class, () -> batchGenerate(algorithm));
     }
     
@@ -179,7 +180,7 @@ class SnowflakeKeyGenerateAlgorithmTest {
         if (algorithm instanceof InstanceContextAware) {
             ((InstanceContextAware) algorithm).setInstanceContext(INSTANCE);
         }
-        setLastMilliseconds(algorithm, timeService.getCurrentMillis());
+        setLastMillis(algorithm, timeService.getCurrentMillis());
         setSequence(algorithm, (1 << DEFAULT_SEQUENCE_BITS) - 1L);
         List<Comparable<?>> expected = Arrays.asList(4194304L, 4194305L, 4194306L, 8388608L, 8388609L, 8388610L, 12582913L, 12582914L, 12582915L, 16777216L);
         List<Comparable<?>> actual = new ArrayList<>(DEFAULT_KEY_AMOUNT);
@@ -190,13 +191,13 @@ class SnowflakeKeyGenerateAlgorithmTest {
     }
     
     @SneakyThrows(ReflectiveOperationException.class)
-    private void setLastMilliseconds(final KeyGenerateAlgorithm algorithm, final Number value) {
-        Plugins.getMemberAccessor().set(SnowflakeKeyGenerateAlgorithm.class.getDeclaredField("lastMilliseconds"), algorithm, value);
+    private void setLastMillis(final KeyGenerateAlgorithm algorithm, final Number value) {
+        Plugins.getMemberAccessor().set(SnowflakeKeyGenerateAlgorithm.class.getDeclaredField("lastMillis"), algorithm, new AtomicLong(value.longValue()));
     }
     
     @SneakyThrows(ReflectiveOperationException.class)
     private void setSequence(final KeyGenerateAlgorithm algorithm, final Number value) {
-        Plugins.getMemberAccessor().set(SnowflakeKeyGenerateAlgorithm.class.getDeclaredField("sequence"), algorithm, value);
+        Plugins.getMemberAccessor().set(SnowflakeKeyGenerateAlgorithm.class.getDeclaredField("sequence"), algorithm, new AtomicLong(value.longValue()));
     }
     
     @Test