Add new neon_local option to spin up read-only replica. #3714

lubennikovaav · 2023-02-28T11:34:24Z

UPD:

Usage example:

./target/debug/neon_local start
./target/debug/neon_local tenant create --set-default
./target/debug/neon_local pg start repl --hot-standby=true

This is a rebased version of #850
This is only a control_ctl part of the patch, that starts postgres in hot standby mode:

psql -p 55432 postgres -U cloud_admin -c 'select pg_is_in_recovery();'
 pg_is_in_recovery 
-------------------
 t
(1 row)

Some design questions:

Should we instead use pg start replica --timeline-id=... --hot_standby=on syntax and not depend on primary node name at all?
@MMeent , WDYT?
How to pass safekeeper conninfo to primary_conninfo? And how to handle possible safekeeper failures?

@arssher , is there any service discovery that tracks state of safekeepers and reports it to control plane?
There is a broker, but IIUC, only pageserver and safekeepers use it.

pass full list of all safekeepers to primary_conninfo

MMeent · 2023-02-28T12:14:36Z

Should we instead use pg start replica --timeline-id=... --hot_standby=on syntax and not depend on primary node name at all?

I think that would be quite useful, yes.

arssher · 2023-02-28T13:39:50Z

is there any service discovery that tracks state of safekeepers and reports it to control plane?

No, and using it would be hard, as in turn it might fail and/or have connectivity issues with some of safekeepers. I'd propose to just wire full list of all safekeepers to primary_conninfo (connection strings support multiple hosts). That would be already a good start. Later we can also add logic like 'if currently seemingly good connection doesn't provide new data for a long time, let's try another one'. Or think about declaring safekeeper itself as disconnected from the quorum.

Usage example: neon_local pg start main_roo --replicates main

vadim2404 · 2023-03-06T14:30:49Z

control_plane/src/bin/neon_local.rs

@@ -918,6 +972,12 @@ fn cli() -> Command {
        .help("Specify Lsn on the timeline to start from. By default, end of the timeline would be used.")
        .required(false);

+    let hot_standby_arg = Arg::new("hot-standby")


At this moment you need to specify

--hot-standby=true

What if leave just flag --hot-standby?

let hot_standby_arg = Arg::new("hot-standby") .long("hot-standby") .action(ArgAction::SetTrue) .help("If set, the node will be a hot replica on the specified timeline") .required(false);

and then instead of

let hot_standby = sub_args .get_one::<bool>("hot-standby") .copied() .unwrap_or(false);

you can just use

let hot_standby = sub_args.get_flag("hot-standby");

Does that still allow --hot-standby=false?

it will be false if you're not providing this flag

Yes, but that doesn't answer my question. Can you make it explicit that you're not using hot standby with --hot-standby=false?

didn't try it, personally. But why do you need this?

for instance ls command. and when you provide ls -l it changes the representation. You don't need to write ls --list=false

Therefore I'm trying to understand the use case

I'm not sure that you need to template the neon_local

Don't we use it in k8s/containers?

compute_ctl is used here

Don't we use it in k8s/containers?

Nope, neon_local is for local development only.
I think, for k8s we will just provide all these config lines from console before starting the compute.

Wouldn't it also need the standby.signal and some other special handling?

MMeent · 2023-03-07T11:30:34Z

control_plane/src/postgresql_conf.rs

@@ -13,7 +13,7 @@ use std::io::BufRead;
 use std::str::FromStr;

 /// In-memory representation of a postgresql.conf file
-#[derive(Default)]
+#[derive(Default, Debug)]


Is this change still needed? I couldn't find the code that would need this.

MMeent · 2023-03-09T16:08:49Z

control_plane/src/compute.rs

+            Replication::Static(Lsn::from_str(lsn_str)?)
+        } else if let Some(slot_name) = conf.get("primary_slot_name") {
+            let slot_name = slot_name.to_string();
+            let prefix = format!("repl_{}_{}_", timeline_id, name);


Suggested change

let prefix = format!("repl_{}_{}_", timeline_id, name);

let prefix = format!("repl_{}_", timeline_id);

See https://github.com/neondatabase/neon/pull/3714/files#diff-8219525a31af104cdd3e3ced077045b49cdb50819bd97f50eb4f642dc0565047R421

Notes: - This still needs UI support from the Console - I've not tuned any GUCs for PostgreSQL to make this work better - Safekeeper has gotten a tweak in which WAL is sent and how: It now sends zero-ed WAL data from the start of the timeline's first segment up to the first byte of the timeline to be compatible with normal PostgreSQL WAL streaming. - This includes the commits of #3714 Fixes one part of #769 Co-authored-by: Anastasia Lubennikova <anastasia@neon.tech>

lubennikovaav added 3 commits March 2, 2023 17:38

WIP: Add new neon_local option to spin up read-only replica.

cb9a558

Usage example: neon_local pg start main_roo --replicates main

Change --replicates option to --hot-standby flag

2c683b3

pass multiple safekeeper hostaddrs to primary_conninfo

afaaeb9

lubennikovaav force-pushed the ro_replica_comute_ctl branch from 2571ccc to afaaeb9 Compare March 6, 2023 10:50

lubennikovaav changed the title ~~WIP: Add new neon_local option to spin up read-only replica.~~ Add new neon_local option to spin up read-only replica. Mar 6, 2023

lubennikovaav requested a review from MMeent March 6, 2023 11:09

lubennikovaav marked this pull request as ready for review March 6, 2023 11:09

lubennikovaav requested review from a team as code owners March 6, 2023 11:09

lubennikovaav requested review from problame and removed request for a team and problame March 6, 2023 11:10

vadim2404 reviewed Mar 6, 2023

View reviewed changes

vadim2404 mentioned this pull request Mar 6, 2023

Epic: Support read-only replicas following the primary #769

Closed

7 tasks

MMeent approved these changes Mar 9, 2023

View reviewed changes

MMeent reviewed Mar 9, 2023

View reviewed changes

MMeent mentioned this pull request Mar 10, 2023

Feature/replicas #3793

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new neon_local option to spin up read-only replica. #3714

Add new neon_local option to spin up read-only replica. #3714

lubennikovaav commented Feb 28, 2023 •

edited

Loading

MMeent commented Feb 28, 2023

arssher commented Feb 28, 2023

vadim2404 Mar 6, 2023

MMeent Mar 6, 2023

vadim2404 Mar 6, 2023

MMeent Mar 6, 2023

vadim2404 Mar 6, 2023

vadim2404 Mar 6, 2023

MMeent Mar 6, 2023

vadim2404 Mar 6, 2023

lubennikovaav Mar 6, 2023

MMeent Mar 6, 2023

MMeent Mar 7, 2023

MMeent Mar 9, 2023

	let prefix = format!("repl_{}_{}_", timeline_id, name);
	let prefix = format!("repl_{}_", timeline_id);

Add new neon_local option to spin up read-only replica. #3714

Are you sure you want to change the base?

Add new neon_local option to spin up read-only replica. #3714

Conversation

lubennikovaav commented Feb 28, 2023 • edited Loading

MMeent commented Feb 28, 2023

arssher commented Feb 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lubennikovaav commented Feb 28, 2023 •

edited

Loading