On Wed, Jan 13, 2021 at 3:16 PM Bharath Rupireddy <[email protected]> wrote: > > On Wed, Jan 13, 2021 at 2:36 PM japin <[email protected]> wrote: > > > In summary, I feel we need to fix the publisher sending the inserts > > > even though the table is dropped from the publication, that is the > > > patch patch proposed by japin. This solves the bug reported in this > > > thread. > > > > > > And also, it's good to have the LogicalRepRelMap invalidation fix as a > > > 0002 patch in invalidate_syncing_table_states, subscription_change_cb > > > and logicalrep_rel_open as proposed by me. > > > > > > Thoughts? > > > > > > > I think invalidate the LogicalRepRelMap is necessary. If the table isn't in > > subscription, can we remove the LogicalRepRelMapEntry from LogicalRepRelMap? > > IIUC, it's not possible to know the relid of the alter > publication...dropped table in the invalidation callbacks > invalidate_syncing_table_states or subscription_change_cb, so it's > better to set state of all the entries to SUBREL_STATE_UNKNOWN, so > that, in the next logicalrep_rel_open call the function > GetSubscriptionRelState will be called and the pg_subscription_rel > will be read freshly. > > This is inline with the reasoning specified at [1]. > > [1] - > https://www.postgresql.org/message-id/CAFiTN-udwuc_h0TaFrSEKb-Yo1vVvkjz9TDRw7VE3P-KiPSGbQ%40mail.gmail.com
Attaching following two patches:
0001 - Makes publisher to not publish the changes for the alter
publication ... dropped tables. Original patch is by japin, I added
comments, changed the function name and adjusted the code a bit.
0002 - Invalidates the relation map cache in subscriber syscache
invalidation callbacks. Currently, I'm setting entry->state to
SUBREL_STATE_UNKNOWN in the new invalidation function that's
introduced logicalrep_relmap_invalidate, so that in the next call to
logicalrep_rel_open the entry's state will be read from the system
catalogues using GetSubscriptionRelState. If this is sound's bit
strange, I can add a boolean invalid to LogicalRepRelMapEntry
structure and set that here and in logicalrep_rel_open, I can have
something like:
if (entry->state != SUBREL_STATE_READY || entry->invalid)
entry->state = GetSubscriptionRelState(MySubscription->oid,
entry->localreloid,
&entry->statelsn);
if (entry->invalid)
entry->invalid = false;
make check and make check-world passes on both patches.
If okay with the fixes, I will try to add a test case and also a cf
entry so that these patches get a chance to be tested on all the
platforms.
Thoughts?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
v1-0001-Fix-ALTER-PUBLICATION.DROP-TABLE-behaviour.patch
Description: Binary data
v1-0002-Invalidate-relation-map-cache-in-subscriber-sysca.patch
Description: Binary data
