From 798c36f5bccfe981db4c14a85f6e54dd77db856f Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 8 Dec 2023 17:23:39 +0800 Subject: [PATCH 1/3] fix: use EPochWithGap::new_max_epoch if the provided epoch is max epoch --- src/common/src/util/epoch.rs | 7 +++++-- src/storage/hummock_sdk/src/lib.rs | 17 +++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/common/src/util/epoch.rs b/src/common/src/util/epoch.rs index e5478fae769bb..34982f1217e5f 100644 --- a/src/common/src/util/epoch.rs +++ b/src/common/src/util/epoch.rs @@ -116,8 +116,11 @@ impl Epoch { pub const EPOCH_AVAILABLE_BITS: u64 = 16; pub const MAX_SPILL_TIMES: u16 = ((1 << EPOCH_AVAILABLE_BITS) - 1) as u16; -pub const EPOCH_MASK: u64 = (1 << EPOCH_AVAILABLE_BITS) - 1; -pub const MAX_EPOCH: u64 = u64::MAX & !EPOCH_MASK; +// Low EPOCH_AVAILABLE_BITS bits set to 1 +pub const EPOCH_SPILL_TIME_MASK: u64 = (1 << EPOCH_AVAILABLE_BITS) - 1; +// High (64-EPOCH_AVAILABLE_BITS) bits set to 1 +pub const EPOCH_MASK: u64 = !EPOCH_SPILL_TIME_MASK; +pub const MAX_EPOCH: u64 = u64::MAX & EPOCH_MASK; pub fn is_max_epoch(epoch: u64) -> bool { // Since we have write `MAX_EPOCH` as max epoch to sstable in some previous version, diff --git a/src/storage/hummock_sdk/src/lib.rs b/src/storage/hummock_sdk/src/lib.rs index c6cb930e62738..a0c096d06adc4 100644 --- a/src/storage/hummock_sdk/src/lib.rs +++ b/src/storage/hummock_sdk/src/lib.rs @@ -26,7 +26,7 @@ mod key_cmp; use std::cmp::Ordering; pub use key_cmp::*; -use risingwave_common::util::epoch::EPOCH_MASK; +use risingwave_common::util::epoch::EPOCH_SPILL_TIME_MASK; use risingwave_pb::common::{batch_query_epoch, BatchQueryEpoch}; use risingwave_pb::hummock::SstableInfo; @@ -284,11 +284,12 @@ impl EpochWithGap { // So for compatibility, we must skip checking it for u64::MAX. See bug description in https://github.com/risingwavelabs/risingwave/issues/13717 #[cfg(not(feature = "enable_test_epoch"))] { - debug_assert!( - ((epoch & EPOCH_MASK) == 0) || risingwave_common::util::epoch::is_max_epoch(epoch) - ); - let epoch_with_gap = epoch + spill_offset as u64; - EpochWithGap(epoch_with_gap) + if risingwave_common::util::epoch::is_max_epoch(epoch) { + EpochWithGap::new_max_epoch() + } else { + debug_assert!((epoch & EPOCH_SPILL_TIME_MASK) == 0); + EpochWithGap(epoch + spill_offset) + } } #[cfg(feature = "enable_test_epoch")] { @@ -322,7 +323,7 @@ impl EpochWithGap { pub fn pure_epoch(&self) -> HummockEpoch { #[cfg(not(feature = "enable_test_epoch"))] { - self.0 & !EPOCH_MASK + self.0 & !EPOCH_SPILL_TIME_MASK } #[cfg(feature = "enable_test_epoch")] { @@ -331,6 +332,6 @@ impl EpochWithGap { } pub fn offset(&self) -> u64 { - self.0 & EPOCH_MASK + self.0 & EPOCH_SPILL_TIME_MASK } } From 5bf6bfd9d42179428a39fcc70df78f46619e9637 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 8 Dec 2023 17:41:28 +0800 Subject: [PATCH 2/3] fix compile error --- src/storage/hummock_sdk/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/hummock_sdk/src/lib.rs b/src/storage/hummock_sdk/src/lib.rs index a0c096d06adc4..af5b170dcc497 100644 --- a/src/storage/hummock_sdk/src/lib.rs +++ b/src/storage/hummock_sdk/src/lib.rs @@ -288,7 +288,7 @@ impl EpochWithGap { EpochWithGap::new_max_epoch() } else { debug_assert!((epoch & EPOCH_SPILL_TIME_MASK) == 0); - EpochWithGap(epoch + spill_offset) + EpochWithGap(epoch + spill_offset as u64) } } #[cfg(feature = "enable_test_epoch")] From 0bdcc8863d42801ab70bfacdd14355b49387c2be Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Sun, 10 Dec 2023 21:46:42 +0800 Subject: [PATCH 3/3] address comment --- src/common/src/util/epoch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/src/util/epoch.rs b/src/common/src/util/epoch.rs index 34982f1217e5f..b9a47b29b1071 100644 --- a/src/common/src/util/epoch.rs +++ b/src/common/src/util/epoch.rs @@ -119,7 +119,7 @@ pub const MAX_SPILL_TIMES: u16 = ((1 << EPOCH_AVAILABLE_BITS) - 1) as u16; // Low EPOCH_AVAILABLE_BITS bits set to 1 pub const EPOCH_SPILL_TIME_MASK: u64 = (1 << EPOCH_AVAILABLE_BITS) - 1; // High (64-EPOCH_AVAILABLE_BITS) bits set to 1 -pub const EPOCH_MASK: u64 = !EPOCH_SPILL_TIME_MASK; +const EPOCH_MASK: u64 = !EPOCH_SPILL_TIME_MASK; pub const MAX_EPOCH: u64 = u64::MAX & EPOCH_MASK; pub fn is_max_epoch(epoch: u64) -> bool {