Thread: pgsql: Allow logical decoding on standbys

pgsql: Allow logical decoding on standbys

From
Andres Freund
Date:
Allow logical decoding on standbys

Unsurprisingly, this requires wal_level = logical to be set on the primary and
standby. The infrastructure added in 26669757b6a ensures that slots are
invalidated if the primary's wal_level is lowered.

Creating a slot on a standby waits for a xl_running_xact record to be
processed. If the primary is idle (and thus not emitting xl_running_xact
records), that can take a while.  To make that faster, this commit also
introduces the pg_log_standby_snapshot() function. By executing it on the
primary, completion of slot creation on the standby can be accelerated.

Note that logical decoding on a standby does not itself enforce that required
catalog rows are not removed. The user has to use physical replication slots +
hot_standby_feedback or other measures to prevent that. If catalog rows
required for a slot are removed, the slot is invalidated.

See 6af1793954e for an overall design of logical decoding on a standby.

Bumps catversion, for the addition of the pg_log_standby_snapshot() function.

Author: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Author: Andres Freund <andres@anarazel.de> (in an older version)
Author: Amit Khandekar <amitdkhan.pg@gmail.com> (in an older version)
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: FabrÌzio de Royes Mello <fabriziomello@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/0fdab27ad68a059a1663fa5ce48d76333f1bd74c

Modified Files
--------------
doc/src/sgml/func.sgml                    | 15 ++++++++
doc/src/sgml/logicaldecoding.sgml         | 27 +++++++++++++++
src/backend/access/transam/xlog.c         | 11 ++++++
src/backend/access/transam/xlogfuncs.c    | 31 +++++++++++++++++
src/backend/catalog/system_functions.sql  |  2 ++
src/backend/replication/logical/decode.c  | 30 +++++++++++++++-
src/backend/replication/logical/logical.c | 36 ++++++++++---------
src/backend/replication/slot.c            | 57 ++++++++++++++++---------------
src/backend/replication/walsender.c       | 48 +++++++++++++++++---------
src/include/access/xlog.h                 |  1 +
src/include/catalog/catversion.h          |  2 +-
src/include/catalog/pg_proc.dat           |  3 ++
12 files changed, 202 insertions(+), 61 deletions(-)


Re: pgsql: Allow logical decoding on standbys

From
Kyotaro Horiguchi
Date:
> Allow logical decoding on standbys

This adds the following error message.

+    errmsg("logical decoding on a standby requires wal_level to be at least logical on the primary")));

We alredy have a nearly identical message as follows.

> errmsg("logical decoding requires wal_level >= logical")));

And we used to writte this kind of conditions, like "wal_level >=
logical", using a mathematical operator. Don't we need to unify them?

And, a nearby commit addds the following message.

+            appendStringInfo(&err_detail, _("Logical decoding on standby requires wal_level to be at least logical on
theprimary server"));
 

This is omitting the indefinite article before "standby". I'm not sure
what to do about it but feel like we don't need it here.

diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index 5508cc2177..beef399b42 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -177,7 +177,7 @@ xlog_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
                     Assert(RecoveryInProgress());
                     ereport(ERROR,
                             (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                             errmsg("logical decoding on a standby requires wal_level to be at least logical on the
primary")));
+                             errmsg("logical decoding on standby requires wal_level >= logical on the primary")));
                 }
                 break;
             }
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 82dae95080..7e1f677f7a 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -137,7 +137,7 @@ CheckLogicalDecodingRequirements(void)
         if (GetActiveWalLevelOnStandby() < WAL_LEVEL_LOGICAL)
             ereport(ERROR,
                     (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                     errmsg("logical decoding on a standby requires wal_level to be at least logical on the
primary")));
+                     errmsg("logical decoding on standby requires wal_level >= logical on the primary")));
     }
 }
 


regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



Re: pgsql: Allow logical decoding on standbys

From
"Drouvot, Bertrand"
Date:
Hi,

On 4/11/23 5:02 AM, Kyotaro Horiguchi wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you
canconfirm the sender and know the content is safe. 
>
>
>
>> Allow logical decoding on standbys
>
> This adds the following error message.
>
> +       errmsg("logical decoding on a standby requires wal_level to be at least logical on the primary")));
>
> We alredy have a nearly identical message as follows.
>
>> errmsg("logical decoding requires wal_level >= logical")));
>
> And we used to writte this kind of conditions, like "wal_level >=
> logical", using a mathematical operator. Don't we need to unify them?
>
> And, a nearby commit addds the following message.
>
> +                       appendStringInfo(&err_detail, _("Logical decoding on standby requires wal_level to be at
leastlogical on the primary server")); 
>
> This is omitting the indefinite article before "standby". I'm not sure
> what to do about it but feel like we don't need it here.
>
> diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
> index 5508cc2177..beef399b42 100644
> --- a/src/backend/replication/logical/decode.c
> +++ b/src/backend/replication/logical/decode.c
> @@ -177,7 +177,7 @@ xlog_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
>                                          Assert(RecoveryInProgress());
>                                          ereport(ERROR,
>                                                          (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> -                                                        errmsg("logical decoding on a standby requires wal_level to
beat least logical on the primary"))); 
> +                                                        errmsg("logical decoding on standby requires wal_level >=
logicalon the primary"))); 
>                                  }
>                                  break;
>                          }
> diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
> index 82dae95080..7e1f677f7a 100644
> --- a/src/backend/replication/logical/logical.c
> +++ b/src/backend/replication/logical/logical.c
> @@ -137,7 +137,7 @@ CheckLogicalDecodingRequirements(void)
>                  if (GetActiveWalLevelOnStandby() < WAL_LEVEL_LOGICAL)
>                          ereport(ERROR,
>                                          (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> -                                        errmsg("logical decoding on a standby requires wal_level to be at least
logicalon the primary"))); 
> +                                        errmsg("logical decoding on standby requires wal_level >= logical on the
primary")));
>          }
>   }
>
>
>


Thanks for the feedback!

I think you've fair points and that your proposals are correct (just not sure about the indefinite article though).

Once we agree on the wording, then 035_standby_logical_decoding.pl would need to be changed accordingly.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



Amazon Web Services EMEA SARL, 38 avenue John F. Kennedy, L-1855 Luxembourg, R.C.S. Luxembourg B186284

Amazon Web Services EMEA SARL, succursale francaise, 31 Place des Corolles, Tour Carpe Diem, F-92400 Courbevoie, SIREN
831001 334, RCS Nanterre, APE 6311Z,  TVA FR30831001334 



Re: pgsql: Allow logical decoding on standbys

From
Andres Freund
Date:
Hi,

On 2023-04-11 12:02:53 +0900, Kyotaro Horiguchi wrote:
> > Allow logical decoding on standbys
> 
> This adds the following error message.
> 
> +    errmsg("logical decoding on a standby requires wal_level to be at least logical on the primary")));
> 
> We alredy have a nearly identical message as follows.
> 
> > errmsg("logical decoding requires wal_level >= logical")));
> 
> And we used to writte this kind of conditions, like "wal_level >=
> logical", using a mathematical operator. Don't we need to unify them?

I guess. It doesn't seem particularly important, but why not.


> And, a nearby commit addds the following message.
> 
> +            appendStringInfo(&err_detail, _("Logical decoding on standby requires wal_level to be at least logical
onthe primary server"));
 
> 
> This is omitting the indefinite article before "standby". I'm not sure
> what to do about it but feel like we don't need it here.

I don't think we need it either, but I'm not a native speaker.

Greetings,

Andres Freund



Re: pgsql: Allow logical decoding on standbys

From
Andres Freund
Date:
Hi,

On 2023-04-12 10:42:44 -0700, Andres Freund wrote:
> On 2023-04-11 12:02:53 +0900, Kyotaro Horiguchi wrote:
> > > Allow logical decoding on standbys
> > 
> > This adds the following error message.
> > 
> > +    errmsg("logical decoding on a standby requires wal_level to be at least logical on the primary")));
> > 
> > We alredy have a nearly identical message as follows.
> > 
> > > errmsg("logical decoding requires wal_level >= logical")));
> > 
> > And we used to writte this kind of conditions, like "wal_level >=
> > logical", using a mathematical operator. Don't we need to unify them?
> 
> I guess. It doesn't seem particularly important, but why not.

If we do that, we should adjust the errdetail() that you (correctly!) pointed
out as needing punctuation as well.  Pushed those improvements together.

Greetings,

Andres Freund



Re: pgsql: Allow logical decoding on standbys

From
Kyotaro Horiguchi
Date:
At Wed, 12 Apr 2023 11:10:00 -0700, Andres Freund <andres@anarazel.de> wrote in 
> Hi,
> 
> On 2023-04-12 10:42:44 -0700, Andres Freund wrote:
> > On 2023-04-11 12:02:53 +0900, Kyotaro Horiguchi wrote:
> > > > Allow logical decoding on standbys
> > > 
> > > This adds the following error message.
> > > 
> > > +    errmsg("logical decoding on a standby requires wal_level to be at least logical on the primary")));
> > > 
> > > We alredy have a nearly identical message as follows.
> > > 
> > > > errmsg("logical decoding requires wal_level >= logical")));
> > > 
> > > And we used to writte this kind of conditions, like "wal_level >=
> > > logical", using a mathematical operator. Don't we need to unify them?
> > 
> > I guess. It doesn't seem particularly important, but why not.
> 
> If we do that, we should adjust the errdetail() that you (correctly!) pointed
> out as needing punctuation as well.  Pushed those improvements together.

Thanks!

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



Re: pgsql: Allow logical decoding on standbys

From
Robert Haas
Date:
On Wed, Apr 12, 2023 at 1:43 PM Andres Freund <andres@anarazel.de> wrote:
> > And, a nearby commit addds the following message.
> >
> > +                     appendStringInfo(&err_detail, _("Logical decoding on standby requires wal_level to be at
leastlogical on the primary server")); 
> >
> > This is omitting the indefinite article before "standby". I'm not sure
> > what to do about it but feel like we don't need it here.
>
> I don't think we need it either, but I'm not a native speaker.

I am a native speaker of English and I think it is not required here.
If we were going to add an article, I think we would want an
indefinite one ("a standby") not a definite one ("the standby")
because we're talking about a general truism, not a situation that
pertains to one specific server. However, I don't think we should add
that because the current phrasing is more consistent with our general
practice. It's not the way you would actually talk, but it the way
that people often write error messages. For example, we say "file not
found" not "the file was not found," and nobody gets upset or confused
by that.

--
Robert Haas
EDB: http://www.enterprisedb.com



Re: pgsql: Allow logical decoding on standbys

From
Kyotaro Horiguchi
Date:
At Thu, 13 Apr 2023 12:07:19 -0400, Robert Haas <robertmhaas@gmail.com> wrote in 
> On Wed, Apr 12, 2023 at 1:43=E2=80=AFPM Andres Freund <andres@anarazel.de> =
> wrote:
> > > And, a nearby commit addds the following message.
> > >
> > > +                     appendStringInfo(&err_detail, _("Logical decoding=
>  on standby requires wal_level to be at least logical on the primary server=
> "));
> > >
> > > This is omitting the indefinite article before "standby". I'm not sure
> > > what to do about it but feel like we don't need it here.
> >
> > I don't think we need it either, but I'm not a native speaker.
> 
> I am a native speaker of English and I think it is not required here.
> If we were going to add an article, I think we would want an
> indefinite one ("a standby") not a definite one ("the standby")
> because we're talking about a general truism, not a situation that
> pertains to one specific server. However, I don't think we should add
> that because the current phrasing is more consistent with our general
> practice. It's not the way you would actually talk, but it the way
> that people often write error messages. For example, we say "file not
> found" not "the file was not found," and nobody gets upset or confused
> by that.

Thaks for the clarification!

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center