Re: speed up a logical replica setup

Euler Taveira <euler@eulerto.com>

From: "Euler Taveira" <euler@eulerto.com>
To: "vignesh C" <vignesh21@gmail.com>
Cc: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "Michael Paquier" <michael@paquier.xyz>, "Peter Eisentraut" <peter@eisentraut.org>, "Andres Freund" <andres@anarazel.de>, "Ashutosh Bapat" <ashutosh.bapat.oss@gmail.com>, "Amit Kapila" <amit.kapila16@gmail.com>, "Shlok Kyal" <shlok.kyal.oss@gmail.com>, Fabrízio de Royes Mello <fabriziomello@gmail.com>
Date: 2024-03-07T04:43:02Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. pg_createsubscriber: Remove obsolete comment

  2. pg_createsubscriber: Fix an unpredictable recovery wait time.

  3. Fix unstable test in 040_pg_createsubscriber.

  4. Fix the testcase introduced in commit 81d20fbf7a.

  5. Further weaken new pg_createsubscriber test on Windows.

  6. Temporarily(?) weaken new pg_createsubscriber test on Windows.

  7. Make pg_createsubscriber warn if publisher has two-phase commit enabled.

  8. Make pg_createsubscriber more wary about quoting connection parameters.

  9. pg_createsubscriber: Remove failover replication slots on subscriber

  10. pg_createsubscriber: Remove replication slot check on primary

  11. pg_createsubscriber: Only --recovery-timeout controls the end of recovery process

  12. pg_createsubscriber: creates a new logical replica from a standby server

  13. Add some const decorations

  14. Add option force_initdb to PostgreSQL::Test::Cluster:init()

  15. Remove MSVC scripts

On Wed, Mar 6, 2024, at 8:24 AM, vignesh C wrote:
> Few comments:

Thanks for your review. Some changes are included in v26.

> 1)   Can we use strdup here instead of atoi, as we do similarly in
> case of pg_dump too, else we will do double conversion, convert using
> atoi and again to string while forming the connection string:
> +                       case 'p':
> +                               if ((opt.sub_port = atoi(optarg)) <= 0)
> +                                       pg_fatal("invalid subscriber
> port number");
> +                               break;

I don't have a strong preference but decided to provide a patch for it. See
v26-0003.

> 2) We can have some valid range for this, else we will end up in some
> unexpected values when a higher number is specified:
> +                       case 't':
> +                               opt.recovery_timeout = atoi(optarg);
> +                               break;

I wouldn't like to add an arbitrary value. Suggestions?

> 3) Now that we have addressed most of the items, can we handle this TODO:
> +               /*
> +                * TODO use primary_conninfo (if available) from subscriber and
> +                * extract publisher connection string. Assume that there are
> +                * identical entries for physical and logical
> replication. If there is
> +                * not, we would fail anyway.
> +                */
> +               pg_log_error("no publisher connection string specified");
> +               pg_log_error_hint("Try \"%s --help\" for more
> information.", progname);
> +               exit(1);

It is not in my top priority at the moment.

> 4)  By default the log level as info here, I was not sure how to set
> it to debug level to get these error messages:
> +               pg_log_debug("publisher(%d): connection string: %s",
> i, dbinfo[i].pubconninfo);
> +               pg_log_debug("subscriber(%d): connection string: %s",
> i, dbinfo[i].subconninfo);

      <term><option>-v</option></term>
      <term><option>--verbose</option></term>
      <listitem>
       <para>
        Enables verbose mode. This will cause
        <application>pg_createsubscriber</application> to output progress messages
        and detailed information about each step to standard error.
        Repeating the option causes additional debug-level messages to appear on
        standard error.
       </para>

> 5) Currently in non verbose mode there are no messages printed on
> console, we could have a few of them printed irrespective of verbose
> or not like the following:
> a) creating publication
> b) creating replication slot
> c) waiting for the target server to reach the consistent state
> d) If pg_createsubscriber fails after this point, you must recreate
> the physical replica before continuing.
> e) creating subscription

That's the idea. Quiet mode by default.

> 6) The message should be "waiting for the target server to reach the
> consistent state":
> +#define NUM_CONN_ATTEMPTS      5
> +
> +       pg_log_info("waiting the target server to reach the consistent state");
> +
> +       conn = connect_database(conninfo, true);

Fixed.


--
Euler Taveira
EDB   https://www.enterprisedb.com/