Thread: pgsql: Allow logical decoding on standbys
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(-)
> 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
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
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
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
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
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
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