Refactor WorkerIdAssignedException (#31317)

apache · May 20, 2024 · d40ea0a · d40ea0a
1 parent ed015b1
commit d40ea0a
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 17 deletions.
diff --git a/docs/document/content/user-manual/error-code/sql-error-code.cn.md b/docs/document/content/user-manual/error-code/sql-error-code.cn.md
@@ -103,7 +103,7 @@ SQL 错误码以标准的 SQL State，Vendor Code 和详细错误信息提供，
 | Vendor Code | SQL State | 错误信息                                                          |
 |-------------|-----------|---------------------------------------------------------------|
 | 17000       | 44000     | Mode must be 'cluster'.                                       |
-| 17001       | HY000     | Work ID assigned failed, which can not exceed 1024.           |
+| 17001       | HY000     | Worker ID assigned failed, which should be in [0, %s).        |
 | 17010       | HY000     | Cluster persist repository error, reason is: %s               |
 | 17020       | HY000     | The cluster status is %s, can not support SQL statement '%s'. |
 | 17030       | HY000     | Cluster is already locked.                                    |

diff --git a/docs/document/content/user-manual/error-code/sql-error-code.en.md b/docs/document/content/user-manual/error-code/sql-error-code.en.md
@@ -103,7 +103,7 @@ SQL error codes provide by standard `SQL State`, `Vendor Code` and `Reason`, whi
 | Vendor Code | SQL State | Reason                                                        |
 |-------------|-----------|---------------------------------------------------------------|
 | 17000       | 44000     | Mode must be 'cluster'.                                       |
-| 17001       | HY000     | Work ID assigned failed, which can not exceed 1024.           |
+| 17001       | HY000     | Worker ID assigned failed, which should be in [0, %s).        |
 | 17010       | HY000     | Cluster persist repository error, reason is: %s               |
 | 17020       | HY000     | The cluster status is %s, can not support SQL statement '%s'. |
 | 17030       | HY000     | Cluster is already locked.                                    |

diff --git a/...id/exception/WorkIdAssignedException.java → ...e/workerid/WorkerIdAssignedException.java b/...id/exception/WorkIdAssignedException.java → ...e/workerid/WorkerIdAssignedException.java
@@ -15,19 +15,19 @@
  * limitations under the License.
  */
 
-package org.apache.shardingsphere.mode.manager.cluster.coordinator.registry.workerid.exception;
+package org.apache.shardingsphere.infra.instance.workerid;
 
 import org.apache.shardingsphere.infra.exception.core.external.sql.sqlstate.XOpenSQLState;
 import org.apache.shardingsphere.infra.exception.core.external.sql.type.kernel.category.ClusterSQLException;
 
 /**
- * Work id assigned exception.
+ * Worker ID assigned exception.
  */
-public final class WorkIdAssignedException extends ClusterSQLException {
+public final class WorkerIdAssignedException extends ClusterSQLException {
 
     private static final long serialVersionUID = 4782736481041926266L;
 
-    public WorkIdAssignedException() {
-        super(XOpenSQLState.GENERAL_ERROR, 1, "Work ID assigned failed, which can not exceed 1024.");
+    public WorkerIdAssignedException() {
+        super(XOpenSQLState.GENERAL_ERROR, 1, "Worker ID assigned failed, which should be in [0, %s).", WorkerIdGenerator.MAX_WORKER_ID + 1);
     }
 }
diff --git a/...ode/manager/cluster/coordinator/registry/workerid/generator/ClusterWorkerIdGenerator.java b/...ode/manager/cluster/coordinator/registry/workerid/generator/ClusterWorkerIdGenerator.java
@@ -23,7 +23,7 @@
 import org.apache.shardingsphere.infra.instance.metadata.InstanceMetaData;
 import org.apache.shardingsphere.infra.instance.workerid.WorkerIdGenerator;
 import org.apache.shardingsphere.mode.manager.cluster.coordinator.registry.status.compute.service.ComputeNodeStatusService;
-import org.apache.shardingsphere.mode.manager.cluster.coordinator.registry.workerid.exception.WorkIdAssignedException;
+import org.apache.shardingsphere.infra.instance.workerid.WorkerIdAssignedException;
 import org.apache.shardingsphere.mode.manager.cluster.coordinator.registry.workerid.node.WorkerIdNode;
 import org.apache.shardingsphere.mode.repository.cluster.ClusterPersistRepository;
 import org.apache.shardingsphere.mode.repository.cluster.exception.ClusterPersistRepositoryException;
@@ -57,12 +57,16 @@ public ClusterWorkerIdGenerator(final ClusterPersistRepository repository, final
 
     @Override
     public int generate(final Properties props) {
-        int result = computeNodeStatusService.loadInstanceWorkerId(instanceMetaData.getId()).orElseGet(this::reGenerate);
-        checkIneffectiveConfiguration(result, props);
+        int result = loadExistedWorkerId().orElseGet(this::generateNewWorkerId);
+        logWarning(result, props);
         return result;
     }
 
-    private int reGenerate() {
+    private Optional<Integer> loadExistedWorkerId() {
+        return computeNodeStatusService.loadInstanceWorkerId(instanceMetaData.getId());
+    }
+
+    private int generateNewWorkerId() {
         Optional<Integer> generatedWorkId;
         do {
             generatedWorkId = generateAvailableWorkerId();
@@ -74,7 +78,7 @@ private int reGenerate() {
 
     private Optional<Integer> generateAvailableWorkerId() {
         Collection<Integer> assignedWorkerIds = computeNodeStatusService.getAssignedWorkerIds();
-        ShardingSpherePreconditions.checkState(assignedWorkerIds.size() <= 1024, WorkIdAssignedException::new);
+        ShardingSpherePreconditions.checkState(assignedWorkerIds.size() <= MAX_WORKER_ID + 1, WorkerIdAssignedException::new);
         Collection<Integer> availableWorkerIds = new LinkedList<>();
         for (int i = 0; i < 1024; i++) {
             availableWorkerIds.add(i);
@@ -93,10 +97,10 @@ private Optional<Integer> generateAvailableWorkerId() {
         }
     }
 
-    private void checkIneffectiveConfiguration(final int generatedWorkerId, final Properties props) {
-        if (!isWarned.get() && null != props && props.containsKey(WORKER_ID_KEY)) {
+    private void logWarning(final int generatedWorkerId, final Properties props) {
+        if (!isWarned.get() && props.containsKey(WORKER_ID_KEY)) {
             isWarned.set(true);
-            log.warn("No need to configured {} in cluster mode, system assigned {} was {}", WORKER_ID_KEY, WORKER_ID_KEY, generatedWorkerId);
+            log.warn("It is unnecessary to configure {} in cluster mode, system assigned {} was {}", WORKER_ID_KEY, WORKER_ID_KEY, generatedWorkerId);
         }
     }
 }
diff --git a/...hardingsphere/mode/manager/standalone/workerid/generator/StandaloneWorkerIdGenerator.java b/...hardingsphere/mode/manager/standalone/workerid/generator/StandaloneWorkerIdGenerator.java
@@ -17,7 +17,8 @@
 
 package org.apache.shardingsphere.mode.manager.standalone.workerid.generator;
 
-import com.google.common.base.Preconditions;
+import org.apache.shardingsphere.infra.exception.core.ShardingSpherePreconditions;
+import org.apache.shardingsphere.infra.instance.workerid.WorkerIdAssignedException;
 import org.apache.shardingsphere.infra.instance.workerid.WorkerIdGenerator;
 
 import java.util.Properties;
@@ -35,7 +36,7 @@ public int generate(final Properties props) {
             return DEFAULT_WORKER_ID;
         }
         int result = Integer.parseInt(props.get(WORKER_ID_KEY).toString());
-        Preconditions.checkState(result <= MAX_WORKER_ID, "%s can not exceed %s", WORKER_ID_KEY, MAX_WORKER_ID);
+        ShardingSpherePreconditions.checkState(result >= 0 && result <= MAX_WORKER_ID, WorkerIdAssignedException::new);
         return result;
     }
 }