Skip to content

Commit

Permalink
Disable log-server role by default to fix v1.0.x compatibility
Browse files Browse the repository at this point in the history
This change disables the log-server role by default to avoid breaking. Additionally, if log-server role is enabled, log-server will not start unless the binary is built with `-F replicated-loglet` to protect the public release from starting the server. This change is necessary to maintain compatibility with v1.0.x.

This change also attempts to detect the presence of log-store and asks the user to remove it manually. We don't remove it automatically to avoid accidental data wipes if accidental rollbacks took place from future versions back to this.
  • Loading branch information
AhmedSoliman committed Sep 18, 2024
1 parent be0b332 commit cf424f8
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 4 deletions.
3 changes: 2 additions & 1 deletion crates/node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ publish = false
[features]
default = []
memory-loglet = ["restate-bifrost/memory-loglet"]
replicated-loglet = ["restate-bifrost/replicated-loglet"]
options_schema = [
"dep:schemars",
"restate-admin/options_schema",
Expand All @@ -18,7 +19,7 @@ options_schema = [

[dependencies]
restate-admin = { workspace = true, features = ["servers"] }
restate-bifrost = { workspace = true, features = ["replicated-loglet"] }
restate-bifrost = { workspace = true }
restate-core = { workspace = true }
restate-errors = { workspace = true }
restate-log-server = { workspace = true }
Expand Down
24 changes: 22 additions & 2 deletions crates/node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use restate_core::{
spawn_metadata_manager, MetadataBuilder, MetadataKind, MetadataManager, TargetVersion,
};
use restate_core::{task_center, TaskKind};
#[cfg(feature = "replicated-loglet")]
use restate_log_server::LogServerService;
use restate_metadata_store::local::LocalMetadataStoreService;
use restate_metadata_store::MetadataStoreClient;
Expand Down Expand Up @@ -108,6 +109,7 @@ pub struct Node {
metadata_store_role: Option<LocalMetadataStoreService>,
admin_role: Option<AdminRole>,
worker_role: Option<WorkerRole>,
#[cfg(feature = "replicated-loglet")]
log_server: Option<LogServerService>,
server: NetworkServer,
}
Expand Down Expand Up @@ -164,6 +166,7 @@ impl Node {

// Setup bifrost
// replicated-loglet
#[cfg(feature = "replicated-loglet")]
let replicated_loglet_factory = restate_bifrost::providers::replicated_loglet::Factory::new(
updateable_config
.clone()
Expand All @@ -175,12 +178,18 @@ impl Node {
&mut router_builder,
);
let bifrost_svc = BifrostService::new(tc.clone(), metadata.clone())
.enable_local_loglet(&updateable_config)
.with_factory(replicated_loglet_factory);
.enable_local_loglet(&updateable_config);

#[cfg(feature = "replicated-loglet")]
let bifrost_svc = bifrost_svc.with_factory(replicated_loglet_factory);

#[cfg(feature = "memory-loglet")]
let bifrost_svc = bifrost_svc.enable_in_memory_loglet();

#[cfg(not(feature = "replicated-loglet"))]
warn_if_log_store_left_artifacts(&config);

#[cfg(feature = "replicated-loglet")]
let log_server = if config.has_role(Role::LogServer) {
Some(
LogServerService::create(
Expand Down Expand Up @@ -260,6 +269,7 @@ impl Node {
metadata_store_client,
admin_role,
worker_role,
#[cfg(feature = "replicated-loglet")]
log_server,
server,
})
Expand Down Expand Up @@ -375,6 +385,7 @@ impl Node {
tc.run_in_scope("bifrost-init", None, self.bifrost.start())
.await?;

#[cfg(feature = "replicated-loglet")]
if let Some(log_server) = self.log_server {
tc.spawn(
TaskKind::SystemBoot,
Expand Down Expand Up @@ -608,3 +619,12 @@ impl Node {
.await
}
}

#[cfg(not(feature = "replicated-loglet"))]
fn warn_if_log_store_left_artifacts(config: &Configuration) {
if config.log_server.data_dir().exists() {
tracing::warn!("Log server data directory '{}' exists, \
but log-server is not implemented in this version of restate-server. \
This may indicate that the log-server role was previously enabled and the data directory was not cleaned up. If this was created by v1.1.1 of restate-server, please remove this directory to avoid potential future conflicts.", config.log_server.data_dir().display());
}
}
22 changes: 21 additions & 1 deletion crates/types/src/config/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,13 @@ impl CommonOptions {
impl Default for CommonOptions {
fn default() -> Self {
Self {
roles: EnumSet::all(),
// todo (asoli): Remove this when:
// a. The safe rollback version supports log-server (at least supports parsing the
// config with the log-server role)
// b. When log-server becomes enabled by default.
//
// see "roles_compat_test" test below.
roles: EnumSet::all() - Role::LogServer,
node_name: None,
force_node_id: None,
cluster_name: "localcluster".to_owned(),
Expand Down Expand Up @@ -542,3 +548,17 @@ impl Default for TracingOptions {
}
}
}
#[cfg(test)]
mod tests {
use crate::nodes_config::Role;

use super::CommonOptions;

#[test]
fn roles_compat_test() {
let opts = CommonOptions::default();
// make sure we don't add log-server by default until previous version can parse nodes
// configuration with this role.
assert!(!opts.roles.contains(Role::LogServer));
}
}
1 change: 1 addition & 0 deletions server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ options_schema = [
"restate-types/schemars",
]
memory-loglet = ["restate-node/memory-loglet"]
replicated-loglet = ["restate-node/replicated-loglet"]

[dependencies]
restate-admin = { workspace = true }
Expand Down

0 comments on commit cf424f8

Please sign in to comment.