wuchong commented on code in PR #1470:
URL: https://github.com/apache/fluss/pull/1470#discussion_r2252925964


##########
fluss-server/src/main/java/com/alibaba/fluss/server/coordinator/CompletedSnapshotStoreManager.java:
##########
@@ -120,13 +121,46 @@ private CompletedSnapshotStore 
createCompletedSnapshotStore(
         LOG.info("Trying to fetch {} snapshots from storage.", 
numberOfInitialSnapshots);
 
         for (CompletedSnapshotHandle snapshotStateHandle : initialSnapshots) {
-            
retrievedSnapshots.add(checkNotNull(snapshotStateHandle.retrieveCompleteSnapshot()));
+            try {
+                retrievedSnapshots.add(
+                        
checkNotNull(snapshotStateHandle.retrieveCompleteSnapshot()));
+            } catch (Exception e) {
+                if 
(e.getMessage().contains(SNAPSHOT_META_DATA_NOT_EXISTS_ERROR_MESSAGE)) {
+                    LOG.error(
+                            "metadata not found for snapshot {} of table 
bucket {}, maybe snapshot already removed or broken, detail errorMsg: {}",

Review Comment:
   minor: Let's capitalize the first letter of log messages to maintain 
consistency with the project's logging style.



##########
fluss-server/src/main/java/com/alibaba/fluss/server/coordinator/CompletedSnapshotStoreManager.java:
##########
@@ -120,13 +121,46 @@ private CompletedSnapshotStore 
createCompletedSnapshotStore(
         LOG.info("Trying to fetch {} snapshots from storage.", 
numberOfInitialSnapshots);
 
         for (CompletedSnapshotHandle snapshotStateHandle : initialSnapshots) {
-            
retrievedSnapshots.add(checkNotNull(snapshotStateHandle.retrieveCompleteSnapshot()));
+            try {
+                retrievedSnapshots.add(
+                        
checkNotNull(snapshotStateHandle.retrieveCompleteSnapshot()));
+            } catch (Exception e) {
+                if 
(e.getMessage().contains(SNAPSHOT_META_DATA_NOT_EXISTS_ERROR_MESSAGE)) {
+                    LOG.error(
+                            "metadata not found for snapshot {} of table 
bucket {}, maybe snapshot already removed or broken, detail errorMsg: {}",
+                            snapshotStateHandle.getSnapshotId(),
+                            tableBucket,
+                            e.getMessage());

Review Comment:
   Could we also print the full exception stack trace? I'm concerned that the 
root cause might be hidden within nested exceptions.
   
   ```suggestion
                               e.getMessage(), 
                               e);
   ```



##########
fluss-server/src/main/java/com/alibaba/fluss/server/kv/snapshot/CompletedSnapshotHandle.java:
##########
@@ -40,11 +40,13 @@
  */
 public class CompletedSnapshotHandle {
 
+    private long snapshotId;

Review Comment:
   use `final` modifier to mark this field is immutable. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to