[lake] Record a file path storing log offsets in lake snapshot property #2223

luoyuxia · 2025-12-22T07:30:36Z

Purpose

Linked issue: close #2224

Brief change log

In TieringCommitOperator, first prepare commit log offsets to fluss cluster which will write a file to store the log offsets
then, record the file path in lake snapshot property, commit the snapshot

Tests

Existing test

API and Format

Documentation

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...ink-common/src/main/java/org/apache/fluss/flink/tiering/committer/TieringCommitOperator.java

fluss-server/src/test/java/org/apache/fluss/server/coordinator/TestCoordinatorGateway.java

fluss-rpc/src/test/java/org/apache/fluss/rpc/TestingTabletGatewayService.java

fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorEventProcessor.java

fluss-server/src/test/java/org/apache/fluss/server/tablet/TestTabletServerGateway.java

.../src/main/java/org/apache/fluss/flink/tiering/committer/FlussTableLakeSnapshotCommitter.java

fluss-common/src/main/java/org/apache/fluss/lake/committer/CommittedLakeSnapshot.java

fluss-rpc/src/main/proto/FlussApi.proto

Copilot

Pull request overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTableJsonSerde.java

.../src/main/java/org/apache/fluss/flink/tiering/committer/FlussTableLakeSnapshotCommitter.java

fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTableHelper.java

Copilot

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...ink-common/src/main/java/org/apache/fluss/flink/tiering/committer/TieringCommitOperator.java

fluss-server/src/main/java/org/apache/fluss/server/entity/CommitLakeTableSnapshotData.java

fluss-server/src/main/java/org/apache/fluss/server/RpcServiceBase.java

fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorEventProcessor.java

...common/src/test/java/org/apache/fluss/flink/tiering/committer/TieringCommitOperatorTest.java

fluss-common/src/test/java/org/apache/fluss/utils/json/TableBucketOffsetsJsonSerdeTest.java

luoyuxia · 2025-12-25T02:29:09Z

@wuchong Could you please help review this pr? The pr also handle the back compabitlity when use v2 to serialize lake table snapshot

wuchong · 2025-12-26T09:08:11Z

fluss-rpc/src/main/proto/FlussApi.proto


-message CommitLakeTableSnapshotRequest {
-  repeated PbLakeTableSnapshotInfo tables_req = 1;
+message PrepareCommitLakeTableSnapshotRequest {


PrepareLakeTableSnapshotRequest

prepare and commit are 2 different phases.

wuchong · 2025-12-26T09:08:31Z

fluss-rpc/src/main/proto/FlussApi.proto

+message PrepareCommitLakeTableSnapshotResponse {
+  repeated PbPrepareCommitLakeTableRespForTable prepare_commit_lake_table_resp = 1;


wuchong · 2025-12-26T09:14:36Z

fluss-rpc/src/main/proto/FlussApi.proto

+  optional int64 max_timestamp = 6;
+}
+
+message PbPrepareCommitLakeTableRespForTable {


add a table_id field, as the PrepareCommitLakeTableSnapshotRequest request has multiple table ids, we need to distinguish which table is the PbPrepareCommitLakeTableRespForTable belong to.

wuchong · 2025-12-26T09:19:56Z

fluss-rpc/src/main/proto/FlussApi.proto

+  optional int32 error_code = 2;
+  optional string error_message = 3;


Let's follow a standard that error_code and error_message as the first 2 fields, because we will add more fields at the end.

wuchong · 2025-12-26T09:21:12Z

fluss-rpc/src/main/proto/FlussApi.proto

+  optional string error_message = 3;
+}
+
+message PbTableBucketOffsets {


rename to PbTableOffsets? This makes it more clear it is a set of offsets for a table.

wuchong · 2025-12-26T09:47:02Z

fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTable.java

     * @return the LakeTableSnapshot
     */
-    public LakeTableSnapshot getLatestTableSnapshot() throws Exception {
+    public LakeTableSnapshot getLatestTableSnapshot() throws IOException {


Consider renaming this method to getOrReadLatestTableSnapshot to explicitly indicate that it may perform I/O operations (e.g., reading from remote storage) when the snapshot isn't already available in memory. This makes the method’s behavior clearer to callers and improves code readability.

wuchong · 2025-12-26T09:49:56Z

fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTableJsonSerde.java

+            // Version 1: ZK node contains full snapshot data, use LakeTableSnapshotJsonSerde
+            LakeTableSnapshotJsonSerde.INSTANCE.serialize(
+                    lakeTable.getLatestTableSnapshot(), generator);
+        } else {
            generator.writeStartObject();
+            generator.writeNumberField(VERSION_KEY, CURRENT_VERSION);
+
+            generator.writeArrayFieldStart(LAKE_SNAPSHOTS);
+            for (LakeTable.LakeSnapshotMetadata lakeSnapshotMetadata :


It would be better to extract separate serializeV1() and serializeV2() methods to improve maintainability and readability. The same applies to the deserialize.

wuchong · 2025-12-26T09:50:48Z

fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTableSnapshotJsonSerde.java

+ * @see LakeTableJsonSerde for the current format (version 2) that uses this serde for legacy
+ *     compatibility
 */
 public class LakeTableSnapshotJsonSerde


Consider to rename this to LakeTableSnapshotLegacyJsonSerde (Currenlty there is too many LakeXxxSerde)

wuchong · 2025-12-26T10:03:47Z

fluss-server/src/test/java/org/apache/fluss/server/zk/data/LakeTableSnapshotJsonSerdeTest.java

    }
-
-    @Test
-    void testBackwardCompatibility() {


We still need this for backward compatibility for the max_timestamp and log_start_offset fields.

wuchong · 2025-12-26T10:08:49Z

fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorService.java

    }

+    @Override
+    public CompletableFuture<PrepareCommitLakeTableSnapshotResponse> prepareCommitLakeTableSnapshot(


move this method before commitLakeTableSnapshot.

luoyuxia force-pushed the allow-commit-offset-to-fluss branch from 33969e9 to 5442462 Compare December 22, 2025 08:45

luoyuxia requested a review from Copilot December 22, 2025 08:47

Copilot AI reviewed Dec 22, 2025

View reviewed changes

Copilot started reviewing on behalf of luoyuxia December 22, 2025 09:19 View session

luoyuxia force-pushed the allow-commit-offset-to-fluss branch from 5442462 to 68a5039 Compare December 22, 2025 12:49

luoyuxia marked this pull request as ready for review December 22, 2025 13:00

luoyuxia requested a review from Copilot December 22, 2025 13:00

Copilot started reviewing on behalf of luoyuxia December 22, 2025 13:01 View session

Copilot AI reviewed Dec 22, 2025

View reviewed changes

luoyuxia force-pushed the allow-commit-offset-to-fluss branch 2 times, most recently from 06af001 to 3889069 Compare December 22, 2025 13:22

luoyuxia changed the title ~~Allow commit offset to fluss~~ [lake] Record a file path storing log offsets in lake snapshot property Dec 23, 2025

luoyuxia mentioned this pull request Dec 24, 2025

[lake] Fix zk lake snapshot node compatible issue #2228

Closed

luoyuxia marked this pull request as draft December 24, 2025 03:07

luoyuxia force-pushed the allow-commit-offset-to-fluss branch from a6c7f69 to 747e91b Compare December 24, 2025 12:35

luoyuxia requested a review from Copilot December 24, 2025 12:36

Copilot started reviewing on behalf of luoyuxia December 24, 2025 12:36 View session

luoyuxia force-pushed the allow-commit-offset-to-fluss branch from 747e91b to f31d6f5 Compare December 24, 2025 12:40

Copilot AI reviewed Dec 24, 2025

View reviewed changes

luoyuxia requested a review from Copilot December 24, 2025 23:56

Copilot started reviewing on behalf of luoyuxia December 24, 2025 23:57 View session

Copilot AI reviewed Dec 25, 2025

View reviewed changes

luoyuxia force-pushed the allow-commit-offset-to-fluss branch from 37c8016 to f7a09fa Compare December 25, 2025 02:06

luoyuxia marked this pull request as ready for review December 25, 2025 02:06

luoyuxia force-pushed the allow-commit-offset-to-fluss branch from f7a09fa to f470b32 Compare December 25, 2025 02:27

luoyuxia added 4 commits December 26, 2025 09:59

[lake] Only record lake snapshot file path in lake snapshots

7d290f6

minor fix

c864509

refactor

c625004

minor fix

c29d0f6

move prepare commit to coordinator gateway

0451cbf

luoyuxia force-pushed the allow-commit-offset-to-fluss branch from f470b32 to 0451cbf Compare December 26, 2025 02:19

wuchong reviewed Dec 26, 2025

View reviewed changes

wuchong added the priority=blocker label Dec 28, 2025

		message PrepareCommitLakeTableSnapshotResponse {
		repeated PbPrepareCommitLakeTableRespForTable prepare_commit_lake_table_resp = 1;

		optional int32 error_code = 2;
		optional string error_message = 3;

[lake] Record a file path storing log offsets in lake snapshot property #2223

Are you sure you want to change the base?

[lake] Record a file path storing log offsets in lake snapshot property #2223

Conversation

luoyuxia commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Brief change log

Tests

API and Format

Documentation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luoyuxia commented Dec 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

luoyuxia commented Dec 22, 2025 •

edited

Loading