public inbox for [email protected]
help / color / mirror / Atom feedFrom: Fabrízio de Royes Mello <[email protected]>
To: Drouvot, Bertrand <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: tushar <[email protected]>
Cc: [pgdg] Robert Haas <[email protected]>
Cc: Amit Khandekar <[email protected]>
Cc: Rahila Syed <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Minimal logical decoding on standbys
Date: Tue, 23 Mar 2021 11:29:41 -0300
Message-ID: <CAFcNs+q9FvSW7gMqM__aeWOoDNqfcQCoB8LiO-X33fvqGO8mkQ@mail.gmail.com> (raw)
In-Reply-To: <CAFcNs+rwMtfBkNkAodB9mZUAxyqvYHV0FzANm+uRiDLW6O7q1g@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CAFcNs+qEA+uxZm_CkZqq+3mGsVe=qtD20qGzBv7xL1sxTMFwFA@mail.gmail.com>
<[email protected]>
<CAFcNs+qWENuzW1Re8A=7FMRMOoiwW3qRKJqXeuTYSgVJE7qxeg@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAFcNs+otdTGim2YMS5vZPYX-6__ogH4aqcfBKJP2JOo1GBewGQ@mail.gmail.com>
<[email protected]>
<CAFcNs+rS7GHh0=oQ1yJRnShH0YwkUen4ec4Rji0W0qFJKtDZcA@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAFcNs+rwMtfBkNkAodB9mZUAxyqvYHV0FzANm+uRiDLW6O7q1g@mail.gmail.com>
On Tue, Mar 23, 2021 at 10:18 AM Fabrízio de Royes Mello <
[email protected]> wrote:
>
> LGTM too... Reviewing new changes now to move it forward and make this
patch set ready for commiter review.
>
According to the feature LGTM and all tests passed. Documentation is also
OK. Some minor comments:
+ <para>
+ A logical replication slot can also be created on a hot standby. To
prevent
+ <command>VACUUM</command> from removing required rows from the system
+ catalogs, <varname>hot_standby_feedback</varname> should be set on the
+ standby. In spite of that, if any required rows get removed, the slot
gets
+ dropped. Existing logical slots on standby also get dropped if
wal_level
+ on primary is reduced to less than 'logical'.
+ </para>
Remove extra space before "Existing logical slots..."
+ pg_stat_get_db_conflict_logicalslot(D.oid) AS
confl_logicalslot,
Move it to the end of pg_stat_database_conflicts columns
+ * is being reduced. Hence this extra check.
Remove extra space before "Hence this..."
+ /* Send the other backend, a conflict recovery signal */
+
+ SetInvalidVirtualTransactionId(vxid);
Remove extra empty line
+ if (restart_lsn % XLOG_BLCKSZ != 0)
+ elog(ERROR, "invalid replay pointer");
Add an empty line after this "IF" for code readability
+void
+ResolveRecoveryConflictWithLogicalSlots(Oid dboid, TransactionId xid,
+ char *conflict_reason)
+{
+ int i;
+ bool found_conflict = false;
+
+ if (max_replication_slots <= 0)
+ return;
What about adding an "Assert(max_replication_slots >= 0);" before the
replication slots check?
One last thing is about the name of TAP tests, we should rename them
because there are other TAP tests starting with 022_ and 023_. It should be
renamed to:
src/test/recovery/t/022_standby_logical_decoding_xmins.pl ->
src/test/recovery/t/024_standby_logical_decoding_xmins.pl
src/test/recovery/t/023_standby_logical_decoding_conflicts.pl
-> src/test/recovery/t/025_standby_logical_decoding_conflicts.pl
Regards,
--
Fabrízio de Royes Mello
PostgreSQL Developer at OnGres Inc. - https://ongres.com
view thread (196+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Minimal logical decoding on standbys
In-Reply-To: <CAFcNs+q9FvSW7gMqM__aeWOoDNqfcQCoB8LiO-X33fvqGO8mkQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox