> > 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.
Thanks for the review!
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..."
done in v13 attached.
+ pg_stat_get_db_conflict_logicalslot(D.oid) AS confl_logicalslot,
Move it to the end of pg_stat_database_conflicts columns
done in v13 attached.
+ * is being reduced. Hence this extra check.
Remove extra space before "Hence this..."
done in v13 attached.
+ /* Send the other backend, a conflict recovery signal */ + + SetInvalidVirtualTransactionId(vxid);
What about adding an "Assert(max_replication_slots >= 0);" before the replication slots check?
Makes sense, in v13 attached: Assert added and then also changed the following if accordingly to "== 0".
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: