diff --git a/control_plane/src/bin/neon_local.rs b/control_plane/src/bin/neon_local.rs index 14b83c1252..a307e7cceb 100644 --- a/control_plane/src/bin/neon_local.rs +++ b/control_plane/src/bin/neon_local.rs @@ -375,7 +375,7 @@ fn handle_init(init_match: &ArgMatches) -> anyhow::Result { // Initialize pageserver, create initial tenant and timeline. for ps_conf in &env.pageservers { PageServerNode::from_env(&env, ps_conf) - .initialize(&pageserver_config_overrides(init_match)) + .initialize() .unwrap_or_else(|e| { eprintln!("pageserver init failed: {e:?}"); exit(1); @@ -397,15 +397,6 @@ fn get_default_pageserver(env: &local_env::LocalEnv) -> PageServerNode { PageServerNode::from_env(env, ps_conf) } -fn pageserver_config_overrides(init_match: &ArgMatches) -> Vec<&str> { - init_match - .get_many::("pageserver-config-override") - .into_iter() - .flatten() - .map(String::as_str) - .collect() -} - async fn handle_tenant( tenant_match: &ArgMatches, env: &mut local_env::LocalEnv, @@ -1068,10 +1059,7 @@ fn get_pageserver(env: &local_env::LocalEnv, args: &ArgMatches) -> Result Result<()> { match sub_match.subcommand() { Some(("start", subcommand_args)) => { - if let Err(e) = get_pageserver(env, subcommand_args)? - .start(&pageserver_config_overrides(subcommand_args)) - .await - { + if let Err(e) = get_pageserver(env, subcommand_args)?.start().await { eprintln!("pageserver start failed: {e}"); exit(1); } @@ -1244,10 +1232,7 @@ async fn handle_start_all(sub_match: &ArgMatches, env: &local_env::LocalEnv) -> for ps_conf in &env.pageservers { let pageserver = PageServerNode::from_env(env, ps_conf); - if let Err(e) = pageserver - .start(&pageserver_config_overrides(sub_match)) - .await - { + if let Err(e) = pageserver.start().await { eprintln!("pageserver {} start failed: {:#}", ps_conf.id, e); try_stop_all(env, true).await; exit(1); @@ -1388,13 +1373,6 @@ fn cli() -> Command { .required(false) .value_name("stop-mode"); - let pageserver_config_args = Arg::new("pageserver-config-override") - .long("pageserver-config-override") - .num_args(1) - .action(ArgAction::Append) - .help("Additional pageserver's configuration options or overrides, refer to pageserver's 'config-override' CLI parameter docs for more") - .required(false); - let remote_ext_config_args = Arg::new("remote-ext-config") .long("remote-ext-config") .num_args(1) diff --git a/control_plane/src/pageserver.rs b/control_plane/src/pageserver.rs index c0a366e3b9..958f873b6a 100644 --- a/control_plane/src/pageserver.rs +++ b/control_plane/src/pageserver.rs @@ -77,7 +77,7 @@ impl PageServerNode { /// Merge overrides provided by the user on the command line with our default overides derived from neon_local configuration. /// /// These all end up on the command line of the `pageserver` binary. - fn neon_local_overrides(&self, cli_overrides: &[&str]) -> Vec { + fn neon_local_overrides(&self) -> Vec { // FIXME: the paths should be shell-escaped to handle paths with spaces, quotas etc. let pg_distrib_dir_param = format!( "pg_distrib_dir='{}'", @@ -179,9 +179,9 @@ impl PageServerNode { } /// Initializes a pageserver node by creating its config with the overrides provided. - pub fn initialize(&self, config_overrides: &[&str]) -> anyhow::Result<()> { + pub fn initialize(&self) -> anyhow::Result<()> { // First, run `pageserver --init` and wait for it to write a config into FS and exit. - self.pageserver_init(config_overrides) + self.pageserver_init() .with_context(|| format!("Failed to run init for pageserver node {}", self.conf.id)) } @@ -197,11 +197,11 @@ impl PageServerNode { .expect("non-Unicode path") } - pub async fn start(&self, config_overrides: &[&str]) -> anyhow::Result<()> { - self.start_node(config_overrides).await + pub async fn start(&self) -> anyhow::Result<()> { + self.start_node().await } - fn pageserver_init(&self, config_overrides: &[&str]) -> anyhow::Result<()> { + fn pageserver_init(&self) -> anyhow::Result<()> { let datadir = self.repo_path(); let node_id = self.conf.id; println!( @@ -219,7 +219,7 @@ impl PageServerNode { let datadir_path_str = datadir.to_str().with_context(|| { format!("Cannot start pageserver node {node_id} in path that has no string representation: {datadir:?}") })?; - let mut args = self.pageserver_basic_args(config_overrides, datadir_path_str); + let mut args = self.pageserver_basic_args(datadir_path_str); args.push(Cow::Borrowed("--init")); let init_output = Command::new(self.env.pageserver_bin()) @@ -262,7 +262,7 @@ impl PageServerNode { Ok(()) } - async fn start_node(&self, config_overrides: &[&str]) -> anyhow::Result<()> { + async fn start_node(&self) -> anyhow::Result<()> { // TODO: using a thread here because start_process() is not async but we need to call check_status() let datadir = self.repo_path(); print!( @@ -279,7 +279,7 @@ impl PageServerNode { self.conf.id, datadir, ) })?; - let args = self.pageserver_basic_args(config_overrides, datadir_path_str); + let args = self.pageserver_basic_args(datadir_path_str); background_process::start_process( "pageserver", &datadir, @@ -301,14 +301,10 @@ impl PageServerNode { Ok(()) } - fn pageserver_basic_args<'a>( - &self, - config_overrides: &'a [&'a str], - datadir_path_str: &'a str, - ) -> Vec> { + fn pageserver_basic_args<'a>(&self, datadir_path_str: &'a str) -> Vec> { let mut args = vec![Cow::Borrowed("-D"), Cow::Borrowed(datadir_path_str)]; - let overrides = self.neon_local_overrides(config_overrides); + let overrides = self.neon_local_overrides(); for config_override in overrides { args.push(Cow::Borrowed("-c")); args.push(Cow::Owned(config_override)); diff --git a/docs/settings.md b/docs/settings.md index 817f97d8ba..071f7686b9 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -40,18 +40,10 @@ Note the `[remote_storage]` section: it's a [table](https://toml.io/en/v1.0.0#ta - or can be placed anywhere if rewritten in identical form as [inline table](https://toml.io/en/v1.0.0#inline-table): `remote_storage = {foo = 2}` -### Config values - -All values can be passed as an argument to the pageserver binary, using the `-c` parameter and specified as a valid TOML string. All tables should be passed in the inline form. - -Example: `${PAGESERVER_BIN} -c "checkpoint_timeout = '10 m'" -c "remote_storage={local_path='/some/local/path/'}"` - -Note that TOML distinguishes between strings and integers, the former require single or double quotes around them. - #### broker_endpoint A storage broker endpoint to connect and pull the information from. Default is -`'http://127.0.0.1:50051'`. +`'http://127.0.0.1:50051'`. #### checkpoint_distance @@ -158,7 +150,7 @@ The default distrib dir is `./pg_install/`. A directory in the file system, where pageserver will store its files. The default is `./.neon/`. -This parameter has a special CLI alias (`-D`) and can not be overridden with regular `-c` way. +This parameter has a special CLI alias (`-D`). ##### Remote storage diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index e9433de05b..c71f436cf3 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -153,7 +153,8 @@ fn initialize_config( ) -> anyhow::Result> { let init = arg_matches.get_flag("init"); - let file_contents: Option = match std::fs::File::open(cfg_file_path) { + // Load the config file from disk or use the default config if in init mode. + let config: toml_edit::Document = match std::fs::File::open(cfg_file_path) { Ok(mut f) => { if init { anyhow::bail!("config file already exists: {cfg_file_path}"); @@ -162,38 +163,25 @@ fn initialize_config( if md.is_file() { let mut s = String::new(); f.read_to_string(&mut s).context("read config file")?; - Some(s.parse().context("parse config file toml")?) + s.parse().context("parse config file toml")? } else { anyhow::bail!("directory entry exists but is not a file: {cfg_file_path}"); } } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + if init { + DEFAULT_CONFIG_FILE + .parse() + .expect("unit tests ensure this works") + } else { + anyhow::bail!("config file not found: {cfg_file_path}"); + } + } Err(e) => { anyhow::bail!("open pageserver config: {e}: {cfg_file_path}"); } }; - let mut effective_config = file_contents.unwrap_or_else(|| { - DEFAULT_CONFIG_FILE - .parse() - .expect("unit tests ensure this works") - }); - - // Patch with overrides from the command line - if let Some(values) = arg_matches.get_many::("config-override") { - for option_line in values { - let doc = toml_edit::Document::from_str(option_line).with_context(|| { - format!("Option '{option_line}' could not be parsed as a toml document") - })?; - - for (key, item) in doc.iter() { - effective_config.insert(key, item.clone()); - } - } - } - - debug!("Resulting toml: {effective_config}"); - // Construct the runtime representation let conf = PageServerConf::parse_and_validate(&effective_config, workdir) .context("Failed to parse pageserver configuration")?; @@ -752,15 +740,6 @@ fn cli() -> Command { .long("workdir") .help("Working directory for the pageserver"), ) - // See `settings.md` for more details on the extra configuration patameters pageserver can process - .arg( - Arg::new("config-override") - .short('c') - .num_args(1) - .action(ArgAction::Append) - .help("Additional configuration overrides of the ones from the toml config file (or new ones to add there). \ - Any option has to be a valid toml document, example: `-c=\"foo='hey'\"` `-c=\"foo={value=1}\"`"), - ) .arg( Arg::new("enabled-features") .long("enabled-features") diff --git a/test_runner/README.md b/test_runner/README.md index 97062b4bd3..051897744a 100644 --- a/test_runner/README.md +++ b/test_runner/README.md @@ -80,8 +80,6 @@ should go. Useful parameters and commands: -`--pageserver-config-override=${value}` `-c` values to pass into pageserver through neon_local cli - `--preserve-database-files` to preserve pageserver (layer) and safekeer (segment) timeline files on disk after running a test suite. Such files might be large, so removed by default; but might be useful for debugging or creation of svg images with layer file contents.