You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by ji...@apache.org on 2024/12/18 07:13:59 UTC

(iotdb) branch fix_set_default_sg_level_13 created (now ce2b54865bc)

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

jiangtian pushed a change to branch fix_set_default_sg_level_13
in repository https://gitbox.apache.org/repos/asf/iotdb.git


      at ce2b54865bc allow illegal config during startup

This branch includes the following new commits:

     new b0ef9d262e5 Fix setting illegal default_storage_group_level does not report an error
     new 67157688ae6 spotless
     new ce2b54865bc allow illegal config during startup

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(iotdb) 03/03: allow illegal config during startup

Posted by ji...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jiangtian pushed a commit to branch fix_set_default_sg_level_13
in repository https://gitbox.apache.org/repos/asf/iotdb.git

commit ce2b54865bcde1ed32be77f52feab1be7d5d1e18
Author: Tian Jiang <jt...@163.com>
AuthorDate: Wed Dec 18 14:53:38 2024 +0800

    allow illegal config during startup
---
 .../iotdb/it/env/cluster/config/MppCommonConfig.java      |  6 ++++++
 .../it/env/cluster/config/MppSharedCommonConfig.java      |  7 +++++++
 .../java/org/apache/iotdb/itbase/env/CommonConfig.java    |  4 ++++
 .../org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java   | 14 ++++++++++++++
 .../main/java/org/apache/iotdb/db/conf/IoTDBConfig.java   | 15 +++++++++++----
 .../java/org/apache/iotdb/db/conf/IoTDBDescriptor.java    | 12 ++++++++----
 .../java/org/apache/iotdb/db/metadata/MetaUtilsTest.java  |  9 ++++++---
 .../resources/conf/iotdb-system.properties.template       |  3 ++-
 8 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/integration-test/src/main/java/org/apache/iotdb/it/env/cluster/config/MppCommonConfig.java b/integration-test/src/main/java/org/apache/iotdb/it/env/cluster/config/MppCommonConfig.java
index 4edf6dd9420..edabfddd7bf 100644
--- a/integration-test/src/main/java/org/apache/iotdb/it/env/cluster/config/MppCommonConfig.java
+++ b/integration-test/src/main/java/org/apache/iotdb/it/env/cluster/config/MppCommonConfig.java
@@ -490,6 +490,12 @@ public class MppCommonConfig extends MppBaseConfig implements CommonConfig {
     return this;
   }
 
+  @Override
+  public CommonConfig setDefaultStorageGroupLevel(int defaultStorageGroupLevel) {
+    setProperty("default_storage_group_level", String.valueOf(defaultStorageGroupLevel));
+    return this;
+  }
+
   // For part of the log directory
   public String getClusterConfigStr() {
     return fromConsensusFullNameToAbbr(properties.getProperty(CONFIG_NODE_CONSENSUS_PROTOCOL_CLASS))
diff --git a/integration-test/src/main/java/org/apache/iotdb/it/env/cluster/config/MppSharedCommonConfig.java b/integration-test/src/main/java/org/apache/iotdb/it/env/cluster/config/MppSharedCommonConfig.java
index 700ca3821c3..c167cad215f 100644
--- a/integration-test/src/main/java/org/apache/iotdb/it/env/cluster/config/MppSharedCommonConfig.java
+++ b/integration-test/src/main/java/org/apache/iotdb/it/env/cluster/config/MppSharedCommonConfig.java
@@ -498,4 +498,11 @@ public class MppSharedCommonConfig implements CommonConfig {
     cnConfig.setQueryMemoryProportion(queryMemoryProportion);
     return this;
   }
+
+  @Override
+  public CommonConfig setDefaultStorageGroupLevel(int defaultStorageGroupLevel) {
+    dnConfig.setDefaultStorageGroupLevel(defaultStorageGroupLevel);
+    cnConfig.setDefaultStorageGroupLevel(defaultStorageGroupLevel);
+    return this;
+  }
 }
diff --git a/integration-test/src/main/java/org/apache/iotdb/itbase/env/CommonConfig.java b/integration-test/src/main/java/org/apache/iotdb/itbase/env/CommonConfig.java
index 38f5a9d8030..1ad4668b53c 100644
--- a/integration-test/src/main/java/org/apache/iotdb/itbase/env/CommonConfig.java
+++ b/integration-test/src/main/java/org/apache/iotdb/itbase/env/CommonConfig.java
@@ -155,4 +155,8 @@ public interface CommonConfig {
       int pipeConnectorRequestSliceThresholdBytes);
 
   CommonConfig setQueryMemoryProportion(String queryMemoryProportion);
+
+  default CommonConfig setDefaultStorageGroupLevel(int defaultStorageGroupLevel) {
+    return this;
+  }
 }
diff --git a/integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java b/integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java
index 289dc895f88..fa8a450d675 100644
--- a/integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java
+++ b/integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java
@@ -236,5 +236,19 @@ public class IoTDBSetConfigurationIT {
         assertTrue(e.getMessage().contains("Illegal defaultStorageGroupLevel: -1, should >= 1"));
       }
     }
+
+    // can start with an illegal value
+    EnvFactory.getEnv().cleanClusterEnvironment();
+    EnvFactory.getEnv().getConfig().getCommonConfig().setDefaultStorageGroupLevel(-1);
+    EnvFactory.getEnv().initClusterEnvironment();
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+      statement.execute("INSERT INTO root.a.b.c.d1(timestamp, s1) VALUES (1, 1)");
+      ResultSet databases = statement.executeQuery("show databases");
+      databases.next();
+      // the default value should take effect
+      Assert.assertEquals("root.a", databases.getString(1));
+      assertFalse(databases.next());
+    }
   }
 }
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java
index 738833bc515..f1d0cc13bc6 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java
@@ -2433,11 +2433,18 @@ public class IoTDBConfig {
     return defaultStorageGroupLevel;
   }
 
-  void setDefaultStorageGroupLevel(int defaultStorageGroupLevel) {
+  void setDefaultStorageGroupLevel(int defaultStorageGroupLevel, boolean startUp) {
     if (defaultStorageGroupLevel < 1) {
-      throw new IllegalArgumentException(
-          String.format(
-              "Illegal defaultStorageGroupLevel: %d, should >= 1", defaultStorageGroupLevel));
+      if (startUp) {
+        logger.warn(
+            "Illegal defaultStorageGroupLevel: {}, should >= 1, use default value 1",
+            defaultStorageGroupLevel);
+        defaultStorageGroupLevel = 1;
+      } else {
+        throw new IllegalArgumentException(
+            String.format(
+                "Illegal defaultStorageGroupLevel: %d, should >= 1", defaultStorageGroupLevel));
+      }
     }
     this.defaultStorageGroupLevel = defaultStorageGroupLevel;
   }
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
index 47d0f938fb3..9134e381980 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
@@ -798,7 +798,8 @@ public class IoTDBDescriptor {
 
     conf.setRpcMaxConcurrentClientNum(maxConcurrentClientNum);
 
-    loadAutoCreateSchemaProps(properties);
+    boolean startUp = true;
+    loadAutoCreateSchemaProps(properties, startUp);
 
     conf.setTsFileStorageFs(
         properties.getProperty("tsfile_storage_fs", conf.getTsFileStorageFs().toString()));
@@ -1624,7 +1625,8 @@ public class IoTDBDescriptor {
     return Math.max(Math.min(newThrottleThreshold, MAX_THROTTLE_THRESHOLD), MIN_THROTTLE_THRESHOLD);
   }
 
-  private void loadAutoCreateSchemaProps(Properties properties) throws IOException {
+  private void loadAutoCreateSchemaProps(Properties properties, boolean startUp)
+      throws IOException {
     conf.setAutoCreateSchemaEnabled(
         Boolean.parseBoolean(
             properties.getProperty(
@@ -1656,7 +1658,8 @@ public class IoTDBDescriptor {
             properties.getProperty(
                 "default_storage_group_level",
                 ConfigurationFileUtils.getConfigurationDefaultValue(
-                    "default_storage_group_level"))));
+                    "default_storage_group_level"))),
+        startUp);
     conf.setDefaultBooleanEncoding(
         properties.getProperty(
             "default_boolean_encoding",
@@ -1896,7 +1899,8 @@ public class IoTDBDescriptor {
       loadTimedService(properties);
       StorageEngine.getInstance().rebootTimedService();
       // update params of creating schema automatically
-      loadAutoCreateSchemaProps(properties);
+      boolean startUp = false;
+      loadAutoCreateSchemaProps(properties, startUp);
 
       // update tsfile-format config
       loadTsFileProps(properties);
diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/metadata/MetaUtilsTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/metadata/MetaUtilsTest.java
index 986fdf95846..6d5b736aa74 100644
--- a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/metadata/MetaUtilsTest.java
+++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/metadata/MetaUtilsTest.java
@@ -45,7 +45,6 @@ public class MetaUtilsTest {
 
   private final IMNodeFactory<IMemMNode> nodeFactory =
       MNodeFactoryLoader.getInstance().getMemMNodeIMNodeFactory();
-  ;
 
   @Test
   public void testGetMultiFullPaths() {
@@ -128,7 +127,9 @@ public class MetaUtilsTest {
       MetaUtils.getStorageGroupPathByLevel(new PartialPath("root1.laptop.d1.s1"), level);
     } catch (MetadataException e) {
       caughtException = true;
-      assertEquals("root1.laptop.d1.s1 is not a legal path", e.getMessage());
+      assertEquals(
+          "root1.laptop.d1.s1 is not a legal path, because it does not start with root",
+          e.getMessage());
     }
     assertTrue(caughtException);
 
@@ -137,7 +138,9 @@ public class MetaUtilsTest {
       MetaUtils.getStorageGroupPathByLevel(new PartialPath("root"), level);
     } catch (MetadataException e) {
       caughtException = true;
-      assertEquals("root is not a legal path", e.getMessage());
+      assertEquals(
+          "root is not a legal path, because it is no longer than default sg level: 1",
+          e.getMessage());
     }
     assertTrue(caughtException);
   }
diff --git a/iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template b/iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template
index 0865aa511a2..b190633c371 100644
--- a/iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template
+++ b/iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template
@@ -855,7 +855,8 @@ enable_auto_create_schema=true
 # Datatype: int
 # Range: [1, Integer.MAX_VALUE]
 # Default: 1
-# When illegal: report an error and change nothing
+# When illegal: report an error and change nothing during hot-reload; use the default value in
+# start-up
 default_storage_group_level=1
 
 # ALL data types: BOOLEAN, INT32, INT64, FLOAT, DOUBLE, TEXT


(iotdb) 01/03: Fix setting illegal default_storage_group_level does not report an error

Posted by ji...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jiangtian pushed a commit to branch fix_set_default_sg_level_13
in repository https://gitbox.apache.org/repos/asf/iotdb.git

commit b0ef9d262e593b6d6658fda417258e5ae8ed02dc
Author: Tian Jiang <jt...@163.com>
AuthorDate: Wed Dec 18 12:06:59 2024 +0800

    Fix setting illegal default_storage_group_level does not report an error
    
    (cherry picked from commit 391d153759086ff947e238b196021e71aafbce1b)
---
 .../iotdb/db/it/IoTDBSetConfigurationIT.java       | 35 ++++++++++++++++++++++
 .../java/org/apache/iotdb/db/conf/IoTDBConfig.java |  5 ++++
 .../exception/sql/StatementAnalyzeException.java   |  4 +++
 .../analyze/cache/partition/PartitionCache.java    |  2 +-
 .../schemaengine/schemaregion/utils/MetaUtils.java |  9 ++++--
 .../conf/iotdb-system.properties.template          |  4 +++
 6 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java b/integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java
index da6db7a12a8..289dc895f88 100644
--- a/integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java
+++ b/integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java
@@ -39,11 +39,16 @@ import java.io.IOException;
 import java.nio.file.Files;
 import java.sql.Connection;
 import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.Arrays;
 import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 @RunWith(IoTDBTestRunner.class)
 @Category({LocalStandaloneIT.class})
 public class IoTDBSetConfigurationIT {
@@ -202,4 +207,34 @@ public class IoTDBSetConfigurationIT {
             + CommonConfig.SYSTEM_CONFIG_NAME;
     return new File(systemPropertiesPath);
   }
+
+  @Test
+  public void testSetDefaultSGLevel() throws SQLException {
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+      // legal value
+      statement.execute("set configuration \"default_storage_group_level\"=\"3\"");
+      statement.execute("INSERT INTO root.a.b.c.d1(timestamp, s1) VALUES (1, 1)");
+      ResultSet databases = statement.executeQuery("show databases");
+      databases.next();
+      Assert.assertEquals("root.a.b.c", databases.getString(1));
+      assertFalse(databases.next());
+
+      // path too short
+      try {
+        statement.execute("INSERT INTO root.fail(timestamp, s1) VALUES (1, 1)");
+      } catch (SQLException e) {
+        assertEquals(
+            "509: root.fail is not a legal path, because it is no longer than default sg level: 3",
+            e.getMessage());
+      }
+
+      // illegal value
+      try {
+        statement.execute("set configuration \"default_storage_group_level\"=\"-1\"");
+      } catch (SQLException e) {
+        assertTrue(e.getMessage().contains("Illegal defaultStorageGroupLevel: -1, should >= 1"));
+      }
+    }
+  }
 }
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java
index 577bfa25e12..738833bc515 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java
@@ -2434,6 +2434,11 @@ public class IoTDBConfig {
   }
 
   void setDefaultStorageGroupLevel(int defaultStorageGroupLevel) {
+    if (defaultStorageGroupLevel < 1) {
+      throw new IllegalArgumentException(
+          String.format(
+              "Illegal defaultStorageGroupLevel: %d, should >= 1", defaultStorageGroupLevel));
+    }
     this.defaultStorageGroupLevel = defaultStorageGroupLevel;
   }
 
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/exception/sql/StatementAnalyzeException.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/exception/sql/StatementAnalyzeException.java
index 3ca1e397c37..219d73b1b7c 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/exception/sql/StatementAnalyzeException.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/exception/sql/StatementAnalyzeException.java
@@ -28,4 +28,8 @@ public class StatementAnalyzeException extends RuntimeException {
   public StatementAnalyzeException(Exception cause) {
     super(cause);
   }
+
+  public StatementAnalyzeException(String message, Throwable cause) {
+    super(message, cause);
+  }
 }
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/cache/partition/PartitionCache.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/cache/partition/PartitionCache.java
index 56934931b8c..91785849e76 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/cache/partition/PartitionCache.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/cache/partition/PartitionCache.java
@@ -384,7 +384,7 @@ public class PartitionCache {
         }
       } catch (TException | MetadataException | ClientManagerException e) {
         throw new StatementAnalyzeException(
-            "An error occurred when executing getDeviceToStorageGroup():" + e.getMessage());
+            "An error occurred when executing getDeviceToDatabase():" + e.getMessage(), e);
       }
     }
   }
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/utils/MetaUtils.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/utils/MetaUtils.java
index 67fbb6550d7..108b5f14d9d 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/utils/MetaUtils.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/utils/MetaUtils.java
@@ -61,8 +61,13 @@ public class MetaUtils {
   public static PartialPath getStorageGroupPathByLevel(PartialPath path, int level)
       throws MetadataException {
     String[] nodeNames = path.getNodes();
-    if (nodeNames.length <= level || !nodeNames[0].equals(IoTDBConstant.PATH_ROOT)) {
-      throw new IllegalPathException(path.getFullPath());
+    if (nodeNames.length <= level) {
+      throw new IllegalPathException(
+          path.getFullPath(), "it is no longer than default sg level: " + level);
+    }
+    if (!nodeNames[0].equals(IoTDBConstant.PATH_ROOT)) {
+      throw new IllegalPathException(
+          path.getFullPath(), "it does not start with " + IoTDBConstant.PATH_ROOT);
     }
     String[] storageGroupNodes = new String[level + 1];
     System.arraycopy(nodeNames, 0, storageGroupNodes, 0, level + 1);
diff --git a/iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template b/iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template
index d6ca096864f..0865aa511a2 100644
--- a/iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template
+++ b/iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template
@@ -850,8 +850,12 @@ enable_auto_create_schema=true
 # Database level when creating schema automatically is enabled
 # e.g. root.sg0.d1.s2
 #      we will set root.sg0 as the database if database level is 1
+# If the incoming path is shorter than this value, the creation/insertion will fail.
 # effectiveMode: hot_reload
 # Datatype: int
+# Range: [1, Integer.MAX_VALUE]
+# Default: 1
+# When illegal: report an error and change nothing
 default_storage_group_level=1
 
 # ALL data types: BOOLEAN, INT32, INT64, FLOAT, DOUBLE, TEXT


(iotdb) 02/03: spotless

Posted by ji...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jiangtian pushed a commit to branch fix_set_default_sg_level_13
in repository https://gitbox.apache.org/repos/asf/iotdb.git

commit 67157688ae6c02664dc82fd3f15503c5a957d430
Author: Tian Jiang <jt...@163.com>
AuthorDate: Wed Dec 18 13:13:40 2024 +0800

    spotless