OneSizeFitsQuorum commented on code in PR #12752:
URL: https://github.com/apache/iotdb/pull/12752#discussion_r1643961359


##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/DataNode.java:
##########
@@ -397,15 +386,18 @@ private void storeRuntimeConfigurations(
   /**
    * Register this DataNode into cluster.
    *
+   * @param isPreCheck do pre-check before formal registration
    * @throws StartupException if register failed.
-   * @throws IOException if serialize cluster name and dataNode Id failed.
+   * @throws IOException if serialize cluster name and DataNode id failed.

Review Comment:
   datanode



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/file/SystemPropertiesHandler.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.commons.file;
+
+import org.apache.iotdb.commons.exception.StartupException;
+
+import org.apache.ratis.util.AutoCloseableLock;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Properties;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+public class SystemPropertiesHandler {
+  private static final Logger LOGGER = 
LoggerFactory.getLogger(SystemPropertiesHandler.class);
+
+  private final File formalFile;
+  private final File tmpFile;
+  //    private ImmutableProperties cache;

Review Comment:
   。



##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java:
##########
@@ -407,11 +407,12 @@ public DataSet registerDataNode(TDataNodeRegisterReq req) 
{
               req.getClusterName(),
               req.getDataNodeConfiguration().getLocation(),
               this);
-      if (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+      if (!req.isPreCheck() && status.getCode() == 
TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
         return nodeManager.registerDataNode(req);
       }
     }
     DataNodeRegisterResp resp = new DataNodeRegisterResp();
+    resp.setPreCheck(req.isPreCheck());

Review Comment:
   do not add this



##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/conf/SystemPropertiesUtils.java:
##########
@@ -74,11 +66,7 @@ private SystemPropertiesUtils() {
 
   // TODO: This needs removal of statics ...
   public static void reinitializeStatics() {
-    systemPropertiesFile =
-        new File(
-            ConfigNodeDescriptor.getInstance().getConf().getSystemDir()
-                + File.separator
-                + ConfigNodeConstant.SYSTEM_FILE_NAME);
+    systemPropertiesHandler = SystemPropertiesHandler.getInstance();

Review Comment:
   init



##########
iotdb-protocol/thrift-confignode/src/main/thrift/confignode.thrift:
##########
@@ -404,6 +405,7 @@ struct TConfigNodeRegisterReq {
   1: required TClusterParameters clusterParameters
   2: required common.TConfigNodeLocation configNodeLocation
   3: optional TNodeVersionInfo versionInfo
+  4: optional bool preCheck

Review Comment:
   remove



##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/service/ConfigNode.java:
##########
@@ -205,7 +205,9 @@ public void active() {
       // We set up Non-Seed ConfigNode's RPC service before sending the 
register request
       // in order to facilitate the scheduling of capacity expansion process 
in ConfigNode-leader
       setUpRPCService();
-      sendRegisterConfigNodeRequest();
+      sendRegisterConfigNodeRequest(true);
+      SystemPropertiesUtils.storeSystemParameters();

Review Comment:
   remove 



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/DataNode.java:
##########
@@ -170,9 +167,6 @@ private DataNode() {
 
   // TODO: This needs removal of statics ...
   public static void reinitializeStatics() {
-    SYSTEM_PROPERTIES =

Review Comment:
   add init



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/file/SystemPropertiesHandler.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.commons.file;
+
+import org.apache.iotdb.commons.exception.StartupException;
+
+import org.apache.ratis.util.AutoCloseableLock;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Properties;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+public class SystemPropertiesHandler {
+  private static final Logger LOGGER = 
LoggerFactory.getLogger(SystemPropertiesHandler.class);
+
+  private final File formalFile;
+  private final File tmpFile;
+  //    private ImmutableProperties cache;
+  ReadWriteLock lock = new ReentrantReadWriteLock();
+
+  private SystemPropertiesHandler(String filePath) {
+    this.formalFile = SystemFileFactory.INSTANCE.getFile(filePath);
+    this.tmpFile = SystemFileFactory.INSTANCE.getFile(filePath + ".tmp");
+  }
+
+  /**
+   * Put key and value into properties
+   *
+   * @param keyOrValue [key0, value0, key1, value1, ... ]
+   */
+  public void put(Object... keyOrValue) throws IOException {
+    if (keyOrValue.length % 2 != 0) {
+      throw new IllegalArgumentException(
+          "Length of parameters should be evenly divided by 2, but the actual 
length is "
+              + keyOrValue.length
+              + " : "
+              + Arrays.toString(keyOrValue));
+    }
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.writeLock());
+        FileOutputStream outputStream = new FileOutputStream(tmpFile)) {
+      Properties properties = new Properties();
+      // read old properties
+      if (formalFile.exists()) {
+        try (FileInputStream inputStream = new FileInputStream(formalFile)) {
+          properties.load(new InputStreamReader(inputStream, 
StandardCharsets.UTF_8));
+        }
+      }
+      // store new properties
+      for (int i = 0; i < keyOrValue.length; i += 2) {
+        properties.put(keyOrValue[i], keyOrValue[i + 1]);
+      }
+      properties.store(new OutputStreamWriter(outputStream, 
StandardCharsets.UTF_8), "");
+      outputStream.getFD().sync();
+      replaceFormalFile();
+    }
+  }
+
+  public void overwrite(Properties properties) throws IOException {
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.writeLock())) {
+      if (!formalFile.exists()) {
+        writeImpl(properties, formalFile);
+        return;
+      }
+      writeImpl(properties, tmpFile);
+      replaceFormalFile();
+    }
+  }
+
+  public Properties read() throws IOException {
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.readLock())) {
+      Properties properties = new Properties();
+      if (formalFile.exists()) {
+        try (FileInputStream inputStream = new FileInputStream(formalFile)) {
+          properties.load(new InputStreamReader(inputStream, 
StandardCharsets.UTF_8));
+        }
+      }
+      return properties;
+    }
+  }
+
+  public boolean fileExist() {
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.readLock())) {
+      return formalFile.exists() || tmpFile.exists();
+    }
+  }
+
+  public boolean isFirstStart() {
+    return !fileExist();
+  }
+
+  private void recover() throws StartupException {
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.writeLock())) {
+      if (formalFile.exists() && !tmpFile.exists()) {
+        // No need to recover
+        return;
+      }
+      if (!formalFile.exists() && !tmpFile.exists()) {
+        // First start
+        return;
+      }
+      if (formalFile.exists() && tmpFile.exists()) {
+        if (!tmpFile.delete()) {
+          LOGGER.warn(
+              "Delete system.properties tmp file fail, you may manually delete 
it: {}",
+              tmpFile.getAbsoluteFile());
+        }
+        return;
+      }
+      if (!formalFile.exists() && tmpFile.exists()) {
+        replaceFormalFile();
+        return;
+      }
+    } catch (IOException e) {
+      throw new StartupException(e);
+    }
+    throw new StartupException("Should never touch here");
+  }
+
+  private void writeImpl(Properties properties, File file) throws IOException {
+    try (FileOutputStream fileOutputStream = new FileOutputStream(file)) {
+      properties.store(new OutputStreamWriter(fileOutputStream, 
StandardCharsets.UTF_8), "");
+    }
+  }
+
+  private void replaceFormalFile() throws IOException {
+    if (!tmpFile.exists()) {
+      throw new FileNotFoundException(
+          "Tmp system properties file must exist when call replaceFormalFile");
+    }
+    if (formalFile.exists() && !formalFile.delete()) {
+      String msg =
+          String.format(
+              "Delete formal system properties file fail: %s", 
formalFile.getAbsoluteFile());
+      throw new IOException(msg);
+    }
+    if (!tmpFile.renameTo(formalFile)) {
+      String msg =
+          String.format(
+              "Failed to replace formal system properties file, you may 
manually rename it: %s -> %s",
+              tmpFile.getAbsolutePath(), formalFile.getAbsolutePath());
+      throw new IOException(msg);
+    }
+  }
+
+  public static void init(String filePath) throws StartupException {

Review Comment:
   try to remove filepath



##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/response/datanode/DataNodeRegisterResp.java:
##########
@@ -38,8 +38,8 @@ public class DataNodeRegisterResp implements DataSet {
   private TSStatus status;
   private List<TConfigNodeLocation> configNodeList;
   private Integer dataNodeId;
-
   private TRuntimeConfiguration runtimeConfiguration;
+  private boolean preCheck = false;

Review Comment:
   .



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/file/SystemPropertiesHandler.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.commons.file;
+
+import org.apache.iotdb.commons.exception.StartupException;
+
+import org.apache.ratis.util.AutoCloseableLock;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Properties;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+public class SystemPropertiesHandler {
+  private static final Logger LOGGER = 
LoggerFactory.getLogger(SystemPropertiesHandler.class);
+
+  private final File formalFile;
+  private final File tmpFile;
+  //    private ImmutableProperties cache;
+  ReadWriteLock lock = new ReentrantReadWriteLock();
+
+  private SystemPropertiesHandler(String filePath) {
+    this.formalFile = SystemFileFactory.INSTANCE.getFile(filePath);
+    this.tmpFile = SystemFileFactory.INSTANCE.getFile(filePath + ".tmp");
+  }
+
+  /**
+   * Put key and value into properties
+   *
+   * @param keyOrValue [key0, value0, key1, value1, ... ]
+   */
+  public void put(Object... keyOrValue) throws IOException {
+    if (keyOrValue.length % 2 != 0) {
+      throw new IllegalArgumentException(
+          "Length of parameters should be evenly divided by 2, but the actual 
length is "
+              + keyOrValue.length
+              + " : "
+              + Arrays.toString(keyOrValue));
+    }
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.writeLock());
+        FileOutputStream outputStream = new FileOutputStream(tmpFile)) {
+      Properties properties = new Properties();
+      // read old properties
+      if (formalFile.exists()) {
+        try (FileInputStream inputStream = new FileInputStream(formalFile)) {
+          properties.load(new InputStreamReader(inputStream, 
StandardCharsets.UTF_8));
+        }
+      }
+      // store new properties
+      for (int i = 0; i < keyOrValue.length; i += 2) {
+        properties.put(keyOrValue[i], keyOrValue[i + 1]);
+      }
+      properties.store(new OutputStreamWriter(outputStream, 
StandardCharsets.UTF_8), "");
+      outputStream.getFD().sync();
+      replaceFormalFile();
+    }
+  }
+
+  public void overwrite(Properties properties) throws IOException {
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.writeLock())) {
+      if (!formalFile.exists()) {
+        writeImpl(properties, formalFile);
+        return;
+      }
+      writeImpl(properties, tmpFile);
+      replaceFormalFile();
+    }
+  }
+
+  public Properties read() throws IOException {
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.readLock())) {
+      Properties properties = new Properties();
+      if (formalFile.exists()) {
+        try (FileInputStream inputStream = new FileInputStream(formalFile)) {
+          properties.load(new InputStreamReader(inputStream, 
StandardCharsets.UTF_8));
+        }
+      }
+      return properties;
+    }
+  }
+
+  public boolean fileExist() {
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.readLock())) {
+      return formalFile.exists() || tmpFile.exists();
+    }
+  }
+
+  public boolean isFirstStart() {
+    return !fileExist();
+  }
+
+  private void recover() throws StartupException {
+    try (AutoCloseableLock ignore = 
AutoCloseableLock.acquire(lock.writeLock())) {
+      if (formalFile.exists() && !tmpFile.exists()) {
+        // No need to recover
+        return;
+      }
+      if (!formalFile.exists() && !tmpFile.exists()) {
+        // First start
+        return;
+      }
+      if (formalFile.exists() && tmpFile.exists()) {
+        if (!tmpFile.delete()) {
+          LOGGER.warn(
+              "Delete system.properties tmp file fail, you may manually delete 
it: {}",
+              tmpFile.getAbsoluteFile());
+        }
+        return;
+      }
+      if (!formalFile.exists() && tmpFile.exists()) {
+        replaceFormalFile();
+        return;
+      }
+    } catch (IOException e) {
+      throw new StartupException(e);
+    }
+    throw new StartupException("Should never touch here");
+  }
+
+  private void writeImpl(Properties properties, File file) throws IOException {
+    try (FileOutputStream fileOutputStream = new FileOutputStream(file)) {
+      properties.store(new OutputStreamWriter(fileOutputStream, 
StandardCharsets.UTF_8), "");
+    }
+  }
+
+  private void replaceFormalFile() throws IOException {
+    if (!tmpFile.exists()) {
+      throw new FileNotFoundException(
+          "Tmp system properties file must exist when call replaceFormalFile");
+    }
+    if (formalFile.exists() && !formalFile.delete()) {
+      String msg =
+          String.format(
+              "Delete formal system properties file fail: %s", 
formalFile.getAbsoluteFile());
+      throw new IOException(msg);
+    }
+    if (!tmpFile.renameTo(formalFile)) {
+      String msg =
+          String.format(
+              "Failed to replace formal system properties file, you may 
manually rename it: %s -> %s",
+              tmpFile.getAbsolutePath(), formalFile.getAbsolutePath());
+      throw new IOException(msg);
+    }
+  }
+
+  public static void init(String filePath) throws StartupException {
+    Holder.INSTANCE = new SystemPropertiesHandler(filePath);
+    Holder.INSTANCE.recover();
+  }
+
+  public static SystemPropertiesHandler getInstance() {
+    if (Holder.INSTANCE == null) {
+      LOGGER.warn(

Review Comment:
   printstacktrace



##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/response/datanode/DataNodeRegisterResp.java:
##########
@@ -84,7 +88,7 @@ public TDataNodeRegisterResp 
convertToRpcDataNodeRegisterResp() {
     resp.setStatus(status);
     resp.setConfigNodeList(configNodeList);
 
-    if (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+    if (!preCheck && status.getCode() == 
TSStatusCode.SUCCESS_STATUS.getStatusCode()) {

Review Comment:
   .



-- 
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