xingtanzjr commented on code in PR #7136:
URL: https://github.com/apache/iotdb/pull/7136#discussion_r965852077


##########
server/src/main/java/org/apache/iotdb/db/localconfignode/LocalDataPartitionTable.java:
##########
@@ -19,116 +19,72 @@
 
 package org.apache.iotdb.db.localconfignode;
 
+import org.apache.iotdb.commons.consensus.ConsensusGroupId;
 import org.apache.iotdb.commons.consensus.DataRegionId;
-import org.apache.iotdb.commons.exception.IllegalPathException;
 import org.apache.iotdb.commons.path.PartialPath;
+import org.apache.iotdb.db.conf.IoTDBDescriptor;
 
-import java.util.ArrayList;
-import java.util.Collections;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.concurrent.atomic.AtomicInteger;
 
-// This class is used for data partition maintaining the map between storage 
group and
-// dataRegionIds.
 public class LocalDataPartitionTable {
-
-  private AtomicInteger dataRegionIdGenerator;
-
-  private Map<PartialPath, List<DataRegionId>> table;
-
-  private static class LocalDataPartitionTableHolder {
-    private static final LocalDataPartitionTable INSTANCE = new 
LocalDataPartitionTable();
-
-    private LocalDataPartitionTableHolder() {};
-  }
-
-  private LocalDataPartitionTable() {}
-
-  public static LocalDataPartitionTable getInstance() {
-    return LocalDataPartitionTableHolder.INSTANCE;
-  }
-
-  public synchronized void init(Map<String, List<DataRegionId>> 
recoveredLocalDataRegionInfo)
-      throws IllegalPathException {
-    table = new ConcurrentHashMap<>();
-    dataRegionIdGenerator = new AtomicInteger(0);
-    for (Map.Entry<String, List<DataRegionId>> entry : 
recoveredLocalDataRegionInfo.entrySet()) {
-      String storageGroup = entry.getKey();
-      List<DataRegionId> dataRegionIdList = new CopyOnWriteArrayList<>();
-      table.put(new PartialPath(storageGroup), dataRegionIdList);
-      for (DataRegionId dataRegionId : 
recoveredLocalDataRegionInfo.get(storageGroup)) {
-        dataRegionIdList.add(dataRegionId);
-
-        if (dataRegionId.getId() >= dataRegionIdGenerator.get()) {
-          dataRegionIdGenerator.set(dataRegionId.getId() + 1);
-        }
-      }
+  private static final Logger LOG = 
LoggerFactory.getLogger(LocalDataPartitionTable.class);
+
+  private String storageGroupName;
+  private final int regionNum;
+  private DataRegionId[] regionIds;
+
+  public LocalDataPartitionTable(String storageGroupName, List<DataRegionId> 
regions) {
+    this.storageGroupName = storageGroupName;
+    this.regionNum = regions.size();
+    regions.sort(Comparator.comparingInt(ConsensusGroupId::getId));
+    this.regionIds = new DataRegionId[regions.size()];
+    for (int i = 0; i < regions.size(); ++i) {
+      regionIds[i] = regions.get(i);
     }
   }
 
-  public synchronized void clear() {
-    if (table != null) {
-      table.clear();
-      table = null;
-    }
-
-    if (dataRegionIdGenerator != null) {
-      dataRegionIdGenerator = null;
-    }
-  }
-
-  public synchronized void putDataRegionId(PartialPath storageGroup, 
DataRegionId dataRegionId) {
-    table.get(storageGroup).add(dataRegionId);
-  }
-
-  public synchronized void removeDataRegionId(PartialPath storageGroup, 
DataRegionId dataRegionId) {
-    table.get(storageGroup).remove(dataRegionId);
-  }
-
-  public DataRegionId getDataRegionId(PartialPath storageGroup, PartialPath 
path) {
-    if (!table.containsKey(storageGroup)) {
-      return null;
-    }
-    return table.get(storageGroup).get(0);
-  }
-
-  public List<DataRegionId> getInvolvedDataRegionIds(
-      PartialPath storageGroup, PartialPath pathPattern, boolean 
isPrefixMatch) {
-    List<DataRegionId> result = new ArrayList<>();
-    if (table.containsKey(storageGroup)) {
-      result.addAll(table.get(storageGroup));
-    }
-    return result;
+  public LocalDataPartitionTable(String storageGroupName) {
+    this.storageGroupName = storageGroupName;
+    this.regionNum = 
IoTDBDescriptor.getInstance().getConfig().getDataRegionNum();
+    this.regionIds = new DataRegionId[regionNum];
   }
 
-  public List<DataRegionId> getDataRegionIdsByStorageGroup(PartialPath 
storageGroup) {
-    return table.getOrDefault(storageGroup, Collections.emptyList());
+  /**
+   * Get the data region id which the path located in.
+   *
+   * @param path The full path for the series.
+   * @return The region id for the path.
+   */
+  public DataRegionId getDataRegionId(PartialPath path) {
+    int idx = Math.abs(path.hashCode()) % regionNum;

Review Comment:
   We need to use a common method to calculate the `index` for partition 
algorithm to avoid the inconsistent calculation ways in other places such as 
line 78.
   
   Furthermore, the Math.abs(path.hashCode()) may generate negative value so it 
is not safe here. Try to use a more safe way to calculate the index



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to