fix incorrect sanity check

risingwavelabs · Sep 26, 2024 · 228bcfe · 228bcfe
1 parent d05f603
commit 228bcfe
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 6 deletions.
diff --git a/src/storage/hummock_sdk/src/version.rs b/src/storage/hummock_sdk/src/version.rs
@@ -290,7 +290,7 @@ impl HummockVersion {
                     .iter()
                     .map(|(table_id, info)| (*table_id, info.committed_epoch))
                     .collect();
-                new_state_table_committed_epochs == prev_state_table_committed_epochs
+                new_state_table_committed_epochs != prev_state_table_committed_epochs
             })
     }
 }

diff --git a/src/storage/src/hummock/event_handler/hummock_event_handler.rs b/src/storage/src/hummock/event_handler/hummock_event_handler.rs
@@ -626,7 +626,26 @@ impl HummockEventHandler {
                                 .into_iter(),
                         );
                     }
-                    is_snapshot_changed |= version_to_apply.apply_version_delta(version_delta);
+                    is_snapshot_changed |= {
+                        #[cfg(debug_assertions)]
+                        {
+                            let old_version = version_to_apply.clone();
+                            let is_snapshot_changed =
+                                version_to_apply.apply_version_delta(version_delta);
+                            assert_eq!(
+                                is_snapshot_changed,
+                                old_version.is_new_version_snapshot_changed(&version_to_apply),
+                                "{:#?}\n{:#?}",
+                                old_version.state_table_info.info(),
+                                version_to_apply.state_table_info.info()
+                            );
+                            is_snapshot_changed
+                        }
+                        #[cfg(not(debug_assertions))]
+                        {
+                            version_to_apply.apply_version_delta(version_delta)
+                        }
+                    };
                 }
 
                 (is_snapshot_changed, version_to_apply)
@@ -668,12 +687,22 @@ impl HummockEventHandler {
             );
         }
 
-        debug_assert_eq!(
-            is_snapshot_changed,
-            pinned_version
+        if cfg!(debug_assertions) {
+            // Only check that when the snapshot is changed, the `is_snapshot_changed` must be true.
+            // In some corner case, it can happen that `is_snapshot_changed` is true, but the snapshot does
+            // not change between two versions
+            if pinned_version
                 .version()
                 .is_new_version_snapshot_changed(new_pinned_version.version())
-        );
+            {
+                assert!(
+                    is_snapshot_changed,
+                    "{:#?}\n{:#?}",
+                    pinned_version.version().state_table_info.info(),
+                    new_pinned_version.version().state_table_info.info()
+                );
+            }
+        }
 
         let max_committed_epoch = new_pinned_version.visible_table_committed_epoch();