From 2c683b38dee92d4d1292493a5c978df00eb13766 Mon Sep 17 00:00:00 2001 From: Anastasia Lubennikova Date: Tue, 28 Feb 2023 20:31:04 +0200 Subject: [PATCH] Change --replicates option to --hot-standby flag --- control_plane/src/bin/neon_local.rs | 68 +++++++++++++++++++---------- control_plane/src/compute.rs | 20 +++------ 2 files changed, 52 insertions(+), 36 deletions(-) diff --git a/control_plane/src/bin/neon_local.rs b/control_plane/src/bin/neon_local.rs index 76964e8f3b..1a6d0fcd14 100644 --- a/control_plane/src/bin/neon_local.rs +++ b/control_plane/src/bin/neon_local.rs @@ -473,7 +473,14 @@ fn handle_timeline(timeline_match: &ArgMatches, env: &mut local_env::LocalEnv) - println!("Creating node for imported timeline ..."); env.register_branch_mapping(name.to_string(), tenant_id, timeline_id)?; - cplane.new_node(tenant_id, name, timeline_id, None, None, pg_version, None)?; + cplane.new_node( + tenant_id, + name, + timeline_id, + None, + pg_version, + Replication::Primary, + )?; println!("Done"); } Some(("branch", branch_match)) => { @@ -617,16 +624,25 @@ fn handle_pg(pg_match: &ArgMatches, env: &local_env::LocalEnv) -> Result<()> { .copied() .context("Failed to parse postgres version from the argument string")?; - let replica = sub_args.get_one::("replicates").map(|s| s.as_str()); + let hot_standby = sub_args + .get_one::("hot-standby") + .copied() + .unwrap_or(false); + + let replication = match (lsn, hot_standby) { + (Some(lsn), false) => Replication::Static(lsn), + (None, true) => Replication::Replica, + (None, false) => Replication::Primary, + (Some(_), true) => anyhow::bail!("cannot specify both lsn and hot-standby"), + }; cplane.new_node( tenant_id, &node_name, timeline_id, - lsn, port, pg_version, - replica, + replication, )?; } "start" => { @@ -645,19 +661,20 @@ fn handle_pg(pg_match: &ArgMatches, env: &local_env::LocalEnv) -> Result<()> { None }; - let replica = sub_args.get_one::("replicates").map(|s| s.as_str()); + let hot_standby = sub_args + .get_one::("hot-standby") + .copied() + .unwrap_or(false); if let Some(node) = node { - if let Some(repl) = replica { - if let Replication::Replica(source) = &node.replication { - if source != repl { - println!("replication ignored: node already has replica configured") - } - } else { - println!( - "replication ignored: pre-existing node was not configured as replica" - ) + match (&node.replication, hot_standby) { + (Replication::Static(_), true) => { + bail!("Cannot start a node in hot standby mode when it is already configured as a static replica") } + (Replication::Primary, true) => { + bail!("Cannot start a node as a hot standby replica, it is already configured as primary node") + } + _ => {} } println!("Starting existing postgres {node_name}..."); node.start(&auth_token)?; @@ -680,6 +697,14 @@ fn handle_pg(pg_match: &ArgMatches, env: &local_env::LocalEnv) -> Result<()> { .get_one::("pg-version") .copied() .context("Failed to `pg-version` from the argument string")?; + + let replication = match (lsn, hot_standby) { + (Some(lsn), false) => Replication::Static(lsn), + (None, true) => Replication::Replica, + (None, false) => Replication::Primary, + (Some(_), true) => anyhow::bail!("cannot specify both lsn and hot-standby"), + }; + // when used with custom port this results in non obvious behaviour // port is remembered from first start command, i e // start --port X @@ -691,10 +716,9 @@ fn handle_pg(pg_match: &ArgMatches, env: &local_env::LocalEnv) -> Result<()> { tenant_id, node_name, timeline_id, - lsn, port, pg_version, - replica, + replication, )?; node.start(&auth_token)?; } @@ -948,10 +972,10 @@ fn cli() -> Command { .help("Specify Lsn on the timeline to start from. By default, end of the timeline would be used.") .required(false); - let replicates_arg = Arg::new("replicates") - .short('r') - .help("If configured, the node will be a hot replica of the node identified by the name") - .long("replicates") + let hot_standby_arg = Arg::new("hot-standby") + .value_parser(value_parser!(bool)) + .long("hot-standby") + .help("If set, the node will be a hot replica on the specified timeline") .required(false); Command::new("Neon CLI") @@ -1078,7 +1102,7 @@ fn cli() -> Command { .long("config-only") .required(false)) .arg(pg_version_arg.clone()) - .arg(replicates_arg.clone()) + .arg(hot_standby_arg.clone()) ) .subcommand(Command::new("start") .about("Start a postgres compute node.\n This command actually creates new node from scratch, but preserves existing config files") @@ -1089,7 +1113,7 @@ fn cli() -> Command { .arg(lsn_arg) .arg(port_arg) .arg(pg_version_arg) - .arg(replicates_arg.clone()) + .arg(hot_standby_arg) ) .subcommand( Command::new("stop") diff --git a/control_plane/src/compute.rs b/control_plane/src/compute.rs index b09f5dadc8..a6ca66539c 100644 --- a/control_plane/src/compute.rs +++ b/control_plane/src/compute.rs @@ -78,20 +78,12 @@ impl ComputeControlPlane { tenant_id: TenantId, name: &str, timeline_id: TimelineId, - lsn: Option, port: Option, pg_version: u32, - standby_of: Option<&str>, + replication: Replication, ) -> Result> { let port = port.unwrap_or_else(|| self.get_port()); - let replication = match (lsn, standby_of) { - (Some(lsn), None) => Replication::Static(lsn), - (None, Some(standby_of)) => Replication::Replica(standby_of.to_owned()), - (None, None) => Replication::Primary, - (Some(_), Some(_)) => anyhow::bail!("cannot specify both lsn and standby_of"), - }; - let node = Arc::new(PostgresNode { name: name.to_owned(), address: SocketAddr::new("127.0.0.1".parse().unwrap(), port), @@ -124,7 +116,7 @@ pub enum Replication { // if recovery_target_lsn is provided Static(Lsn), // Hot standby running on a timleine - Replica(String), + Replica, } #[derive(Debug)] @@ -188,7 +180,7 @@ impl PostgresNode { let slot_name = slot_name.to_string(); let prefix = format!("repl_{}_{}_", timeline_id, name); assert!(slot_name.starts_with(&prefix)); - Replication::Replica(slot_name[prefix.len()..].to_owned()) + Replication::Replica } else { Replication::Primary }; @@ -403,7 +395,7 @@ impl PostgresNode { Replication::Static(lsn) => { conf.append("recovery_target_lsn", &lsn.to_string()); } - Replication::Replica(of) => { + Replication::Replica => { assert!(!self.env.safekeepers.is_empty()); // TODO: use future host field from safekeeper spec @@ -417,7 +409,7 @@ impl PostgresNode { &self.tenant_id.to_string(), ); - let slot_name = format!("repl_{}_{}_{}", self.timeline_id, self.name, of); + let slot_name = format!("repl_{}_", self.timeline_id); conf.append("primary_conninfo", connstr.as_str()); conf.append("primary_slot_name", slot_name.as_str()); conf.append("hot_standby", "on"); @@ -452,7 +444,7 @@ impl PostgresNode { } } Replication::Static(lsn) => Some(*lsn), - Replication::Replica(_) => { + Replication::Replica => { None // Take the latest snapshot available to start with } };