Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure the composed store adaptors #2637

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 39 additions & 13 deletions linera-client/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,14 @@ use async_trait::async_trait;
use linera_execution::WasmRuntime;
use linera_storage::{DbStorage, Storage};
#[cfg(feature = "storage-service")]
use linera_storage_service::{client::ServiceStoreClient, common::ServiceStoreConfig};
use linera_storage_service::{
client::{ServiceStoreClient, ServiceStoreConfig},
common::ServiceStoreInternalConfig,
};
#[cfg(feature = "dynamodb")]
use linera_views::dynamo_db::{get_config, DynamoDbStore, DynamoDbStoreConfig};
use linera_views::dynamo_db::{
get_config, DynamoDbStore, DynamoDbStoreConfig, DynamoDbStoreInternalConfig,
};
#[cfg(with_storage)]
use linera_views::store::LocalAdminKeyValueStore as _;
use linera_views::{
Expand All @@ -20,12 +25,15 @@ use linera_views::{
use tracing::error;
#[cfg(feature = "rocksdb")]
use {
linera_views::rocks_db::{PathWithGuard, RocksDbSpawnMode, RocksDbStore, RocksDbStoreConfig},
linera_views::rocks_db::{
PathWithGuard, RocksDbSpawnMode, RocksDbStore, RocksDbStoreConfig,
RocksDbStoreInternalConfig,
},
std::path::PathBuf,
};
#[cfg(feature = "scylladb")]
use {
linera_views::scylla_db::{ScyllaDbStore, ScyllaDbStoreConfig},
linera_views::scylla_db::{ScyllaDbStore, ScyllaDbStoreConfig, ScyllaDbStoreInternalConfig},
std::num::NonZeroU16,
tracing::debug,
};
Expand Down Expand Up @@ -335,41 +343,59 @@ impl StorageConfigNamespace {
#[cfg(feature = "storage-service")]
StorageConfig::Service { endpoint } => {
let endpoint = endpoint.clone();
let config = ServiceStoreConfig {
let inner_config = ServiceStoreInternalConfig {
endpoint,
common_config,
common_config: common_config.reduced(),
};
let config = ServiceStoreConfig {
inner_config,
cache_size: common_config.cache_size,
};
Ok(StoreConfig::Service(config, namespace))
}
StorageConfig::Memory => {
let config = MemoryStoreConfig { common_config };
let config = MemoryStoreConfig {
common_config: common_config.reduced(),
};
Ok(StoreConfig::Memory(config, namespace))
}
#[cfg(feature = "rocksdb")]
StorageConfig::RocksDb { path, spawn_mode } => {
let path_buf = path.to_path_buf();
let path_with_guard = PathWithGuard::new(path_buf);
let config = RocksDbStoreConfig {
let inner_config = RocksDbStoreInternalConfig {
path_with_guard,
spawn_mode: *spawn_mode,
common_config,
common_config: common_config.reduced(),
};
let config = RocksDbStoreConfig {
inner_config,
cache_size: common_config.cache_size,
};
Ok(StoreConfig::RocksDb(config, namespace))
}
#[cfg(feature = "dynamodb")]
StorageConfig::DynamoDb { use_localstack } => {
let aws_config = get_config(*use_localstack).await?;
let config = DynamoDbStoreConfig {
let inner_config = DynamoDbStoreInternalConfig {
config: aws_config,
common_config,
common_config: common_config.reduced(),
};
let config = DynamoDbStoreConfig {
inner_config,
cache_size: common_config.cache_size,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache_size belongs to a LruCacheConfig.

Copy link
Contributor

@ma2bd ma2bd Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So compared to this PR, my idea was to replace DynamoDbStoreConfig by LruCacheConfig<DynamoDbStoreConfig> and rename DynamoDbStoreInternalConfig into DynamoDbStoreConfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. Indeed this PR does not implement that idea. However, that can be done next.

Thus the next step would be to remove the "Internal" attribute to the RockDb, ScyllaDb, DynamoDb and others. And they could then be exposed to users so that users could run, e.g. with ScyllaDb with cache or without.

But I think this can be done in a subsequent work.

};
Ok(StoreConfig::DynamoDb(config, namespace))
}
#[cfg(feature = "scylladb")]
StorageConfig::ScyllaDb { uri } => {
let config = ScyllaDbStoreConfig {
let inner_config = ScyllaDbStoreInternalConfig {
uri: uri.to_string(),
common_config,
common_config: common_config.reduced(),
};
let config = ScyllaDbStoreConfig {
inner_config,
cache_size: common_config.cache_size,
};
Ok(StoreConfig::ScyllaDb(config, namespace))
}
Expand Down
16 changes: 11 additions & 5 deletions linera-indexer/lib/src/rocks_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ use std::path::PathBuf;

use clap::Parser as _;
use linera_views::{
rocks_db::{PathWithGuard, RocksDbSpawnMode, RocksDbStore, RocksDbStoreConfig},
store::{AdminKeyValueStore, CommonStoreConfig},
rocks_db::{
PathWithGuard, RocksDbSpawnMode, RocksDbStore, RocksDbStoreConfig,
RocksDbStoreInternalConfig,
},
store::{AdminKeyValueStore, CommonStoreInternalConfig},
};

use crate::{
Expand Down Expand Up @@ -38,21 +41,24 @@ pub type RocksDbRunner = Runner<RocksDbStore, RocksDbConfig>;
impl RocksDbRunner {
pub async fn load() -> Result<Self, IndexerError> {
let config = IndexerConfig::<RocksDbConfig>::parse();
let common_config = CommonStoreConfig {
let common_config = CommonStoreInternalConfig {
max_concurrent_queries: config.client.max_concurrent_queries,
max_stream_queries: config.client.max_stream_queries,
cache_size: config.client.cache_size,
};
let path_buf = config.client.storage.as_path().to_path_buf();
let path_with_guard = PathWithGuard::new(path_buf);
// The tests are run in single threaded mode, therefore we need
// to use the safe default value of SpawnBlocking.
let spawn_mode = RocksDbSpawnMode::SpawnBlocking;
let store_config = RocksDbStoreConfig {
let inner_config = RocksDbStoreInternalConfig {
path_with_guard,
spawn_mode,
common_config,
};
let store_config = RocksDbStoreConfig {
inner_config,
cache_size: config.client.cache_size,
};
let namespace = config.client.namespace.clone();
let root_key = &[];
let store =
Expand Down
13 changes: 8 additions & 5 deletions linera-indexer/lib/src/scylla_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// SPDX-License-Identifier: Apache-2.0

use linera_views::{
scylla_db::{ScyllaDbStore, ScyllaDbStoreConfig},
store::{AdminKeyValueStore, CommonStoreConfig},
scylla_db::{ScyllaDbStore, ScyllaDbStoreConfig, ScyllaDbStoreInternalConfig},
store::{AdminKeyValueStore, CommonStoreInternalConfig},
};

use crate::{
Expand Down Expand Up @@ -35,17 +35,20 @@ pub type ScyllaDbRunner = Runner<ScyllaDbStore, ScyllaDbConfig>;
impl ScyllaDbRunner {
pub async fn load() -> Result<Self, IndexerError> {
let config = <IndexerConfig<ScyllaDbConfig> as clap::Parser>::parse();
let common_config = CommonStoreConfig {
let common_config = CommonStoreInternalConfig {
max_concurrent_queries: config.client.max_concurrent_queries,
max_stream_queries: config.client.max_stream_queries,
cache_size: config.client.cache_size,
};
let namespace = config.client.table.clone();
let root_key = &[];
let store_config = ScyllaDbStoreConfig {
let inner_config = ScyllaDbStoreInternalConfig {
uri: config.client.uri.clone(),
common_config,
};
let store_config = ScyllaDbStoreConfig {
inner_config,
cache_size: config.client.cache_size,
};
let store = ScyllaDbStore::connect(&store_config, &namespace, root_key).await?;
Self::new(config, store).await
}
Expand Down
4 changes: 3 additions & 1 deletion linera-service/src/cli_wrappers/local_net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ async fn make_testing_config(database: Database) -> Result<StorageConfig> {
#[cfg(feature = "scylladb")]
{
let config = ScyllaDbStore::new_test_config().await?;
Ok(StorageConfig::ScyllaDb { uri: config.uri })
Ok(StorageConfig::ScyllaDb {
uri: config.inner_config.uri,
})
}
#[cfg(not(feature = "scylladb"))]
panic!("Database::ScyllaDb is selected without the feature sctlladb");
Expand Down
Loading
Loading