From ea7ffc1a6c547655d768d208df854f19d7beb2af Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 12 Sep 2024 17:10:35 +0800 Subject: [PATCH] give up and use 256 for singleton vnode bitmap Signed-off-by: Bugen Zhao --- src/common/src/hash/consistent_hash/bitmap.rs | 24 +++++++++++++------ .../src/hash/consistent_hash/mapping.rs | 3 ++- src/common/src/hash/table_distribution.rs | 9 +++---- src/meta/src/stream/stream_graph/schedule.rs | 4 ++-- .../log_store_impl/kv_log_store/serde.rs | 2 +- src/stream/src/executor/actor.rs | 3 ++- 6 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/common/src/hash/consistent_hash/bitmap.rs b/src/common/src/hash/consistent_hash/bitmap.rs index eee6a64a2b42c..397335b2e7ea0 100644 --- a/src/common/src/hash/consistent_hash/bitmap.rs +++ b/src/common/src/hash/consistent_hash/bitmap.rs @@ -13,8 +13,9 @@ // limitations under the License. use std::ops::RangeInclusive; +use std::sync::{Arc, LazyLock}; -use crate::bitmap::Bitmap; +use crate::bitmap::{Bitmap, BitmapBuilder}; use crate::hash::table_distribution::SINGLETON_VNODE; use crate::hash::VirtualNode; @@ -39,15 +40,24 @@ impl Bitmap { } /// Returns whether only the [`SINGLETON_VNODE`] is set in the bitmap. - /// - /// Note that this method returning `true` does not imply that the bitmap was created by - /// [`VnodeBitmapExt::singleton`], or that the bitmap has length 1. pub fn is_singleton(&self) -> bool { self.count_ones() == 1 && self.iter_vnodes().next().unwrap() == SINGLETON_VNODE } - /// Creates a bitmap with length 1 and the single bit set. - pub fn singleton() -> Self { - Self::ones(1) + /// Get the reference to a vnode bitmap for singleton actor or table, i.e., with length + /// [`VirtualNode::COUNT`] and only the [`SINGLETON_VNODE`] set to 1. + pub fn singleton() -> &'static Self { + Self::singleton_arc() + } + + /// Get the reference to a vnode bitmap for singleton actor or table, i.e., with length + /// [`VirtualNode::COUNT`] and only the [`SINGLETON_VNODE`] set to 1. + pub fn singleton_arc() -> &'static Arc { + static SINGLETON: LazyLock> = LazyLock::new(|| { + let mut builder = BitmapBuilder::zeroed(VirtualNode::COUNT); + builder.set(SINGLETON_VNODE.to_index(), true); + builder.finish().into() + }); + &SINGLETON } } diff --git a/src/common/src/hash/consistent_hash/mapping.rs b/src/common/src/hash/consistent_hash/mapping.rs index 80d5a56941cf6..644349b10d7ed 100644 --- a/src/common/src/hash/consistent_hash/mapping.rs +++ b/src/common/src/hash/consistent_hash/mapping.rs @@ -140,7 +140,8 @@ impl VnodeMapping { } /// Create a vnode mapping with the single item. Should only be used for singletons. - // TODO(var-vnode): make vnode count 1, also `Distribution::vnode_count` + /// + /// For backwards compatibility, [`VirtualNode::COUNT`] is used as the vnode count. pub fn new_single(item: T::Item) -> Self { Self::new_uniform(std::iter::once(item), VirtualNode::COUNT) } diff --git a/src/common/src/hash/table_distribution.rs b/src/common/src/hash/table_distribution.rs index 5275aca04adb3..822db591c1577 100644 --- a/src/common/src/hash/table_distribution.rs +++ b/src/common/src/hash/table_distribution.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::mem::replace; -use std::sync::{Arc, LazyLock}; +use std::sync::Arc; use itertools::Itertools; use risingwave_pb::plan_common::StorageTableDesc; @@ -74,7 +74,7 @@ impl TableDistribution { ) -> Self { let compute_vnode = if let Some(vnode_col_idx_in_pk) = vnode_col_idx_in_pk { ComputeVnode::VnodeColumnIndex { - vnodes: vnodes.unwrap_or_else(|| Bitmap::singleton().into()), + vnodes: vnodes.unwrap_or_else(|| Bitmap::singleton_arc().clone()), vnode_col_idx_in_pk, } } else if !dist_key_in_pk_indices.is_empty() { @@ -132,13 +132,10 @@ impl TableDistribution { /// Get vnode bitmap if distributed, or a dummy [`Bitmap::singleton()`] if singleton. pub fn vnodes(&self) -> &Arc { - static SINGLETON_VNODES: LazyLock> = - LazyLock::new(|| Bitmap::singleton().into()); - match &self.compute_vnode { ComputeVnode::DistKeyIndices { vnodes, .. } => vnodes, ComputeVnode::VnodeColumnIndex { vnodes, .. } => vnodes, - ComputeVnode::Singleton => &SINGLETON_VNODES, + ComputeVnode::Singleton => Bitmap::singleton_arc(), } } diff --git a/src/meta/src/stream/stream_graph/schedule.rs b/src/meta/src/stream/stream_graph/schedule.rs index f67d8547e28a8..ef2691deb1807 100644 --- a/src/meta/src/stream/stream_graph/schedule.rs +++ b/src/meta/src/stream/stream_graph/schedule.rs @@ -152,8 +152,8 @@ impl Distribution { } /// Get the vnode count of the distribution. - // TODO(var-vnode): after `ServingVnodeMapping::upsert` is made vnode-count-aware, - // we may return 1 for singleton. + /// + /// For backwards compatibility, [`VirtualNode::COUNT`] is used for singleton. pub fn vnode_count(&self) -> usize { match self { Distribution::Singleton(_) => VirtualNode::COUNT, diff --git a/src/stream/src/common/log_store_impl/kv_log_store/serde.rs b/src/stream/src/common/log_store_impl/kv_log_store/serde.rs index ec7cc62f2d49c..6e020536cfb99 100644 --- a/src/stream/src/common/log_store_impl/kv_log_store/serde.rs +++ b/src/stream/src/common/log_store_impl/kv_log_store/serde.rs @@ -201,7 +201,7 @@ impl LogStoreRowSerde { let vnodes = match vnodes { Some(vnodes) => vnodes, - None => Bitmap::singleton().into(), + None => Bitmap::singleton_arc().clone(), }; // epoch and seq_id. The seq_id of barrier is set null, and therefore the second order type diff --git a/src/stream/src/executor/actor.rs b/src/stream/src/executor/actor.rs index 1fb13d8518386..acb6bc291a8a4 100644 --- a/src/stream/src/executor/actor.rs +++ b/src/stream/src/executor/actor.rs @@ -103,7 +103,8 @@ impl ActorContext { fragment_id: stream_actor.fragment_id, mview_definition: stream_actor.mview_definition.clone(), vnode_count: (stream_actor.vnode_bitmap.as_ref()) - // TODO(var-vnode): use 1 for singleton fragment + // An unset `vnode_bitmap` means the actor is a singleton. + // For backwards compatibility, `VirtualNode::COUNT` is used for singleton. .map_or(VirtualNode::COUNT, |b| Bitmap::from(b).len()), cur_mem_val: Arc::new(0.into()), last_mem_val: Arc::new(0.into()),