[DSIP-56] Refactor JDBC registry support session timeout and data change event #16287

ruanwenjun · 2024-07-08T06:44:51Z

Purpose of the pull request

close #16278

Brief change log

Refactor the JDBC Registry
Change default registry of standalone to jdbc

Verify this pull request

Test by UT and E2E

Pull Request Notice

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

...g/apache/dolphinscheduler/plugin/registry/jdbc/model/DTO/JdbcRegistryClientHeartbeatDTO.java

...in/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryDataManager.java

...rc/main/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryServer.java

...in/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryLockManager.java

+                            .build());
+                    return;
+                }
+                log.debug("{} acquire the lock {} success", lockOwner, lockKey);


...in/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryLockManager.java

+                            .build());
+                    return true;
+                }
+                log.debug("{} acquire the lock {} success", lockOwner, lockKey);


...g/apache/dolphinscheduler/plugin/registry/jdbc/model/DTO/JdbcRegistryClientHeartbeatDTO.java

dolphinscheduler-tools/src/main/java/org/apache/dolphinscheduler/tools/command/ICommand.java

...c/src/main/java/org/apache/dolphinscheduler/plugin/registry/jdbc/JdbcRegistryProperties.java

+                        "\n  sessionTimeout -> " + sessionTimeout +
+                        "\n  hikariConfig -> " + hikariConfig +
+                        "\n****************************JdbcRegistryProperties**************************************";
+        log.info(config);


...eduler-tools/src/main/java/org/apache/dolphinscheduler/tools/command/CommandApplication.java

SbloodyS

LGTM

.github/workflows/cluster-test/postgresql_with_zookeeper_registry/dolphinscheduler_env.sh

caishunfeng · 2024-07-11T07:37:54Z

dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql

+  DEFAULT CHARSET = utf8;
+
+DROP TABLE IF EXISTS `t_ds_jdbc_registry_client_heartbeat`;
+CREATE TABLE `t_ds_jdbc_registry_client_heartbeat`


Heartbeat is just an attribute of client, so maybe registry_client is better.

Suggested change

CREATE TABLE `t_ds_jdbc_registry_client_heartbeat`

CREATE TABLE `t_ds_jdbc_registry_client`

The origin table name is t_ds_jdbc_registry_client, but I am worried this will conflict with JdbcRegistryClient which is already exist.

I think it's OK because it has t_ds prefix, and what about using DsJdbcRegistryClient as entity class name?

dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql

caishunfeng · 2024-07-11T07:40:31Z

dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql

+(
+    `id`                 bigint(11) NOT NULL AUTO_INCREMENT COMMENT 'primary key',
+    `event_type`         varchar(64) NOT NULL COMMENT 'ADD, UPDATE, DELETE',
+    `jdbc_registry_data` text        NOT NULL COMMENT 'jdbc registry data',


Suggested change

`jdbc_registry_data` text NOT NULL COMMENT 'jdbc registry data',

`registry_data` text NOT NULL COMMENT 'registry data',

Use jdbc_registry_data may better to know the data is from t_ds_jdbc_registry_data

caishunfeng · 2024-07-11T07:45:32Z

...g/apache/dolphinscheduler/plugin/registry/jdbc/mapper/JdbcRegistryDataChanceEventMapper.java

+    @Select("select max(id) from t_ds_jdbc_registry_data_change_event")
+    Long getMaxId();
+
+    @Select("select * from t_ds_jdbc_registry_data_change_event where id > #{id} order by id asc limit 1000")


Suggested change

@Select("select * from t_ds_jdbc_registry_data_change_event where id > #{id} order by id asc limit 1000")

@Select("select * from t_ds_jdbc_registry_data_change_event where id > #{id} limit 1000")

Directly announce the order is better to let others know we need to make sure the result have order here.

caishunfeng · 2024-07-11T07:51:29Z

...java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/IRegistryRowChangeNotifier.java

+
+    void subscribeRegistryRowChange(RegistryRowChangeListener<T> registryRowChangeListener);
+
+    interface RegistryRowChangeListener<T> {


RegistryRow means RegistryData?

row is a database concept, and it is best to use logical concepts.

This is not only represent to RegistryData, but now only have RegistryData implemenration, this interface is used to notify the jdbc row add/update/delete, so we can directly use jdbc concepts here.

...in/java/org/apache/dolphinscheduler/plugin/registry/jdbc/server/JdbcRegistryServerState.java

...istry-jdbc/src/test/java/org/apache/dolphinscheduler/plugin/registry/jdbc/LockUtilsTest.java

...st/java/org/apache/dolphinscheduler/plugin/registry/jdbc/PostgresqlJdbcRegistryTestCase.java

caishunfeng · 2024-07-12T01:49:53Z

Should update the doc too? see https://dolphinscheduler.apache.org/zh-cn/docs/3.2.1/architecture/configuration

ruanwenjun · 2024-07-12T05:35:50Z

Should update the doc too? see https://dolphinscheduler.apache.org/zh-cn/docs/3.2.1/architecture/configuration

The jdbc relation configuration doc is link to jdbc registry README.md which will be updated in this PR.

https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/architecture/configuration.md#registry-related-configuration

https://github.com/apache/dolphinscheduler/blob/dev/dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-jdbc/README.md

sonarcloud · 2024-07-12T05:48:42Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 60%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

caishunfeng

LGTM overall

SbloodyS

+1

ruanwenjun requested review from EricGao888, SbloodyS and Gallardot as code owners July 8, 2024 06:44

ruanwenjun marked this pull request as draft July 8, 2024 06:44

ruanwenjun self-assigned this Jul 8, 2024

github-actions bot added backend test document labels Jul 8, 2024

github-advanced-security bot found potential problems Jul 8, 2024

View reviewed changes

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch 6 times, most recently from 14c7651 to d2ac15a Compare July 8, 2024 12:30

ruanwenjun added the DSIP label Jul 8, 2024

ruanwenjun added this to the 3.3.0 milestone Jul 8, 2024

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch 5 times, most recently from 379dae8 to af80ef4 Compare July 8, 2024 15:06

ruanwenjun marked this pull request as ready for review July 8, 2024 16:29

ruanwenjun requested a review from caishunfeng as a code owner July 8, 2024 16:29

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch 4 times, most recently from 11409b9 to c93008c Compare July 9, 2024 11:31

github-actions bot added the CI&CD label Jul 9, 2024

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch from c93008c to 4106238 Compare July 9, 2024 11:32

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch from 4106238 to caefdd0 Compare July 9, 2024 15:02

github-advanced-security bot found potential problems Jul 9, 2024

View reviewed changes

dolphinscheduler-tools/src/main/java/org/apache/dolphinscheduler/tools/command/ICommand.java Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Jul 10, 2024

View reviewed changes

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch from d53f811 to 1b3c3aa Compare July 10, 2024 04:26

SbloodyS reviewed Jul 10, 2024

View reviewed changes

...eduler-tools/src/main/java/org/apache/dolphinscheduler/tools/command/CommandApplication.java Outdated Show resolved Hide resolved

ruanwenjun requested a review from SbloodyS July 10, 2024 11:47

SbloodyS previously approved these changes Jul 11, 2024

View reviewed changes

caishunfeng reviewed Jul 11, 2024

View reviewed changes

ruanwenjun dismissed SbloodyS’s stale review via c6ead62 July 11, 2024 09:10

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch from ecc3e97 to c6ead62 Compare July 11, 2024 09:10

ruanwenjun requested a review from caishunfeng July 11, 2024 10:33

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch from c6ead62 to f0578f7 Compare July 11, 2024 10:34

ruanwenjun added 3 commits July 12, 2024 13:37

Support session in jdbc registry

bb2c8c0

Do purge the previous jdbc client in db

200b233

Add registry table to initialize sql

99b6ce1

ruanwenjun force-pushed the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch from f0578f7 to 99b6ce1 Compare July 12, 2024 05:38

caishunfeng approved these changes Jul 12, 2024

View reviewed changes

SbloodyS approved these changes Jul 12, 2024

View reviewed changes

ruanwenjun merged commit 2db8098 into apache:dev Jul 12, 2024
66 of 68 checks passed

ruanwenjun deleted the dev_wenjun_supportSessionTimeoutInJdbcRegistry branch July 12, 2024 06:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DSIP-56] Refactor JDBC registry support session timeout and data change event #16287

[DSIP-56] Refactor JDBC registry support session timeout and data change event #16287

ruanwenjun commented Jul 8, 2024 •

edited

Loading

SbloodyS left a comment

caishunfeng Jul 11, 2024

ruanwenjun Jul 11, 2024

caishunfeng Jul 12, 2024

caishunfeng Jul 11, 2024

ruanwenjun Jul 11, 2024

caishunfeng Jul 11, 2024

ruanwenjun Jul 11, 2024

caishunfeng Jul 11, 2024

caishunfeng Jul 11, 2024

ruanwenjun Jul 11, 2024

caishunfeng commented Jul 12, 2024

ruanwenjun commented Jul 12, 2024 •

edited

Loading

sonarcloud bot commented Jul 12, 2024

caishunfeng left a comment

SbloodyS left a comment

	CREATE TABLE `t_ds_jdbc_registry_client_heartbeat`
	CREATE TABLE `t_ds_jdbc_registry_client`

	`jdbc_registry_data` text NOT NULL COMMENT 'jdbc registry data',
	`registry_data` text NOT NULL COMMENT 'registry data',

	@Select("select * from t_ds_jdbc_registry_data_change_event where id > #{id} order by id asc limit 1000")
	@Select("select * from t_ds_jdbc_registry_data_change_event where id > #{id} limit 1000")


		void subscribeRegistryRowChange(RegistryRowChangeListener<T> registryRowChangeListener);

		interface RegistryRowChangeListener<T> {

[DSIP-56] Refactor JDBC registry support session timeout and data change event #16287

[DSIP-56] Refactor JDBC registry support session timeout and data change event #16287

Conversation

ruanwenjun commented Jul 8, 2024 • edited Loading

Purpose of the pull request

Brief change log

Verify this pull request

Pull Request Notice

SbloodyS left a 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

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

caishunfeng commented Jul 12, 2024

ruanwenjun commented Jul 12, 2024 • edited Loading

sonarcloud bot commented Jul 12, 2024

Quality Gate failed

caishunfeng left a comment

Choose a reason for hiding this comment

SbloodyS left a comment

Choose a reason for hiding this comment

ruanwenjun commented Jul 8, 2024 •

edited

Loading

ruanwenjun commented Jul 12, 2024 •

edited

Loading