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

[1/n][Release] Renames and configuration updates #2566

Closed
wants to merge 2 commits into from
Closed
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
2 changes: 1 addition & 1 deletion benchmarks/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub fn flamegraph_options<'a>() -> Options<'a> {
pub fn restate_configuration() -> Configuration {
let common_options = CommonOptionsBuilder::default()
.base_dir(tempfile::tempdir().expect("tempdir failed").into_path())
.bootstrap_num_partitions(NonZeroU16::new(10).unwrap())
.default_num_partitions(NonZeroU16::new(10).unwrap())
.build()
.expect("building common options should work");

Expand Down
2 changes: 1 addition & 1 deletion crates/local-cluster-runner/examples/three_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ async fn main() -> anyhow::Result<()> {
let mut base_config = Configuration::default();
base_config.common.log_format = LogFormat::Compact;
base_config.common.log_filter = "warn,restate=debug".to_string();
base_config.common.bootstrap_num_partitions = NonZeroU16::new(4).unwrap();
base_config.common.default_num_partitions = NonZeroU16::new(4).unwrap();
base_config.bifrost.default_provider = Replicated;

let nodes = Node::new_test_nodes(
Expand Down
4 changes: 2 additions & 2 deletions crates/local-cluster-runner/src/node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl Node {
) -> Vec<Self> {
let mut nodes = Vec::with_capacity(usize::try_from(size).expect("u32 to fit into usize"));

base_config.common.allow_bootstrap = false;
base_config.common.auto_provision = false;
base_config.common.log_disable_ansi_codes = true;
if !matches!(
base_config.metadata_server.kind,
Expand All @@ -197,7 +197,7 @@ impl Node {

if auto_provision && node_id == 1 {
// the first node will be responsible for bootstrapping the cluster
effective_config.common.allow_bootstrap = true;
effective_config.common.auto_provision = true;
}

// Create a separate ingress role when running a worker
Expand Down
4 changes: 2 additions & 2 deletions crates/node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ impl Node {
)?;
}

if config.common.allow_bootstrap {
if config.common.auto_provision {
TaskCenter::spawn(TaskKind::SystemBoot, "auto-provision-cluster", {
let cluster_configuration = ClusterConfiguration::from_configuration(&config);
let metadata_store_client = self.metadata_store_client.clone();
Expand Down Expand Up @@ -561,7 +561,7 @@ fn num_partitions_to_u32(num_partitions: NonZeroU16) -> u32 {
impl ClusterConfiguration {
pub fn from_configuration(configuration: &Configuration) -> Self {
ClusterConfiguration {
num_partitions: configuration.common.bootstrap_num_partitions,
num_partitions: configuration.common.default_num_partitions,
partition_replication: configuration.admin.default_partition_replication.clone(),
bifrost_provider: ProviderConfiguration::from_configuration(configuration),
}
Expand Down
4 changes: 2 additions & 2 deletions crates/node/src/network_server/grpc_svc_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl NodeCtlSvcHandler {
})
})
.transpose()?
.unwrap_or(config.common.bootstrap_num_partitions);
.unwrap_or(config.common.default_num_partitions);
let partition_replication = request.partition_replication.try_into()?;

let log_provider = request
Expand All @@ -102,7 +102,7 @@ impl NodeCtlSvcHandler {
config
.bifrost
.replicated_loglet
.default_replication_property
.default_log_replication
.clone()
});

Expand Down
12 changes: 9 additions & 3 deletions crates/types/src/config/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,16 @@ pub struct AdminOptions {
#[cfg_attr(feature = "schemars", schemars(with = "String"))]
pub log_tail_update_interval: humantime::Duration,

/// # Default partition replication strategy
/// # Default partition replication factor
///
/// The default partition replication strategy to be used by the cluster controller to place partition
/// processors.
/// [__PREVIEW FEATURE__]
/// The default replication factor for partition processors, this impacts how many replicas
/// each partition will have across the worker nodes of the cluster.
///
/// Note that this value only impacts the cluster initial provisioning and will not be respected after
/// the cluster has been provisioned.
///
/// To update existing clusters use the `restatectl` utility.
#[cfg_attr(feature = "schemars", schemars(with = "String"))]
pub default_partition_replication: PartitionReplication,

Expand Down
20 changes: 14 additions & 6 deletions crates/types/src/config/bifrost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,23 +284,31 @@ pub struct ReplicatedLogletOptions {
/// Value must be between 0 and 1. It will be clamped at `1.0`.
pub readahead_trigger_ratio: f32,

/// # Default replication property
/// # Default log replication factor
///
/// Configures the default replication property to be used by the replicated loglet.
/// Configures the default replication factor to be used by the replicated loglets.
///
/// Note that this value only impacts the cluster initial provisioning and will not be respected after
/// the cluster has been provisioned.
///
/// To update existing clusters use the `restatectl` utility.
// Also allow to specify the replication property as non-zero u8 value to make it simpler to
// pass it in via an env variable.
#[serde_as(
as = "serde_with::PickFirst<(serde_with::DisplayFromStr, ReplicationPropertyFromNonZeroU8)>"
)]
#[serde(default = "default_replication_property")]
#[serde(default = "default_log_replication")]
#[cfg_attr(feature = "schemars", schemars(with = "String"))]
pub default_replication_property: ReplicationProperty,
pub default_log_replication: ReplicationProperty,

/// # Default nodeset size
///
/// Configures the target nodeset size used by the replicated loglet when generating new
/// nodesets for logs. Setting this to 0 will let the system choose a reasonable value based on
/// the effective replication_property at the time of logs reconfiguration.
///
/// Note that this value only impacts the cluster initial provisioning and will not be respected after
/// the cluster has been provisioned.
// hide the configuration option from serialization if it is the default
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe include the same callout here, too?

Suggested change
// hide the configuration option from serialization if it is the default
///
/// To update existing clusters use the `restatectl` utility.
// hide the configuration option from serialization if it is the default

Copy link
Contributor

Choose a reason for hiding this comment

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

Will do.

#[serde(default, skip_serializing_if = "nodeset_size_is_zero")]
// hide the configuration option by excluding it from the Json schema
Expand All @@ -324,7 +332,7 @@ fn nodeset_size_is_zero(i: &NodeSetSize) -> bool {
*i == NodeSetSize::ZERO
}

fn default_replication_property() -> ReplicationProperty {
fn default_log_replication() -> ReplicationProperty {
ReplicationProperty::new(NonZeroU8::new(1).expect("to be non-zero"))
}

Expand All @@ -349,8 +357,8 @@ impl Default for ReplicatedLogletOptions {
),
readahead_records: NonZeroUsize::new(100).unwrap(),
readahead_trigger_ratio: 0.5,
default_replication_property: default_replication_property(),
default_nodeset_size: NodeSetSize::default(),
default_log_replication: default_log_replication(),
}
}
}
53 changes: 31 additions & 22 deletions crates/types/src/config/cli_option_overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub struct CommonOptionCliOverride {

/// Node location
///
/// [PREVIEW FEATURE]
/// Setting the location allows Restate to form a tree-like cluster topology.
/// The value is written in the format of "<region>[.zone]" to assign this node
/// to a specific region, or to a zone within a region.
Expand Down Expand Up @@ -74,10 +75,21 @@ pub struct CommonOptionCliOverride {
#[clap(long, env = "RESTATE_CLUSTER_NAME", global = true)]
pub cluster_name: Option<String>,

/// If true, then a new cluster is bootstrapped. This node *must* have an admin
/// role and a new nodes configuration will be created that includes this node.
#[clap(long, global = true)]
pub allow_bootstrap: Option<bool>,
/// Auto Cluster Provisioning
///
/// If true, then this node is allowed to automatically provision as a new cluster.
/// This node *must* have an admin role and a new nodes configuration will be created that includes this node.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is no longer required that the node that provisions the cluster runs the admin role.

///
/// auto-provision is allowed by default in development mode and is disabled if restate-server runs with `--production` flag
/// to prevent cluster nodes from forming their own clusters, rather than forming a single cluster.
///
/// Use `restatectl` to provision the cluster/node if automatic provisioning is disabled.
///
/// This can also be explicitly disabled by setting this value to false.
///
/// Default: true
#[clap(long, global = true, alias = "allow-bootstrap")]
pub auto_provision: Option<bool>,

/// The working directory which this Restate node should use for relative paths. The default is
/// `restate-data` under the current working directory.
Expand All @@ -97,24 +109,21 @@ pub struct CommonOptionCliOverride {
#[clap(long, global = true)]
pub advertise_address: Option<AdvertisedAddress>,

/// # Partitions
/// Default Number Of Partitions
///
/// Number of partitions that will be provisioned during initial cluster provisioning.
/// partitions are the logical shards used to process messages. Default is 24.
///
/// Number of partitions that will be provisioned during cluster bootstrap,
/// partitions used to process messages.
/// Cannot be higher than `65535` (You should almost never need as many partitions anyway)
///
/// NOTE: This config entry only impacts the initial number of partitions, the
/// NOTE 1: This config entry only impacts the initial number of partitions, the
/// value of this entry is ignored for bootstrapped nodes/clusters.
///
/// Cannot be higher than `4611686018427387903` (You should almost never need as many partitions anyway)
#[clap(long, global = true)]
pub bootstrap_num_partitions: Option<NonZeroU64>,

/// # Automatically provision number of configured partitions
/// NOTE 2: This will be renamed to `default-num-partitions` by default as of v1.3+
///
/// If this option is set to `false`, then one needs to manually write a partition table to
/// the metadata store. Without a partition table, the cluster will not start.
#[clap(long, global = true)]
pub auto_provision_partitions: Option<bool>,
/// Default: 24
#[clap(long, global = true, alias = "bootstrap-num-partitions")]
pub default_num_partitions: Option<NonZeroU64>,
Copy link
Contributor

@pcholakov pcholakov Jan 29, 2025

Choose a reason for hiding this comment

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

This breaks in a weird way when set via an env variable:

❯ RESTATE_BOOTSTRAP_NUM_PARTITIONS=1 restate-server
[RT0002] configuration loading error: duplicate field `default-num-partitions`

Copy link
Contributor

Choose a reason for hiding this comment

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

Update: rebuilt from the PR branch and tested again, this no longer happens. Both of these work as expected:

RESTATE_DEFAULT_NUM_PARTITIONS=1 restate-server
RESTATE_BOOTSTRAP_NUM_PARTITIONS=1 restate-server


/// This timeout is used when shutting down the various Restate components to drain all the internal queues.
#[serde_as(as = "Option<serde_with::DisplayFromStr>")]
Expand Down Expand Up @@ -153,7 +162,7 @@ pub struct CommonOptionCliOverride {
#[clap(long, env = "RESTATE_TRACING_SERVICES_ENDPOINT", global = true)]
pub tracing_services_endpoint: Option<String>,

/// # Distributed Tracing JSON Export Path
/// Distributed Tracing JSON Export Path
///
/// If set, an exporter will be configured to write traces to files using the Jaeger JSON format.
/// Each trace file will start with the `trace` prefix.
Expand All @@ -166,27 +175,27 @@ pub struct CommonOptionCliOverride {
#[clap(long, global = true)]
pub tracing_json_path: Option<PathBuf>,

/// # Tracing Filter
/// Tracing Filter
///
/// Distributed tracing exporter filter.
/// Check the [`RUST_LOG` documentation](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html) for more details how to configure it.
#[clap(long, global = true)]
pub tracing_filter: Option<String>,

/// # Logging Filter
/// Logging Filter
///
/// Log filter configuration. Can be overridden by the `RUST_LOG` environment variable.
/// Check the [`RUST_LOG` documentation](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html) for more details how to configure it.
#[clap(long, global = true)]
pub log_filter: Option<String>,

/// # Logging format
/// Logging format
///
/// Format to use when logging.
#[clap(long, global = true)]
pub log_format: Option<LogFormat>,

/// # Disable ANSI in log output
/// Disable ANSI in log output
///
/// Disable ANSI terminal codes for logs. This is useful when the log collector doesn't support processing ANSI terminal codes.
#[clap(long, global = true)]
Expand Down
Loading
Loading