Change default key column size and make it configurable for MySQL and Oracle #2245

jnmt · 2024-09-20T05:19:07Z

Description

This PR changes the default key column size and makes it configurable for MySQL and Oracle.

Background:
Currently, the default key column size for MySQL and Oracle is 64 bytes due to the underlying database limitation. It is sometimes short for certain applications. For example, ScalarDL uses the binary name of the class to identify a contract, and it often exceeds 64 bytes.

Solution:
We change the default value to 128 bytes. In addition, we make it configurable because the underlying databases limit the total size of primary key columns; i.e., the larger default key column size affects the total number of primary key columns. So, users may want to create a primary key with a large number of small key columns size.

Related issues and/or PRs

N/A

Changes made

Add options for changing the key column size for MySQL and Oracle and use 128 bytes as the default

Checklist

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes.
Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
Tests (unit, integration, etc.) have been added for the changes.
My changes generate no new warnings.
Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

This change is backward-compatible since it doesn't affect existing tables.

Release notes

Added options for changing the key column size for MySQL and Oracle and used 128 bytes as the default.

… Oracle

jnmt · 2024-09-20T07:58:35Z

core/src/main/java/com/scalar/db/storage/jdbc/JdbcConfig.java

+  public static final String MYSQL_VARIABLE_KEY_COLUMN_SIZE =
+      PREFIX + "mysql.variable_key_column_size";
+  public static final String ORACLE_VARIABLE_KEY_COLUMN_SIZE =
+      PREFIX + "oracle.variable_key_column_size";


I introduced two separate settings (only) for MySQL and Oracle for the following reasons.

PostgreSQL also has a limit of 10485760 for each key column (but not for total). So, users might be confused if the common setting jdbc.variable_key_column_size does not affect it.

We can also make the limit of PostgreSQL configurable, but the common settings would decrease the current limit since 10485760 is too large for MySQL and Oracle.

I guess there is no motivation to decrease 10485760 for PostgreSQL.

What is the reasoning for adding variable in the configuration name?
In essence, it is a configuration, implying that the value is configurable (== variable). Following the same logic, we could have added variable in all the other configuration names.

@Torch3333 Thank you for the question. It's a good point. It doesn't mean it's configurable, but the size of the variable (data-type) key column. I would like to distinguish it from other fixed-size data types like int and double since this option only affects data types (e.g., varchar and varbinary) with variable length. Still weird?

Oh, ok. I completely missed this interpretation. I agree with this reasoning. 👍
Thank you for the explanation.

jnmt · 2024-09-20T08:00:59Z

core/src/main/java/com/scalar/db/storage/jdbc/JdbcDatabase.java

@@ -60,8 +59,7 @@ public JdbcDatabase(DatabaseConfig databaseConfig) {
            databaseConfig.getMetadataCacheExpirationTimeSecs());

    OperationChecker operationChecker = new OperationChecker(databaseConfig, tableMetadataManager);
-    QueryBuilder queryBuilder = new QueryBuilder(rdbEngine);
-    jdbcService = new JdbcService(tableMetadataManager, operationChecker, queryBuilder);
+    jdbcService = new JdbcService(tableMetadataManager, operationChecker, rdbEngine);


Push down RdbEngineStrategy since we don't have to create it again at the deeper level (i.e., ConditionMutator).

…urable

komamitsu · 2024-09-24T02:21:46Z

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java

@@ -201,9 +212,9 @@ public String getDataTypeForEngine(DataType scalarDbDataType) {
  public String getDataTypeForKey(DataType dataType) {
    switch (dataType) {
      case TEXT:
-        return "VARCHAR(64)";
+        return "VARCHAR(" + keyColumnSize + ")";


I guess this column definition is also used for coordinator.state.tx_id column, which usually contains a UUID value or a UUID value plus 25 byte prefix for the group commit feature. So, a validation that confirms the key column size is equal to or larger than 64 would be needed somewhere?

@komamitsu Thank you! That's a really good point. We should check it maybe in JdbcConfig. I will add it.

@komamitsu Revised in b04a069. PTAL!

…urable

komamitsu

LGTM! 👍

brfrn169

LGTM! Thank you!

brfrn169 · 2024-09-24T07:42:34Z

core/src/main/java/com/scalar/db/storage/jdbc/JdbcConfig.java

+  // MySQL and Oracle have limitations regarding the total size of key columns. Thus, we should set
+  // a small but enough key column size so that users can create multiple key columns without
+  // exceeding the limit and changing the default. Since we found the old default size of 64 bytes
+  // was small for some applications, we changed it based on the following specifications. See the
+  // official documents for details.
+  // 1) In MySQL, the maximum total size of key columns is 3072 bytes in the default, and thus,
+  // depending on the charset, it can be up to 768 characters long. It can further be reduced if the
+  // different settings are used.
+  // 2) In Oracle, the maximum total size of key columns is approximately 75% of the database block
+  // size minus some overhead. The default block size is 8KB, and it is typically 4kB or 8kB. Thus,
+  // the maximum size can be similar to the MySQL.
+  // See the official documents for details.
+  // https://dev.mysql.com/doc/refman/8.0/en/innodb-limits.html
+  // https://docs.oracle.com/en/database/oracle/oracle-database/23/refrn/logical-database-limits.html


brfrn169 · 2024-09-24T07:47:04Z

core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java

+    // The number 10485760 is due to the maximum length of the character column.
+    // https://www.postgresql.org/docs/15/datatype-character.html


feeblefakie

Left one suggestion.
Other than that, LGTM! Thank you!

feeblefakie · 2024-09-26T03:04:02Z

core/src/main/java/com/scalar/db/storage/jdbc/JdbcConfig.java

+            ORACLE_VARIABLE_KEY_COLUMN_SIZE,
+            DEFAULT_VARIABLE_KEY_COLUMN_SIZE);
+
+    if (mysqlVariableKeyColumnSize < 64 || oracleVariableKeyColumnSize < 64) {


I think it's probably better to make 64 a static variable like `MINIMUM_VARIABLE_KEY_COLUMN_SIZE.
We should probably note why 64.

@feeblefakie Definitely, thank you! I updated it with the reason based on Mitsu's comment in 1072e98. PTAL, just in case.

Torch3333

LGTM, thank you!

…urable

jnmt · 2024-09-26T06:45:20Z

@josh-wong Can you take a look at the message part? I forgot to add you as a reviewer when revising this PR to add the error message.

feeblefakie

LGTM! Thank you!

josh-wong

LGTM! Thank you!🙇🏻‍♂️

… Oracle (#2245)

jnmt added 2 commits September 20, 2024 14:17

Change default key column size and make it configurable for MySQL and…

7244a11

… Oracle

Add comments

16e2aef

jnmt commented Sep 20, 2024

View reviewed changes

jnmt self-assigned this Sep 20, 2024

jnmt added the improvement label Sep 20, 2024

jnmt requested review from komamitsu, brfrn169, feeblefakie and Torch3333 September 20, 2024 09:04

Merge branch 'master' into change-default-key-columns-size-and-config…

d2ff39b

…urable

komamitsu reviewed Sep 24, 2024

View reviewed changes

jnmt added 2 commits September 24, 2024 16:44

Merge branch 'master' into change-default-key-columns-size-and-config…

b25bc82

…urable

Add column size validation

b04a069

jnmt requested a review from komamitsu September 24, 2024 08:33

komamitsu approved these changes Sep 24, 2024

View reviewed changes

brfrn169 approved these changes Sep 24, 2024

View reviewed changes

feeblefakie approved these changes Sep 26, 2024

View reviewed changes

Torch3333 approved these changes Sep 26, 2024

View reviewed changes

jnmt and others added 2 commits September 26, 2024 15:38

Revise based on feedback

1072e98

Merge branch 'master' into change-default-key-columns-size-and-config…

5988dfc

…urable

jnmt requested review from josh-wong and feeblefakie September 26, 2024 06:40

feeblefakie approved these changes Sep 27, 2024

View reviewed changes

josh-wong approved these changes Sep 30, 2024

View reviewed changes

jnmt merged commit a4b2cf9 into master Sep 30, 2024
46 checks passed

jnmt deleted the change-default-key-columns-size-and-configurable branch September 30, 2024 08:28

feeblefakie mentioned this pull request Sep 30, 2024

Backport to branch(3) : Change default key column size and make it configurable for MySQL and Oracle #2266

Merged

jnmt added a commit that referenced this pull request Sep 30, 2024

Change default key column size and make it configurable for MySQL and…

3e27612

… Oracle (#2245)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change default key column size and make it configurable for MySQL and Oracle #2245

Change default key column size and make it configurable for MySQL and Oracle #2245

jnmt commented Sep 20, 2024 •

edited

Loading

jnmt Sep 20, 2024

Torch3333 Sep 25, 2024 •

edited

Loading

jnmt Sep 25, 2024

Torch3333 Sep 26, 2024 •

edited

Loading

jnmt Sep 20, 2024

komamitsu Sep 24, 2024

jnmt Sep 24, 2024

jnmt Sep 24, 2024

komamitsu left a comment

brfrn169 left a comment

brfrn169 Sep 24, 2024

brfrn169 Sep 24, 2024

feeblefakie left a comment

feeblefakie Sep 26, 2024

jnmt Sep 26, 2024 •

edited

Loading

Torch3333 left a comment

jnmt commented Sep 26, 2024

feeblefakie left a comment

josh-wong left a comment

		// The number 10485760 is due to the maximum length of the character column.
		// https://www.postgresql.org/docs/15/datatype-character.html

Change default key column size and make it configurable for MySQL and Oracle #2245

Change default key column size and make it configurable for MySQL and Oracle #2245

Conversation

jnmt commented Sep 20, 2024 • edited Loading

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release notes

Choose a reason for hiding this comment

Torch3333 Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Torch3333 Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

komamitsu left a comment

Choose a reason for hiding this comment

brfrn169 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feeblefakie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnmt Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Torch3333 left a comment

Choose a reason for hiding this comment

jnmt commented Sep 26, 2024

feeblefakie left a comment

Choose a reason for hiding this comment

josh-wong left a comment

Choose a reason for hiding this comment

jnmt commented Sep 20, 2024 •

edited

Loading

Torch3333 Sep 25, 2024 •

edited

Loading

Torch3333 Sep 26, 2024 •

edited

Loading

jnmt Sep 26, 2024 •

edited

Loading