[admin] Add create_time to database and table metadata. #289

loserwang1024 · 2024-12-30T10:10:30Z

Purpose

Linked issue: close #281

fluss-client/src/test/java/com/alibaba/fluss/client/admin/FlussAdminITCase.java

fluss-common/src/main/java/com/alibaba/fluss/metadata/DatabaseInfo.java

fluss-common/src/main/java/com/alibaba/fluss/metadata/TableInfo.java

...onnector-flink/src/test/java/com/alibaba/fluss/connector/flink/catalog/FlinkCatalogTest.java

fluss-server/src/main/java/com/alibaba/fluss/server/zk/data/TableRegistration.java

fluss-server/src/main/java/com/alibaba/fluss/server/coordinator/MetaDataManager.java

wuchong · 2025-01-06T16:47:20Z

fluss-server/src/main/java/com/alibaba/fluss/server/zk/ZooKeeperClient.java

+    /** Get the database in ZK. */
+    public Optional<DatabaseRegistration> getDatabase(String database) throws Exception {
+        Optional<byte[]> bytes = getOrEmpty(DatabaseZNode.path(database));
+        return bytes.map(DatabaseZNode::decode);


we should add a unit test in ZooKeeperClientTest that getDatabase from a legacy node. this may throw a json parse exception.

we can try catch the parse exception and return a default DatabaseRegistration with the created_time and modified_time extracted from `zkClient.checkExists().forPath(path).getCtime()/.getMtime()``.

add the same unit test for getTable as well.

decode legacy table node will not throw parse exception but with -1 time, we can check if it is -1 time, and get the created_time and modified_time extracted from zkClient.checkExists().forPath(path).getCtime()/.getMtime().

we can try catch the parse exception and return a default DatabaseRegistration with the created_time and modified_time extracted from `zkClient.checkExists().forPath(path).getCtime()/.getMtime()``.

I have considered it because it will be more easier, but the meta data will be bounded to zookeeper. It maybe easy now, but will be tricky if we remove the zookeeper later. @wuchong , WDYT?

This is a backward-compatibility, in the past, we only support zookeeper metastore. The standard support of created_time/modified_time is your current implementation (adding additional properties in serialized data).

fluss-server/src/main/java/com/alibaba/fluss/server/zk/data/DatabaseRegistrationJsonSerde.java

fluss-client/src/main/java/com/alibaba/fluss/client/admin/Admin.java

fluss-common/src/main/java/com/alibaba/fluss/metadata/DatabaseDescriptor.java

luoyuxia · 2025-01-07T09:28:40Z

fluss-common/src/main/java/com/alibaba/fluss/metadata/DatabaseDescriptor.java

+         *
+         * <p>Table properties are controlled by Fluss and will change the behavior of the table.
+         */
+        public <T> Builder property(ConfigOption<T> configOption, T value) {


I think this method can be removed since we don't have any pre-defined ConfigOption for database...

Ok, I will modify property as customProperty which Jark Wu has addvised.

fluss-common/src/main/java/com/alibaba/fluss/metadata/DatabaseDescriptor.java

fluss-common/src/main/java/com/alibaba/fluss/metadata/DatabaseInfo.java

fluss-server/src/main/java/com/alibaba/fluss/server/zk/data/DatabaseRegistration.java

loserwang1024 · 2025-01-15T02:41:33Z

@wuchong @luoyuxia , CC

wuchong · 2025-01-15T15:16:47Z

#312 is introducing a compatibility breaking change which requires all the tables to be re-created. So v0.6 is not a compatible version.

How about making modified_time and created_time required in this PR to simplify the compatible logic? Currently, many code still create TableInfo with -1 createdTime and modifiedTime which is not correct and may introduce future bugs. If they are required, we can fix them together.

wuchong · 2025-01-18T13:43:07Z

Please do not squash commits even if it is a big change.

wuchong · 2025-01-18T13:56:39Z

fluss-common/src/main/java/com/alibaba/fluss/metadata/TableInfo.java

@@ -79,20 +96,18 @@ public boolean equals(Object o) {

    @Override
    public int hashCode() {
-        return Objects.hash(tablePath, tableDescriptor, tableId, schemaId);
+        return Objects.hash(tablePath, tableDescriptor, tableId, schemaId, createdTime, modifiedTime);


remove createdTime, modifiedTime from hashCode() if they are not compared in equals().

wuchong · 2025-01-18T13:58:35Z

fluss-client/src/main/java/com/alibaba/fluss/client/admin/FlussAdmin.java

-                                        r.getSchemaId()));
+                                        r.getSchemaId(),
+                                        r.hasCreatedTime() ? r.getCreatedTime() : -1,
+                                        r.hasModifiedTime() ? r.getModifiedTime() : -1));


they are required fields now, no need to check hasXxx()

wuchong · 2025-01-18T13:59:29Z

fluss-server/src/main/java/com/alibaba/fluss/server/zk/data/TableRegistration.java

-                tableDescriptor.getCustomProperties());
+                tableDescriptor.getCustomProperties(),
+                System.currentTimeMillis(),
+                System.currentTimeMillis());


We should use a same time for them when creation.

wuchong · 2025-01-18T14:09:39Z

I pushed a commit to fix the problems.

loserwang1024 force-pushed the create_time branch from 4950011 to caf5158 Compare December 30, 2024 12:40

loserwang1024 requested review from luoyuxia, swuferhong and wuchong December 31, 2024 06:18

wuchong reviewed Jan 6, 2025

View reviewed changes

luoyuxia reviewed Jan 7, 2025

View reviewed changes

loserwang1024 force-pushed the create_time branch 2 times, most recently from 033073b to 2c01767 Compare January 9, 2025 12:09

loserwang1024 requested review from wuchong and luoyuxia January 9, 2025 12:11

loserwang1024 added 2 commits January 17, 2025 17:53

[admin] Add create_time to database and table metadata.

8ef21c0

[admin] Add create_time to database and table metadata.

c465753

loserwang1024 force-pushed the create_time branch from d3287af to c465753 Compare January 18, 2025 06:45

address problems

920457d

wuchong reviewed Jan 18, 2025

View reviewed changes

wuchong merged commit 290d4a6 into alibaba:main Jan 18, 2025
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[admin] Add create_time to database and table metadata. #289

[admin] Add create_time to database and table metadata. #289

loserwang1024 commented Dec 30, 2024

wuchong Jan 6, 2025

loserwang1024 Jan 7, 2025 •

edited

Loading

wuchong Jan 7, 2025

luoyuxia Jan 7, 2025

loserwang1024 Jan 9, 2025

loserwang1024 commented Jan 15, 2025

wuchong commented Jan 15, 2025

wuchong commented Jan 18, 2025

wuchong Jan 18, 2025

wuchong Jan 18, 2025

wuchong Jan 18, 2025

wuchong commented Jan 18, 2025

[admin] Add create_time to database and table metadata. #289

[admin] Add create_time to database and table metadata. #289

Conversation

loserwang1024 commented Dec 30, 2024

Purpose

wuchong Jan 6, 2025

Choose a reason for hiding this comment

loserwang1024 Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

wuchong Jan 7, 2025

Choose a reason for hiding this comment

luoyuxia Jan 7, 2025

Choose a reason for hiding this comment

loserwang1024 Jan 9, 2025

Choose a reason for hiding this comment

loserwang1024 commented Jan 15, 2025

wuchong commented Jan 15, 2025

wuchong commented Jan 18, 2025

wuchong Jan 18, 2025

Choose a reason for hiding this comment

wuchong Jan 18, 2025

Choose a reason for hiding this comment

wuchong Jan 18, 2025

Choose a reason for hiding this comment

wuchong commented Jan 18, 2025

loserwang1024 Jan 7, 2025 •

edited

Loading