-
Notifications
You must be signed in to change notification settings - Fork 431
[KV] Avoid TabletServer to discard already commited kv snapshot #1738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fluss-server/src/main/java/org/apache/fluss/server/kv/snapshot/KvTabletSnapshotTarget.java
Outdated
Show resolved
Hide resolved
fluss-server/src/main/java/org/apache/fluss/server/kv/snapshot/KvTabletSnapshotTarget.java
Outdated
Show resolved
Hide resolved
fluss-server/src/main/java/org/apache/fluss/server/kv/snapshot/KvTabletSnapshotTarget.java
Outdated
Show resolved
Hide resolved
fluss-server/src/main/java/org/apache/fluss/server/kv/snapshot/KvTabletSnapshotTarget.java
Outdated
Show resolved
Hide resolved
fluss-server/src/test/java/org/apache/fluss/server/kv/snapshot/KvTabletSnapshotTargetTest.java
Show resolved
Hide resolved
4ee4e6e to
41b691f
Compare
|
@wuchong Thank you for your review. I have addressed all the comments above, except for the handling of undetermined snapshots, which I still treat as failures. I've explained the reasoning behind this decision above. Please review again when you have time. |
fluss-server/src/main/java/org/apache/fluss/server/kv/snapshot/KvTabletSnapshotTarget.java
Outdated
Show resolved
Hide resolved
|
Hi @platinumhamburg , I left a comment about the snapshot cleanup, and a minor comment in the review. Please let me know your thoughts. |
Hi @wuchong , I agree that your suggestion is a better approach to handle the ZooKeeper query exception. I've modified my implementation accordingly and fixed another minor issue. Thanks for reviewing this again. |
|
@platinumhamburg thanks for the updates. LGTM. |
Purpose
Linked issue: close #1304
Brief change log
Tests
API and Format
Documentation