Skip to content

Commit

Permalink
refactor(state-store): use dyn_clone for boxed state store and remove…
Browse files Browse the repository at this point in the history 
… HummockTrait (#12409)
  • Loading branch information
@wenym1
wenym1 authored Sep 19, 2023
1 parent afd5791 commit 441583a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 113 deletions.
2 changes: 1 addition & 1 deletion src/compute/src/server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ pub async fn compute_node_serve(
observer_manager.start().await;

let mut extra_info_sources: Vec<ExtraInfoSourceRef> = vec![];
if let Some(storage) = state_store.as_hummock_trait() {
if let Some(storage) = state_store.as_hummock() {
extra_info_sources.push(storage.sstable_object_id_manager().clone());
if embedded_compactor_enabled {
tracing::info!("start embedded compactor");
Expand Down
145 changes: 33 additions & 112 deletions src/storage/src/store_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,11 @@ use risingwave_common_service::observer_manager::RpcNotificationClient;
use risingwave_object_store::object::parse_remote_object_store;

use crate::error::StorageResult;
use crate::filter_key_extractor::{
FilterKeyExtractorManager, RemoteTableAccessor, RpcFilterKeyExtractorManager,
};
use crate::hummock::backup_reader::BackupReaderRef;
use crate::filter_key_extractor::{RemoteTableAccessor, RpcFilterKeyExtractorManager};
use crate::hummock::hummock_meta_client::MonitoredHummockMetaClient;
use crate::hummock::sstable_store::SstableStoreRef;
use crate::hummock::{
set_foyer_metrics_registry, FileCache, FoyerRuntimeConfig, FoyerStoreConfig, HummockError,
HummockStorage, MemoryLimiter, SstableObjectIdManagerRef, SstableStore,
HummockStorage, SstableStore,
};
use crate::memory::sled::SledStateStore;
use crate::memory::MemoryStateStore;
Expand All @@ -41,9 +37,9 @@ use crate::monitor::{
use crate::opts::StorageOpts;
use crate::StateStore;

pub type HummockStorageType = impl StateStore + AsHummockTrait;
pub type MemoryStateStoreType = impl StateStore + AsHummockTrait;
pub type SledStateStoreType = impl StateStore + AsHummockTrait;
pub type HummockStorageType = impl StateStore + AsHummock;
pub type MemoryStateStoreType = impl StateStore + AsHummock;
pub type SledStateStoreType = impl StateStore + AsHummock;

/// The type erased [`StateStore`].
#[derive(Clone, EnumAsInner)]
Expand All @@ -66,9 +62,7 @@ pub enum StateStoreImpl {
SledStateStore(Monitored<SledStateStoreType>),
}

fn may_dynamic_dispatch(
state_store: impl StateStore + AsHummockTrait,
) -> impl StateStore + AsHummockTrait {
fn may_dynamic_dispatch(state_store: impl StateStore + AsHummock) -> impl StateStore + AsHummock {
#[cfg(not(debug_assertions))]
{
state_store
Expand All @@ -80,7 +74,7 @@ fn may_dynamic_dispatch(
}
}

fn may_verify(state_store: impl StateStore + AsHummockTrait) -> impl StateStore + AsHummockTrait {
fn may_verify(state_store: impl StateStore + AsHummock) -> impl StateStore + AsHummock {
#[cfg(not(debug_assertions))]
{
state_store
Expand Down Expand Up @@ -143,30 +137,11 @@ impl StateStoreImpl {
)
}

pub fn as_hummock_trait(&self) -> Option<&dyn HummockTrait> {
{
match self {
StateStoreImpl::HummockStateStore(hummock) => Some(
hummock
.inner()
.as_hummock_trait()
.expect("should be hummock"),
),
_ => None,
}
}
}

pub fn as_hummock(&self) -> Option<&HummockStorage> {
match self {
StateStoreImpl::HummockStateStore(hummock) => Some(
hummock
.inner()
.as_hummock_trait()
.expect("should be hummock")
.as_hummock()
.expect("should be hummock"),
),
StateStoreImpl::HummockStateStore(hummock) => {
Some(hummock.inner().as_hummock().expect("should be hummock"))
}
_ => None,
}
}
Expand Down Expand Up @@ -235,9 +210,10 @@ pub mod verify {
use tracing::log::warn;

use crate::error::{StorageError, StorageResult};
use crate::hummock::HummockStorage;
use crate::storage_value::StorageValue;
use crate::store::*;
use crate::store_impl::{AsHummockTrait, HummockTrait};
use crate::store_impl::AsHummock;
use crate::StateStore;

fn assert_result_eq<Item: PartialEq + Debug, E>(
Expand All @@ -264,9 +240,9 @@ pub mod verify {
pub expected: Option<E>,
}

impl<A: AsHummockTrait, E> AsHummockTrait for VerifyStateStore<A, E> {
fn as_hummock_trait(&self) -> Option<&dyn HummockTrait> {
self.actual.as_hummock_trait()
impl<A: AsHummock, E> AsHummock for VerifyStateStore<A, E> {
fn as_hummock(&self) -> Option<&HummockStorage> {
self.actual.as_hummock()
}
}

Expand Down Expand Up @@ -676,60 +652,24 @@ impl StateStoreImpl {
}
}

/// This trait is for aligning some common methods of `state_store_impl` for external use
pub trait HummockTrait {
fn sstable_object_id_manager(&self) -> &SstableObjectIdManagerRef;
fn sstable_store(&self) -> SstableStoreRef;
fn filter_key_extractor_manager(&self) -> &FilterKeyExtractorManager;
fn get_memory_limiter(&self) -> Arc<MemoryLimiter>;
fn backup_reader(&self) -> BackupReaderRef;
pub trait AsHummock {
fn as_hummock(&self) -> Option<&HummockStorage>;
}

impl HummockTrait for HummockStorage {
fn sstable_object_id_manager(&self) -> &SstableObjectIdManagerRef {
self.sstable_object_id_manager()
}

fn sstable_store(&self) -> SstableStoreRef {
self.sstable_store()
}

fn filter_key_extractor_manager(&self) -> &FilterKeyExtractorManager {
self.filter_key_extractor_manager()
}

fn get_memory_limiter(&self) -> Arc<MemoryLimiter> {
self.get_memory_limiter()
}

fn backup_reader(&self) -> BackupReaderRef {
self.backup_reader()
}

impl AsHummock for HummockStorage {
fn as_hummock(&self) -> Option<&HummockStorage> {
Some(self)
}
}

pub trait AsHummockTrait {
fn as_hummock_trait(&self) -> Option<&dyn HummockTrait>;
}

impl AsHummockTrait for HummockStorage {
fn as_hummock_trait(&self) -> Option<&dyn HummockTrait> {
Some(self)
}
}

impl AsHummockTrait for MemoryStateStore {
fn as_hummock_trait(&self) -> Option<&dyn HummockTrait> {
impl AsHummock for MemoryStateStore {
fn as_hummock(&self) -> Option<&HummockStorage> {
None
}
}

impl AsHummockTrait for SledStateStore {
fn as_hummock_trait(&self) -> Option<&dyn HummockTrait> {
impl AsHummock for SledStateStore {
fn as_hummock(&self) -> Option<&HummockStorage> {
None
}
}
Expand All @@ -740,14 +680,16 @@ pub mod boxed_state_store {
use std::ops::{Bound, Deref, DerefMut};

use bytes::Bytes;
use dyn_clone::{clone_trait_object, DynClone};
use futures::stream::BoxStream;
use futures::StreamExt;
use risingwave_hummock_sdk::key::{TableKey, TableKeyRange};
use risingwave_hummock_sdk::HummockReadEpoch;

use crate::error::StorageResult;
use crate::hummock::HummockStorage;
use crate::store::*;
use crate::store_impl::{AsHummockTrait, HummockTrait};
use crate::store_impl::AsHummock;
use crate::StateStore;

// For StateStoreRead
Expand Down Expand Up @@ -1037,44 +979,23 @@ pub mod boxed_state_store {
}
}

// With this trait, we can implement `Clone` for BoxDynamicDispatchedStateStore
pub trait DynamicDispatchedStateStoreCloneBox {
fn clone_box(&self) -> BoxDynamicDispatchedStateStore;
}

pub trait DynamicDispatchedStateStore:
DynamicDispatchedStateStoreCloneBox
+ DynamicDispatchedStateStoreRead
+ DynamicDispatchedStateStoreExt
+ AsHummockTrait
DynClone + DynamicDispatchedStateStoreRead + DynamicDispatchedStateStoreExt + AsHummock
{
}

impl<
S: DynamicDispatchedStateStoreCloneBox
+ DynamicDispatchedStateStoreRead
+ DynamicDispatchedStateStoreExt
+ AsHummockTrait,
> DynamicDispatchedStateStore for S
{
}
clone_trait_object!(DynamicDispatchedStateStore);

impl<S: StateStore + AsHummockTrait> DynamicDispatchedStateStoreCloneBox for S {
fn clone_box(&self) -> BoxDynamicDispatchedStateStore {
Box::new(self.clone())
impl AsHummock for BoxDynamicDispatchedStateStore {
fn as_hummock(&self) -> Option<&HummockStorage> {
self.deref().as_hummock()
}
}

impl AsHummockTrait for BoxDynamicDispatchedStateStore {
fn as_hummock_trait(&self) -> Option<&dyn HummockTrait> {
self.deref().as_hummock_trait()
}
}

impl Clone for BoxDynamicDispatchedStateStore {
fn clone(&self) -> Self {
self.deref().clone_box()
}
impl<
S: DynClone + DynamicDispatchedStateStoreRead + DynamicDispatchedStateStoreExt + AsHummock,
> DynamicDispatchedStateStore for S
{
}

impl StateStore for BoxDynamicDispatchedStateStore {
Expand Down

0 comments on commit 441583a

Please sign in to comment.