You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/04/20 05:10:18 UTC

[GitHub] [iotdb] qiaojialin commented on a diff in pull request #5592: [IOTDB-2942] [IOTDB-2944] Delete duplicate data structure in node-commons and extend TDataNodeLocation

qiaojialin commented on code in PR #5592:
URL: https://github.com/apache/iotdb/pull/5592#discussion_r853649916


##########
confignode/src/assembly/resources/conf/iotdb-confignode.properties:
##########
@@ -152,18 +152,18 @@ data_node_consensus_protocol_class=org.apache.iotdb.consensus.standalone.StandAl
 # Datatype: long
 # default_ttl=36000000
 
-####################
-### Region Configuration
-####################
+# Default number of SchemaRegion replicas
+# Datatype: int
+# default_schema_replication_factor=3

Review Comment:
   ```suggestion
   # schema_replication_factor=3
   ```



##########
confignode/src/assembly/resources/conf/iotdb-confignode.properties:
##########
@@ -152,18 +152,18 @@ data_node_consensus_protocol_class=org.apache.iotdb.consensus.standalone.StandAl
 # Datatype: long
 # default_ttl=36000000
 
-####################
-### Region Configuration
-####################
+# Default number of SchemaRegion replicas
+# Datatype: int
+# default_schema_replication_factor=3
 
-# The number of replicas of each region
+# Default number of DataRegion replicas
 # Datatype: int
-region_replica_count=1
+# default_data_replication_factor=3

Review Comment:
   ```suggestion
   # data_replication_factor=3
   ```



##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConf.java:
##########
@@ -90,12 +90,17 @@ public class ConfigNodeConf {
   /** Default TTL for storage groups that are not set TTL by statements, in ms. */
   private long defaultTTL = Long.MAX_VALUE;
 
-  /** The number of replicas of each region */
-  private int regionReplicaCount = 3;
-  /** The number of SchemaRegions of each StorageGroup */
-  private int schemaRegionCount = 1;
-  /** The number of DataRegions of each StorageGroup */
-  private int dataRegionCount = 1;
+  /** Default number of SchemaRegion replicas */
+  private int defaultSchemaReplicationFactor = 3;

Review Comment:
   ```suggestion
     private int schemaReplicationFactor = 3;
   ```



##########
confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/server/ConfigNodeRPCServerProcessor.java:
##########
@@ -92,23 +89,33 @@ public TDataNodeRegisterResp registerDataNode(TDataNodeRegisterReq req) throws T
   }
 
   @Override
-  public TDataNodeMessageResp getDataNodesMessage(int dataNodeID) throws TException {
+  public TDataNodeLocationResp getDataNodeLocations(int dataNodeID) throws TException {
     QueryDataNodeInfoPlan plan = new QueryDataNodeInfoPlan(dataNodeID);
-    DataNodesInfoDataSet dataSet = (DataNodesInfoDataSet) configManager.getDataNodeInfo(plan);
+    DataNodeLocationsDataSet dataSet =
+        (DataNodeLocationsDataSet) configManager.getDataNodeInfo(plan);
 
-    TDataNodeMessageResp resp = new TDataNodeMessageResp();
-    dataSet.convertToRPCDataNodeMessageResp(resp);
+    TDataNodeLocationResp resp = new TDataNodeLocationResp();
+    dataSet.convertToRpcDataNodeLocationResp(resp);
     return resp;
   }
 
   @Override
   public TSStatus setStorageGroup(TSetStorageGroupReq req) throws TException {
-    SetStorageGroupPlan plan =
-        new SetStorageGroupPlan(new StorageGroupSchema(req.getStorageGroup()));
-
-    // TODO: Set TTL by optional field TSetStorageGroupReq.TTL
-    plan.getSchema().setTTL(ConfigNodeDescriptor.getInstance().getConf().getDefaultTTL());
-
+    TStorageGroupSchema storageGroupSchema = new TStorageGroupSchema();

Review Comment:
   get from TSetStorageGroupReq



##########
confignode/src/main/java/org/apache/iotdb/confignode/physical/crud/GetOrCreateSchemaPartitionPlan.java:
##########
@@ -70,12 +73,12 @@ protected void deserializeImpl(ByteBuffer buffer) throws IOException {
     partitionSlotsMap = new HashMap<>();
     int storageGroupNum = buffer.getInt();
     for (int i = 0; i < storageGroupNum; i++) {
-      String storageGroup = SerializeDeserializeUtil.readString(buffer);
+      String storageGroup = BasicStructureSerializeDeserializeUtil.readString(buffer);
       partitionSlotsMap.put(storageGroup, new ArrayList<>());
       int seriesPartitionSlotNum = buffer.getInt();
       for (int j = 0; j < seriesPartitionSlotNum; j++) {
-        SeriesPartitionSlot seriesPartitionSlot = new SeriesPartitionSlot();
-        seriesPartitionSlot.deserializeImpl(buffer);
+        TSeriesPartitionSlot seriesPartitionSlot =
+            ThriftCommonsSerializeDeserializeUtils.readTSeriesPartitionSlot(buffer);

Review Comment:
   ```suggestion
               ThriftCommonsSerDeUtils.readTSeriesPartitionSlot(buffer);
   ```



##########
confignode/src/test/java/org/apache/iotdb/confignode/physical/PhysicalPlanSerializeDeserializeTest.java:
##########
@@ -52,7 +52,7 @@
 import java.util.Map;
 import java.util.Set;
 
-public class SerializeDeserializeUT {
+public class PhysicalPlanSerializeDeserializeTest {

Review Comment:
   ```suggestion
   public class PhysicalPlanSerDeTest {
   ```



##########
confignode/src/main/java/org/apache/iotdb/confignode/physical/sys/AuthorPlan.java:
##########
@@ -132,11 +132,11 @@ public void setUserName(String userName) {
 
   @Override
   protected void serializeImpl(ByteBuffer buffer) {
-    SerializeDeserializeUtil.write(getPlanTypeOrdinal(authorType), buffer);
-    SerializeDeserializeUtil.write(userName, buffer);
-    SerializeDeserializeUtil.write(roleName, buffer);
-    SerializeDeserializeUtil.write(password, buffer);
-    SerializeDeserializeUtil.write(newPassword, buffer);
+    BasicStructureSerializeDeserializeUtil.write(getPlanTypeOrdinal(authorType), buffer);
+    BasicStructureSerializeDeserializeUtil.write(userName, buffer);

Review Comment:
   ```suggestion
       BasicStructureSerDeUtil.write(userName, buffer);
   ```



##########
confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/server/ConfigNodeRPCServerProcessor.java:
##########
@@ -92,23 +89,32 @@ public TDataNodeRegisterResp registerDataNode(TDataNodeRegisterReq req) throws T
   }
 
   @Override
-  public TDataNodeMessageResp getDataNodesMessage(int dataNodeID) throws TException {
+  public TDataNodeLocationResp getDataNodeLocations(int dataNodeID) throws TException {
     QueryDataNodeInfoPlan plan = new QueryDataNodeInfoPlan(dataNodeID);
-    DataNodesInfoDataSet dataSet = (DataNodesInfoDataSet) configManager.getDataNodeInfo(plan);
+    DataNodeLocationsDataSet dataSet =
+        (DataNodeLocationsDataSet) configManager.getDataNodeInfo(plan);
 
-    TDataNodeMessageResp resp = new TDataNodeMessageResp();
-    dataSet.convertToRPCDataNodeMessageResp(resp);
+    TDataNodeLocationResp resp = new TDataNodeLocationResp();
+    dataSet.convertToRpcDataNodeLocationResp(resp);
     return resp;
   }
 
   @Override
   public TSStatus setStorageGroup(TSetStorageGroupReq req) throws TException {
-    SetStorageGroupPlan plan =
-        new SetStorageGroupPlan(new StorageGroupSchema(req.getStorageGroup()));
-
-    // TODO: Set TTL by optional field TSetStorageGroupReq.TTL
-    plan.getSchema().setTTL(ConfigNodeDescriptor.getInstance().getConf().getDefaultTTL());
-
+    TStorageGroupSchema storageGroupSchema = req.getStorageGroup();

Review Comment:
   set these fields only they are not set



-- 
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: reviews-unsubscribe@iotdb.apache.org

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