[lake] Fix zk lake snapshot node compatible issue #2228
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
luoyuxia
force-pushed
the
fix-compatible-issue
branch
from
feecfc7 to
4660b24
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses a compatibility issue with ZooKeeper lake snapshot node serialization by introducing versioned serialization support. The changes enable seamless upgrades by allowing the coordinator to use different serialization formats based on the client's capabilities.
Key Changes:
- Added optional
lake_snapshot_serialization_versionfield toCommitLakeTableSnapshotRequestfor clients to indicate their serialization capability - Coordinator now routes to v1 (legacy) or v2 (current) serialization based on the presence of this field
- New
upsertLakeTableV1method inLakeTableHelperfor backward compatibility with legacy tiering services
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| fluss-rpc/src/main/proto/FlussApi.proto | Added optional lake_snapshot_serialization_version field to CommitLakeTableSnapshotRequest with comprehensive documentation |
| fluss-server/src/main/java/org/apache/fluss/server/entity/CommitLakeTableSnapshotData.java | Added serializationVersion field to track client's serialization capability |
| fluss-server/src/main/java/org/apache/fluss/server/utils/ServerRpcMessageUtils.java | Extracts serialization version from request, defaults to null for legacy clients |
| fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorEventProcessor.java | Routes to v1 or v2 upsert method based on serialization version presence |
| fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTableHelper.java | Implements upsertLakeTableV1 for legacy format support with merging logic |
| fluss-server/src/main/java/org/apache/fluss/server/zk/ZooKeeperClient.java | Updated upsertLakeTable to support both v1 and v2 encoding formats |
| fluss-server/src/main/java/org/apache/fluss/server/zk/data/ZkData.java | Added encodeV1 method for legacy format encoding |
| fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTableJsonSerde.java | Added public serializeV1 method for backward compatibility |
| fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTableSnapshotJsonSerde.java | Made deserializeVersion1 and toJsonVersion1 public for compatibility testing |
| fluss-server/src/main/java/org/apache/fluss/server/zk/data/lake/LakeTable.java | Changed exception type from Exception to IOException for better specificity |
| fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/tiering/committer/FlussTableLakeSnapshotCommitter.java | Sets serialization version (v2) in commit requests; exposed test helpers |
| fluss-flink/fluss-flink-common/src/test/java/org/apache/fluss/flink/tiering/committer/FlussTableLakeSnapshotCommitterTest.java | Added test for backward compatibility without serialization version |
| fluss-server/src/test/java/org/apache/fluss/server/zk/data/lake/LakeTableHelperTest.java | Simplified test to use new upsertLakeTableV1 method for v1 format testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TablePath tablePath = | ||
| TablePath.of( | ||
| "fluss", | ||
| "test_lagacy_version_commit" + (isPartitioned ? "_partitioned" : "")); |
There was a problem hiding this comment.
Spelling error: "lagacy" should be "legacy".
Suggested change
| "test_lagacy_version_commit" + (isPartitioned ? "_partitioned" : "")); | |
| "test_legacy_version_commit" + (isPartitioned ? "_partitioned" : "")); |
luoyuxia
force-pushed
the
fix-compatible-issue
branch
from
4660b24 to
c5618bc
Compare
Contributor
Author
|
@wuchong Could you please help review this pr which to fix the compatible issue for zk lake snapshot node? |
Contributor
Author
|
Will fix in #2223 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Linked issue: close #xxx
This fix the compatible issue introduced by #2037
After #2037, when upgrade coordinator server firstly, the coordinator server will write zk node with v2 version, but the tablet server which is still not upgraded can't regconize v2 which casue the exception.
Brief change log
So, the upgrade process can be:
Tests
API and Format
Documentation