[#6467] improve(core): Allow JDBC backend configs be configurable #6469

TEOTEO520 · 2025-02-17T14:49:07Z

What changes were proposed in this pull request?

Add two configurations for the Gravitino server: gravitino.entity.store.relational.maxConnections and gravitino.entity.store.relational.maxWaitMillis to control the connection pool when connecting to the JDBC storage backend.
Add docs about these configurations accordingly.

Why are the changes needed?

To support users to configure JDBC connection pool parameters

Fix: #6467

Does this PR introduce any user-facing change?

No

How was this patch tested?

No need to test

jerryshao · 2025-02-18T02:02:18Z

@yuqi1129 can you please help to review?

core/src/main/java/org/apache/gravitino/Configs.java

TEOTEO520 · 2025-02-23T20:18:31Z

All suggestions have been incorporated. Thank you for your thorough review!

tengqm · 2025-02-24T00:56:00Z

lgtm

core/src/main/java/org/apache/gravitino/Configs.java

yuqi1129 · 2025-02-24T02:48:04Z

@TEOTEO520
Please address the comments and try to determine the reason for the CI 's failure.

jerryshao · 2025-02-25T06:41:45Z

core/src/main/java/org/apache/gravitino/Configs.java

+  public static final String ENTITY_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION_KEY =
+      "gravitino.entity.store.relational.max_connections";
+
+  public static final String ENTITY_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLIS_CONNECTION_KEY =
+      "gravitino.entity.store.relational.max_wait_millis";


Please be aware that the configuration should be camel-style.

OK, thank you for reviewing

tengqm · 2025-02-26T00:36:21Z

core/src/main/java/org/apache/gravitino/Configs.java

@@ -137,6 +147,21 @@ private Configs() {}
          .stringConf()
          .createWithDefault(DEFAULT_RELATIONAL_JDBC_BACKEND_PASSWORD);

+  public static final ConfigEntry<Integer> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION =


Suggested change

public static final ConfigEntry<Integer> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION =

public static final ConfigEntry<Integer> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_CONNECTIONS =

tengqm · 2025-02-26T00:36:35Z

core/src/main/java/org/apache/gravitino/Configs.java

@@ -137,6 +147,21 @@ private Configs() {}
          .stringConf()
          .createWithDefault(DEFAULT_RELATIONAL_JDBC_BACKEND_PASSWORD);

+  public static final ConfigEntry<Integer> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION =
+      new ConfigBuilder(ENTITY_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION_KEY)


Suggested change

new ConfigBuilder(ENTITY_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION_KEY)

new ConfigBuilder(ENTITY_RELATIONAL_JDBC_BACKEND_MAX_CONNECTION_KEYS)

tengqm · 2025-02-26T00:38:02Z

core/src/main/java/org/apache/gravitino/Configs.java

+          .intConf()
+          .createWithDefault(DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION);
+
+  public static final ConfigEntry<Long> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLIS_CONNECTION =


Suggested change

public static final ConfigEntry<Long> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLIS_CONNECTION =

public static final ConfigEntry<Long> ENTITY_RELATIONAL_JDBC_BACKEND_WAIT_MILLISECONDS =

tengqm · 2025-02-26T00:38:24Z

core/src/main/java/org/apache/gravitino/Configs.java

@@ -84,6 +90,10 @@ private Configs() {}

  public static final String DEFAULT_RELATIONAL_JDBC_BACKEND_PASSWORD = "gravitino";

+  public static final int DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION = 100;


Suggested change

public static final int DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION = 100;

public static final int DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_CONNECTIONS = 100;

tengqm · 2025-02-26T00:38:45Z

core/src/main/java/org/apache/gravitino/Configs.java

@@ -84,6 +90,10 @@ private Configs() {}

  public static final String DEFAULT_RELATIONAL_JDBC_BACKEND_PASSWORD = "gravitino";

+  public static final int DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION = 100;
+
+  public static final long DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLIS_CONNECTION = 1000L;


Suggested change

public static final long DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLIS_CONNECTION = 1000L;

public static final long DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLISECONDS = 1000L;

All of your suggestions have been adopted, thank you very much

tengqm · 2025-02-26T22:52:56Z

lgtm

jerqi · 2025-02-27T03:00:22Z

integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/BaseIT.java

@@ -381,6 +381,7 @@ protected String readGitCommitIdFromGitFile() {
      String gravitinoHome = System.getenv("GRAVITINO_HOME");
      String gitFolder = gravitinoHome + File.separator + ".git" + File.separator;
      String headFileContent = FileUtils.readFileToString(new File(gitFolder + "HEAD"), "UTF-8");
+      LOG.error("\n\n\noooooooooooooooooooooooooooooooooooooooooooooooooooooo" + headFileContent);


Could remove this line?

jerryshao · 2025-03-03T09:36:58Z

I think you should also update the related documents about newly added configurations.

TEOTEO520 · 2025-03-08T15:45:48Z

I think you should also update the related documents about newly added configurations.

done

yuqi1129 · 2025-03-11T09:11:01Z

@jerqi @jerryshao @tengqm
Do you have any further comments on this PR?

jerqi

just a minor.

jerqi · 2025-03-11T09:22:35Z

docs/gravitino-server-config.md

+| `gravitino.entity.store.relational.jdbcPassword`  | The password that the `JDBCBackend` needs to use when connecting the database. It is required for `MySQL`.                                                                                                                                              | `gravitino`                       | Yes if the jdbc connection url is not `jdbc:h2` | 0.5.0            |
+| `gravitino.entity.store.relational.storagePath`   | The storage path for embedded JDBC storage implementation. It supports both absolute and relative path, if the value is a relative path, the final path is `${GRAVITINO_HOME}/${PATH_YOU_HAVA_SET}`, default value is `${GRAVITINO_HOME}/data/jdbc`     | `${GRAVITINO_HOME}/data/jdbc`     | No                                              | 0.6.0-incubating |
+| `gravitino.entity.store.relational.maxConnections`| The maximum number of connections for the JDBC Backend connection pool                                                                                                                                                                                  | `100`                             | No                                              | 0.9.0            |
+| `gravitino.entity.store.relational.maxWaitMillis` | The maximum wait time in milliseconds for a connection from the JDBC Backend connection pool                                                                                                                                                            | `1000`                            | No                                              | 0.9.0            |


0.9.0-incubting.

done thank u

jerqi · 2025-03-12T02:21:26Z

You should modify the PR description to align to the latest code.

TEOTEO520 force-pushed the feature/support-jdbc-configrable branch from 0b2769e to 3b9872f Compare February 17, 2025 14:50

jerryshao requested a review from yuqi1129 February 18, 2025 02:02

yuqi1129 reviewed Feb 18, 2025

View reviewed changes

yuqi1129 reviewed Feb 24, 2025

View reviewed changes

core/src/main/java/org/apache/gravitino/Configs.java Outdated Show resolved Hide resolved

TEOTEO520 force-pushed the feature/support-jdbc-configrable branch from 974b2fc to 727d369 Compare February 24, 2025 14:34

jerryshao reviewed Feb 25, 2025

View reviewed changes

tengqm reviewed Feb 26, 2025

View reviewed changes

jerqi reviewed Feb 27, 2025

View reviewed changes

TEOTEO520 force-pushed the feature/support-jdbc-configrable branch from b61add5 to 9aa62a3 Compare March 8, 2025 15:45

jerqi reviewed Mar 11, 2025

View reviewed changes

teo and others added 10 commits March 12, 2025 04:08

[apache#6467] improve(core): Allow JDBC backend configs be configurable

40a22ad

improve config names

67f0b77

fix ci test

5f35b9b

add mock conf before for ci

81e23f4

add mock metricsSystem before init sql session config

c8f9937

make configuration to be camel-style

ed2f2b8

improve conf name

a17e342

remove unnecessary line

7462f5a

add docs for config

83382f5

add docs for config

b9c1461

TEOTEO520 force-pushed the feature/support-jdbc-configrable branch from 9aa62a3 to b9c1461 Compare March 11, 2025 20:09

yuqi1129 approved these changes Mar 12, 2025

View reviewed changes

jerqi approved these changes Mar 12, 2025

View reviewed changes

yuqi1129 assigned TEOTEO520 Mar 13, 2025

yuqi1129 merged commit 683ce0c into apache:main Mar 13, 2025
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#6467] improve(core): Allow JDBC backend configs be configurable #6469

[#6467] improve(core): Allow JDBC backend configs be configurable #6469

TEOTEO520 commented Feb 17, 2025 •

edited by yuqi1129

Loading

jerryshao commented Feb 18, 2025

TEOTEO520 commented Feb 23, 2025

tengqm commented Feb 24, 2025

yuqi1129 commented Feb 24, 2025

jerryshao Feb 25, 2025

TEOTEO520 Feb 25, 2025

tengqm Feb 26, 2025

tengqm Feb 26, 2025

tengqm Feb 26, 2025

tengqm Feb 26, 2025

tengqm Feb 26, 2025

TEOTEO520 Feb 26, 2025

tengqm commented Feb 26, 2025

jerqi Feb 27, 2025

TEOTEO520 Mar 2, 2025

jerryshao commented Mar 3, 2025

TEOTEO520 commented Mar 8, 2025

yuqi1129 commented Mar 11, 2025

jerqi left a comment

jerqi Mar 11, 2025

TEOTEO520 Mar 11, 2025

jerqi commented Mar 12, 2025

	public static final ConfigEntry<Integer> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION =
	public static final ConfigEntry<Integer> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_CONNECTIONS =

	new ConfigBuilder(ENTITY_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION_KEY)
	new ConfigBuilder(ENTITY_RELATIONAL_JDBC_BACKEND_MAX_CONNECTION_KEYS)

	public static final ConfigEntry<Long> ENTITY_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLIS_CONNECTION =
	public static final ConfigEntry<Long> ENTITY_RELATIONAL_JDBC_BACKEND_WAIT_MILLISECONDS =

		@@ -84,6 +90,10 @@ private Configs() {}

		public static final String DEFAULT_RELATIONAL_JDBC_BACKEND_PASSWORD = "gravitino";

		public static final int DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_TOTAL_CONNECTION = 100;

	public static final long DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLIS_CONNECTION = 1000L;
	public static final long DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLISECONDS = 1000L;

[#6467] improve(core): Allow JDBC backend configs be configurable #6469

[#6467] improve(core): Allow JDBC backend configs be configurable #6469

Conversation

TEOTEO520 commented Feb 17, 2025 • edited by yuqi1129 Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

jerryshao commented Feb 18, 2025

TEOTEO520 commented Feb 23, 2025

tengqm commented Feb 24, 2025

yuqi1129 commented Feb 24, 2025

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tengqm commented Feb 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryshao commented Mar 3, 2025

TEOTEO520 commented Mar 8, 2025

yuqi1129 commented Mar 11, 2025

jerqi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerqi commented Mar 12, 2025

TEOTEO520 commented Feb 17, 2025 •

edited by yuqi1129

Loading