Skip to content

Commit

Permalink
give up and use 256 for singleton vnode bitmap
Browse files Browse the repository at this point in the history
Signed-off-by: Bugen Zhao <[email protected]>
  • Loading branch information
BugenZhao committed Sep 16, 2024
1 parent d932d1e commit 65dfe36
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 18 deletions.
24 changes: 17 additions & 7 deletions src/common/src/hash/consistent_hash/bitmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<Self> {
static SINGLETON: LazyLock<Arc<Bitmap>> = LazyLock::new(|| {
let mut builder = BitmapBuilder::zeroed(VirtualNode::COUNT);
builder.set(SINGLETON_VNODE.to_index(), true);
builder.finish().into()
});
&SINGLETON
}
}
3 changes: 2 additions & 1 deletion src/common/src/hash/consistent_hash/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ impl<T: VnodeMappingItem> VnodeMapping<T> {
}

/// 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)
}
Expand Down
9 changes: 3 additions & 6 deletions src/common/src/hash/table_distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -132,13 +132,10 @@ impl TableDistribution {

/// Get vnode bitmap if distributed, or a dummy [`Bitmap::singleton()`] if singleton.
pub fn vnodes(&self) -> &Arc<Bitmap> {
static SINGLETON_VNODES: LazyLock<Arc<Bitmap>> =
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(),
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/meta/src/stream/stream_graph/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/stream/src/common/log_store_impl/kv_log_store/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/stream/src/executor/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down

0 comments on commit 65dfe36

Please sign in to comment.