fix ut

risingwavelabs · Sep 18, 2024 · 1da5cff · 1da5cff
1 parent 3746379
commit 1da5cff
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 35 deletions.
diff --git a/src/meta/src/hummock/manager/compaction/compaction_group_schedule.rs b/src/meta/src/hummock/manager/compaction/compaction_group_schedule.rs
@@ -16,6 +16,7 @@ use std::collections::{BTreeMap, HashMap, HashSet, VecDeque};
 use std::ops::DerefMut;
 use std::sync::Arc;
 
+use bytes::Bytes;
 use itertools::Itertools;
 use risingwave_common::catalog::TableId;
 use risingwave_common::hash::VirtualNode;
@@ -421,7 +422,9 @@ impl HummockManager {
             )));
         }
 
-        let split_key = group_split::build_split_key(table_id, vnode);
+        let split_full_key = group_split::build_split_full_key(table_id, vnode);
+
+        // change to vec for partition
         let table_ids = member_table_ids
             .iter()
             .map(|table_id| table_id.table_id)
@@ -430,8 +433,8 @@ impl HummockManager {
         let (table_ids_left, table_ids_right) =
             group_split::split_table_ids_with_table_id_and_vnode(
                 &table_ids,
-                table_id,
-                vnode.to_index(),
+                split_full_key.user_key.table_id.table_id(),
+                split_full_key.user_key.get_vnode_id(),
             );
         if table_ids_left.is_empty() || table_ids_right.is_empty() {
             // not need to split group if all tables are in the same side
@@ -447,6 +450,8 @@ impl HummockManager {
 
         result.push((parent_group_id, table_ids_left));
 
+        let split_key: Bytes = split_full_key.encode().into();
+
         let mut version = HummockVersionTransaction::new(
             &mut versioning.current_version,
             &mut versioning.hummock_version_deltas,
@@ -605,7 +610,6 @@ impl HummockManager {
         // split key
         // 1. table_id = 3, vnode = 0, epoch = 0
         // 2. table_id = 7, vnode = 0, epoch = 0
-        let table_ids = table_ids.iter().cloned().unique().collect_vec();
 
         // The new compaction group id is always generate on the right side
         // Hence, we return the first compaction group id as the result
@@ -633,12 +637,12 @@ impl HummockManager {
         }
 
         // split 2
-        // Use table_id + 1 as the split key to split the table_ids. See the example in L603 and the split rule in `split_compaction_group_impl`.
+        // See the example in L603 and the split rule in `split_compaction_group_impl`.
         let result_vec = self
             .split_compaction_group_impl(
                 target_compaction_group_id,
-                *table_ids.last().unwrap() + 1,
-                VirtualNode::ZERO,
+                *table_ids.last().unwrap(),
+                VirtualNode::MAX,
             )
             .await?;
         assert!(result_vec.len() <= 2);

diff --git a/src/storage/hummock_sdk/src/compaction_group/mod.rs b/src/storage/hummock_sdk/src/compaction_group/mod.rs
@@ -67,7 +67,15 @@ pub mod group_split {
     /// 3. Due to the above rule, `vnode` max will be rewritten as `table_id` + 1, vnode 0
 
     // By default, the split key is constructed with vnode = 0 and epoch = 0, so that we can split table_id to the right group
-    pub fn build_split_key(mut table_id: StateTableId, mut vnode: VirtualNode) -> Bytes {
+    pub fn build_split_key(table_id: StateTableId, vnode: VirtualNode) -> Bytes {
+        build_split_full_key(table_id, vnode).encode().into()
+    }
+
+    /// generate a full key for convenience to get the `table_id` and `vnode`
+    pub fn build_split_full_key(
+        mut table_id: StateTableId,
+        mut vnode: VirtualNode,
+    ) -> FullKey<Vec<u8>> {
         if VirtualNode::MAX == vnode {
             // Modify `table_id` to `next_table_id` to satisfy the `split_to_right`` rule, so that the `table_id`` originally passed in will be split to left.
             table_id = table_id.strict_add(1);
@@ -76,11 +84,9 @@ pub mod group_split {
 
         FullKey::new(
             TableId::from(table_id),
-            TableKey(vnode.to_be_bytes()),
+            TableKey(vnode.to_be_bytes().to_vec()),
             HummockEpoch::MIN,
         )
-        .encode()
-        .into()
     }
 
     #[derive(Debug, PartialEq, Clone)]

diff --git a/src/storage/hummock_test/src/compactor_tests.rs b/src/storage/hummock_test/src/compactor_tests.rs
@@ -2215,28 +2215,11 @@ pub(crate) mod tests {
                 .unwrap();
         }
 
-        // compact
-        compact_once(
-            parent_group_id,
-            0,
-            hummock_manager_ref.clone(),
-            compact_ctx.clone(),
-            filter_key_extractor_manager.clone(),
-            sstable_object_id_manager.clone(),
-        )
-        .await;
-
         let new_cg_id = hummock_manager_ref
             .split_compaction_group(parent_group_id, &split_table_ids, 0)
             .await
             .unwrap();
 
-        assert_ne!(parent_group_id, new_cg_id);
-        assert!(hummock_manager_ref
-            .merge_compaction_group(parent_group_id, new_cg_id)
-            .await
-            .is_err());
-
         write_data(
             &storage,
             (&mut local_1, true),
@@ -2463,14 +2446,7 @@ pub(crate) mod tests {
             .await
             .unwrap();
 
-        // try merge
-        assert!(hummock_manager_ref
-            .merge_compaction_group(parent_group_id, new_cg_id)
-            .await
-            .is_err());
-
         // write left and write
-
         write_data(
             &storage,
             (&mut local_1, true),