Skip to content

Commit aa1749d

Browse files
committed
Make DatabaseKind the single source of truth for RocksDB database names
Move DatabaseKind from node_ctl_svc.proto to common.proto so it is available in restate-types. Add DatabaseKind::db_name() as the canonical mapping from kind to RocksDB database name, and store the kind on DbSpec so that RocksDb::kind() can be queried directly without reverse- engineering the kind from the name string. This eliminates the scattered DB_NAME constants from individual crates and removes the fragile db_name_to_kind() function that could silently go out of sync if a database was renamed.
1 parent 6db826d commit aa1749d

File tree

18 files changed

+142
-89
lines changed

18 files changed

+142
-89
lines changed

crates/bifrost/src/providers/local_loglet/log_store.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,13 @@ use restate_rocksdb::{
1919
use restate_types::config::{Configuration, LocalLogletOptions};
2020
use restate_types::errors::MaybeRetryableError;
2121
use restate_types::live::LiveLoad;
22+
use restate_types::protobuf::common::DatabaseKind;
2223
use restate_types::storage::{StorageDecodeError, StorageEncodeError};
2324

2425
use super::keys::{DATA_KEY_PREFIX_LENGTH, MetadataKey, MetadataKind};
2526
use super::log_state::{LogState, log_state_full_merge, log_state_partial_merge};
2627
use super::log_store_writer::LogStoreWriter;
2728

28-
// matches the default directory name
29-
pub(crate) const DB_NAME: &str = "local-loglet";
30-
3129
pub(crate) const DATA_CF: &str = "logstore_data";
3230
pub(crate) const METADATA_CF: &str = "logstore_metadata";
3331

@@ -74,15 +72,21 @@ impl RocksDbLogStore {
7472
let opts = options.live_load();
7573
let data_dir = opts.data_dir();
7674

77-
let db_spec = DbSpecBuilder::new(DbName::new(DB_NAME), data_dir, RocksConfigurator)
78-
.add_cf_pattern(CfExactPattern::new(DATA_CF), RocksConfigurator)
79-
.add_cf_pattern(CfExactPattern::new(METADATA_CF), RocksConfigurator)
80-
// not very important but it's to reduce the number of merges by flushing.
81-
// it's also a small cf so it should be quick.
82-
.add_to_flush_on_shutdown(CfExactPattern::new(METADATA_CF))
83-
.ensure_column_families(cfs)
84-
.build()
85-
.expect("valid spec");
75+
let kind = DatabaseKind::LocalLoglet;
76+
let db_spec = DbSpecBuilder::new(
77+
DbName::new(kind.db_name()),
78+
kind,
79+
data_dir,
80+
RocksConfigurator,
81+
)
82+
.add_cf_pattern(CfExactPattern::new(DATA_CF), RocksConfigurator)
83+
.add_cf_pattern(CfExactPattern::new(METADATA_CF), RocksConfigurator)
84+
// not very important but it's to reduce the number of merges by flushing.
85+
// it's also a small cf so it should be quick.
86+
.add_to_flush_on_shutdown(CfExactPattern::new(METADATA_CF))
87+
.ensure_column_families(cfs)
88+
.build()
89+
.expect("valid spec");
8690
let rocksdb = db_manager.open_db(db_spec).await?;
8791
Ok(Self { rocksdb })
8892
}

crates/core/protobuf/node_ctl_svc.proto

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,16 +113,8 @@ message EmbeddedMetadataClusterHealth {
113113
repeated restate.common.NodeId members = 1;
114114
}
115115

116-
enum DatabaseKind {
117-
DATABASE_KIND_UNSPECIFIED = 0;
118-
DATABASE_KIND_PARTITION_STORE = 2;
119-
DATABASE_KIND_LOG_SERVER = 3;
120-
DATABASE_KIND_METADATA_SERVER = 4;
121-
DATABASE_KIND_LOCAL_LOGLET = 5;
122-
}
123-
124116
message TriggerCompactionRequest {
125-
repeated DatabaseKind databases = 1;
117+
repeated restate.common.DatabaseKind databases = 1;
126118
}
127119

128120
message TriggerCompactionResponse {

crates/log-server/src/rocksdb_logstore/builder.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ use restate_rocksdb::{
2020
use restate_serde_util::ByteCount;
2121
use restate_types::config::{Configuration, LogServerOptions};
2222
use restate_types::health::HealthStatus;
23-
use restate_types::protobuf::common::LogServerStatus;
23+
use restate_types::protobuf::common::{DatabaseKind, LogServerStatus};
2424

2525
use super::writer::LogStoreWriterBuilder;
26-
use super::{DATA_CF, DB_NAME, METADATA_CF};
26+
use super::{DATA_CF, METADATA_CF};
2727
use super::{RocksDbLogStore, RocksDbLogStoreError};
2828
use crate::logstore::LogStoreState;
2929
use crate::rocksdb_logstore::keys::KeyPrefix;
@@ -36,12 +36,14 @@ pub struct RocksDbLogStoreBuilder {
3636

3737
impl RocksDbLogStoreBuilder {
3838
pub async fn create() -> Result<Self, RocksDbLogStoreError> {
39-
let db_name = DbName::new(DB_NAME);
39+
let kind = DatabaseKind::LogServer;
40+
let db_name = DbName::new(kind.db_name());
4041
let db_manager = RocksDbManager::get();
4142
let cfs = vec![CfName::new(DATA_CF), CfName::new(METADATA_CF)];
4243

4344
let db_spec = DbSpecBuilder::new(
4445
db_name,
46+
kind,
4547
Configuration::pinned().log_server.data_dir(),
4648
RocksConfigurator,
4749
)

crates/log-server/src/rocksdb_logstore/mod.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,5 @@ pub use self::builder::RocksDbLogStoreBuilder;
2929
pub use self::store::RocksDbLogStore;
3030
pub(crate) use error::*;
3131

32-
// matches the default directory name
33-
const DB_NAME: &str = "log-server";
3432
pub const DATA_CF: &str = "data";
3533
pub const METADATA_CF: &str = "metadata";

crates/metadata-server/src/raft/storage/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,5 @@ mod rocksdb_builder;
1515
pub use rocksdb::{BuildError, Error, RocksDbStorage};
1616

1717
const DATA_DIR: &str = "replicated-metadata-server";
18-
const DB_NAME: &str = "replicated-metadata-server";
1918
const DATA_CF: &str = "data";
2019
const METADATA_CF: &str = "metadata";

crates/metadata-server/src/raft/storage/rocksdb_builder.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,21 @@ use restate_rocksdb::{
1818
RocksError,
1919
};
2020
use restate_types::config::{Configuration, MetadataServerOptions, data_dir};
21+
use restate_types::protobuf::common::DatabaseKind;
2122

22-
use crate::raft::storage::{DATA_CF, DATA_DIR, DB_NAME, METADATA_CF};
23+
use crate::raft::storage::{DATA_CF, DATA_DIR, METADATA_CF};
2324

2425
const DATA_CF_BUDGET_RATIO: f64 = 0.85;
2526
const_assert!(DATA_CF_BUDGET_RATIO < 1.0);
2627

2728
pub async fn build_rocksdb() -> Result<Arc<RocksDb>, RocksError> {
29+
let kind = DatabaseKind::MetadataServer;
2830
let data_dir = data_dir(DATA_DIR);
29-
let db_name = DbName::new(DB_NAME);
31+
let db_name = DbName::new(kind.db_name());
3032
let db_manager = RocksDbManager::get();
3133
let cfs = vec![CfName::new(DATA_CF), CfName::new(METADATA_CF)];
3234

33-
let db_spec = DbSpecBuilder::new(db_name, data_dir, RocksConfigurator)
35+
let db_spec = DbSpecBuilder::new(db_name, kind, data_dir, RocksConfigurator)
3436
.add_cf_pattern(CfPrefixPattern::new(DATA_CF), RocksConfigurator)
3537
.add_cf_pattern(CfPrefixPattern::new(METADATA_CF), RocksConfigurator)
3638
// not very important but it's to reduce the number of merges by flushing.

crates/node/src/network_server/grpc_svc_handler.rs

Lines changed: 12 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use restate_types::net::connect_opts::GrpcConnectionOptions;
2828
use restate_core::network::net_util::{DNSResolution, create_tonic_channel};
2929
use restate_core::protobuf::node_ctl_svc::node_ctl_svc_server::{NodeCtlSvc, NodeCtlSvcServer};
3030
use restate_core::protobuf::node_ctl_svc::{
31-
ClusterHealthResponse, DatabaseCompactionResult, DatabaseKind, EmbeddedMetadataClusterHealth,
31+
ClusterHealthResponse, DatabaseCompactionResult, EmbeddedMetadataClusterHealth,
3232
GetMetadataRequest, GetMetadataResponse, IdentResponse, ProvisionClusterRequest,
3333
ProvisionClusterResponse, TriggerCompactionRequest, TriggerCompactionResponse,
3434
};
@@ -43,28 +43,12 @@ use restate_types::logs::metadata::{NodeSetSize, ProviderConfiguration};
4343
use restate_types::metadata::VersionedValue;
4444
use restate_types::nodes_config::Role;
4545
use restate_types::protobuf::cluster::ClusterConfiguration as ProtoClusterConfiguration;
46+
use restate_types::protobuf::common::DatabaseKind;
4647
use restate_types::replication::ReplicationProperty;
4748
use restate_types::storage::StorageCodec;
4849

4950
use crate::{ClusterConfiguration, provision_cluster_metadata};
5051

51-
/// Maps database names to their corresponding DatabaseKind.
52-
/// Returns None for unrecognized names so they are skipped rather than
53-
/// accidentally compacted if new databases are added without updating this mapping.
54-
fn db_name_to_kind(name: &str) -> Option<DatabaseKind> {
55-
if name == "log-server" {
56-
Some(DatabaseKind::LogServer)
57-
} else if name == "replicated-metadata-server" {
58-
Some(DatabaseKind::MetadataServer)
59-
} else if name == "local-loglet" {
60-
Some(DatabaseKind::LocalLoglet)
61-
} else if name == "db" || name.starts_with("db-") {
62-
Some(DatabaseKind::PartitionStore)
63-
} else {
64-
None
65-
}
66-
}
67-
6852
pub struct NodeCtlSvcHandler {
6953
metadata_writer: MetadataWriter,
7054
}
@@ -306,12 +290,10 @@ impl NodeCtlSvc for NodeCtlSvcHandler {
306290
// concurrent I/O from multiple databases.
307291
for db in all_dbs {
308292
let db_name = db.name().to_string();
293+
let kind = db.kind();
309294

310-
// Check if this database should be compacted. Skip databases whose names
311-
// are not recognized so newly added databases are not accidentally compacted.
312-
let should_compact = compact_all || {
313-
db_name_to_kind(&db_name).is_some_and(|kind| requested_kinds.contains(&kind))
314-
};
295+
let should_compact = compact_all
296+
|| (kind != DatabaseKind::Unspecified && requested_kinds.contains(&kind));
315297

316298
if !should_compact {
317299
continue;
@@ -471,27 +453,16 @@ fn write_err_to_status(err: WriteError) -> Status {
471453

472454
#[cfg(test)]
473455
mod tests {
474-
use super::*;
456+
use restate_types::protobuf::common::DatabaseKind;
475457

476458
#[test]
477-
fn test_db_name_to_kind() {
478-
assert_eq!(db_name_to_kind("db"), Some(DatabaseKind::PartitionStore));
479-
assert_eq!(db_name_to_kind("db-0"), Some(DatabaseKind::PartitionStore));
480-
assert_eq!(
481-
db_name_to_kind("db-123"),
482-
Some(DatabaseKind::PartitionStore)
483-
);
484-
assert_eq!(db_name_to_kind("log-server"), Some(DatabaseKind::LogServer));
485-
assert_eq!(
486-
db_name_to_kind("replicated-metadata-server"),
487-
Some(DatabaseKind::MetadataServer)
488-
);
459+
fn test_database_kind_db_names() {
460+
assert_eq!(DatabaseKind::LogServer.db_name(), "log-server");
489461
assert_eq!(
490-
db_name_to_kind("local-loglet"),
491-
Some(DatabaseKind::LocalLoglet)
462+
DatabaseKind::MetadataServer.db_name(),
463+
"replicated-metadata-server"
492464
);
493-
// Unknown names return None so they are safely skipped
494-
assert_eq!(db_name_to_kind("unknown"), None);
495-
assert_eq!(db_name_to_kind("random-name"), None);
465+
assert_eq!(DatabaseKind::LocalLoglet.db_name(), "local-loglet");
466+
assert_eq!(DatabaseKind::PartitionStore.db_name(), "db");
496467
}
497468
}

crates/partition-store/src/partition_store_manager.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use restate_types::config::Configuration;
2222
use restate_types::identifiers::{PartitionId, SnapshotId};
2323
use restate_types::logs::{Lsn, SequenceNumber};
2424
use restate_types::partitions::Partition;
25+
use restate_types::protobuf::common::DatabaseKind;
2526

2627
use crate::SnapshotError;
2728
use crate::memory::MemoryController;
@@ -164,6 +165,7 @@ impl PartitionStoreManager {
164165

165166
let db_spec = DbSpecBuilder::new(
166167
db_name.clone(),
168+
DatabaseKind::PartitionStore,
167169
Configuration::pinned().worker.storage.data_dir(&db_name),
168170
configurator.clone(),
169171
)

crates/rocksdb/src/db_spec.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::path::PathBuf;
1212

1313
use derive_builder::Builder;
1414

15+
use restate_types::protobuf::common::DatabaseKind;
1516
use restate_util_string::ReString;
1617

1718
use crate::BoxedCfMatcher;
@@ -145,6 +146,7 @@ pub enum OpenMode {
145146
#[builder(pattern = "owned", build_fn(name = "build"))]
146147
pub struct DbSpec {
147148
pub(crate) name: DbName,
149+
pub(crate) kind: DatabaseKind,
148150
pub(crate) path: PathBuf,
149151
/// All column families that should be flushed on shutdown, no flush will be performed if empty
150152
/// which should be the default for most cases.
@@ -174,6 +176,10 @@ impl DbSpec {
174176
&self.name
175177
}
176178

179+
pub fn kind(&self) -> DatabaseKind {
180+
self.kind
181+
}
182+
177183
pub fn open_mode(&self) -> OpenMode {
178184
self.db_configurator.get_db_open_mode()
179185
}
@@ -182,11 +188,13 @@ impl DbSpec {
182188
impl DbSpecBuilder {
183189
pub fn new(
184190
name: DbName,
191+
kind: DatabaseKind,
185192
path: PathBuf,
186193
configurator: impl DbConfigurator + Send + Sync + 'static,
187194
) -> DbSpecBuilder {
188195
Self {
189196
name: Some(name),
197+
kind: Some(kind),
190198
path: Some(path),
191199
db_configurator: Some(Box::new(configurator)),
192200
..Self::default()

crates/rocksdb/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ use rocksdb::statistics::HistogramData;
3535
use rocksdb::statistics::Ticker;
3636

3737
use restate_core::ShutdownError;
38+
use restate_types::protobuf::common::DatabaseKind;
3839

3940
// re-exports
4041
pub use self::db_manager::RocksDbManager;
@@ -112,6 +113,10 @@ impl RocksDb {
112113
&self.db.spec().name
113114
}
114115

116+
pub fn kind(&self) -> DatabaseKind {
117+
self.db.spec().kind
118+
}
119+
115120
pub(crate) async fn open(
116121
manager: &'static RocksDbManager,
117122
spec: DbSpec,

0 commit comments

Comments
 (0)