From 9c547da6a6bbad042710846589600f39518ce96e Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Sun, 5 May 2024 16:49:20 +0000 Subject: [PATCH] reduce scope of this PR & fix naming --- control_plane/src/bin/neon_local.rs | 8 ++--- docs/settings.md | 12 +++++-- pageserver/src/bin/pageserver.rs | 52 +++++++++++++++++++-------- test_runner/fixtures/neon_fixtures.py | 16 ++++----- 4 files changed, 59 insertions(+), 29 deletions(-) diff --git a/control_plane/src/bin/neon_local.rs b/control_plane/src/bin/neon_local.rs index c8f8ef3646..b747ccc4d5 100644 --- a/control_plane/src/bin/neon_local.rs +++ b/control_plane/src/bin/neon_local.rs @@ -359,7 +359,7 @@ fn handle_init(init_match: &ArgMatches) -> anyhow::Result { }; let pageserver_config_overrides: toml_edit::Document = - if let Some(path) = init_match.get_one::("pageserver-config-overrides-file") { + if let Some(path) = init_match.get_one::("pageserver-config") { std::fs::read_to_string(path) .context("load pageserver config overrides file")? .parse() @@ -1444,11 +1444,11 @@ fn cli() -> Command { .value_name("config"), ) .arg( - Arg::new("pageserver-config-overrides-file") - .long("pageserver-config-overrides-file") + Arg::new("pageserver-config") + .long("pageserver-config") .required(false) .value_parser(value_parser!(PathBuf)) - .value_name("pageserver-config-overrides-file"), + .value_name("pageserver-config"), ) .arg(pg_version_arg.clone()) .arg(force_arg) diff --git a/docs/settings.md b/docs/settings.md index 071f7686b9..817f97d8ba 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -40,10 +40,18 @@ 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 @@ -150,7 +158,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`). +This parameter has a special CLI alias (`-D`) and can not be overridden with regular `-c` way. ##### Remote storage diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index 1bd7fac343..eb4b8bb8bb 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -6,7 +6,7 @@ use std::env::{var, VarError}; use std::io::Read; use std::sync::Arc; use std::time::Duration; -use std::{env, ops::ControlFlow}; +use std::{env, ops::ControlFlow, str::FromStr}; use anyhow::{anyhow, Context}; use camino::Utf8Path; @@ -153,8 +153,7 @@ fn initialize_config( ) -> anyhow::Result> { let init = arg_matches.get_flag("init"); - // 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) { + let file_contents: Option = match std::fs::File::open(cfg_file_path) { Ok(mut f) => { if init { anyhow::bail!("config file already exists: {cfg_file_path}"); @@ -163,33 +162,46 @@ fn initialize_config( if md.is_file() { let mut s = String::new(); f.read_to_string(&mut s).context("read config file")?; - s.parse().context("parse config file toml")? + Some(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 => { - if init { - DEFAULT_CONFIG_FILE - .parse() - .expect("unit tests ensure this works") - } else { - anyhow::bail!("config file not found: {cfg_file_path}"); - } - } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, 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(&config, workdir) + let conf = PageServerConf::parse_and_validate(&effective_config, workdir) .context("Failed to parse pageserver configuration")?; if init { info!("Writing pageserver config to '{cfg_file_path}'"); - std::fs::write(cfg_file_path, config.to_string()) + std::fs::write(cfg_file_path, effective_config.to_string()) .with_context(|| format!("Failed to write pageserver config to '{cfg_file_path}'"))?; info!("Config successfully written to '{cfg_file_path}'") } @@ -740,6 +752,16 @@ 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") + .long("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/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index dbb9fa9bd3..684d6bd2e5 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -1711,28 +1711,28 @@ class NeonCli(AbstractNeonCli): ) -> "subprocess.CompletedProcess[str]": remote_storage = self.env.pageserver_remote_storage - ps_toml = {} + ps_config = {} if remote_storage is not None: remote_storage_toml_table = remote_storage_to_toml_inline_table(remote_storage) - ps_toml["remote_storage"] = remote_storage_toml_table + ps_config["remote_storage"] = remote_storage_toml_table env_overrides = os.getenv("NEON_PAGESERVER_OVERRIDES") if env_overrides is not None: for o in env_overrides.split(";"): override = toml.loads(o) for key, value in override.items(): - ps_toml[key] = value + ps_config[key] = value if pageserver_config_override is not None: for o in pageserver_config_override.split(";"): override = toml.loads(o) for key, value in override.items(): - ps_toml[key] = value + ps_config[key] = value with ExitStack() as stack: - ps_toml_file = stack.enter_context(tempfile.NamedTemporaryFile(mode="w+")) - ps_toml_file.write(toml.dumps(ps_toml)) - ps_toml_file.flush() + ps_config_file = stack.enter_context(tempfile.NamedTemporaryFile(mode="w+")) + ps_config_file.write(toml.dumps(ps_config)) + ps_config_file.flush() neon_local_config = stack.enter_context(tempfile.NamedTemporaryFile(mode="w+")) neon_local_config.write(toml.dumps(config)) @@ -1743,7 +1743,7 @@ class NeonCli(AbstractNeonCli): f"--config={neon_local_config.name}", "--pg-version", self.env.pg_version, - f"--pageserver-config-overrides-file={ps_toml_file.name}", + f"--pageserver-config={ps_config_file.name}", ] if force is not None: