Re: Minimal logical decoding on standbys

Поиск
Список
Период
Сортировка
От Fabrízio de Royes Mello
Тема Re: Minimal logical decoding on standbys
Дата
Msg-id CAFcNs+q9FvSW7gMqM__aeWOoDNqfcQCoB8LiO-X33fvqGO8mkQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Minimal logical decoding on standbys  (Fabrízio de Royes Mello <fabriziomello@gmail.com>)
Ответы Re: Minimal logical decoding on standbys  ("Drouvot, Bertrand" <bdrouvot@amazon.com>)
Список pgsql-hackers


On Tue, Mar 23, 2021 at 10:18 AM Fabrízio de Royes Mello <fabriziomello@gmail.com> 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:


Regards,

--
   Fabrízio de Royes Mello
   PostgreSQL Developer at OnGres Inc. - https://ongres.com

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Andrei Zubkov
Дата:
Сообщение: Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements
Следующее
От: David Steele
Дата:
Сообщение: Re: Add Nullif case for eval_const_expressions_mutator