On Wed, Feb 14, 2024 at 12:58 PM vignesh C <vignes...@gmail.com> wrote: > > On Wed, 7 Feb 2024 at 16:27, vignesh C <vignes...@gmail.com> wrote: > > > > I was able to reproduce the issue consistently with the changes shared > > by Tom Lane at [1]. > > I have made changes to change ALTER SUBSCRIPTION to DROP+CREATE > > SUBSCRIPTION and verified that the test has passed consistently for > > >50 runs that I ran. Also the test execution time increased for this > > case is very negligibly: > > Without patch: 7.991 seconds > > With test change patch: 8.121 seconds > > > > The test changes for the same are attached. > > Alternative, this could also be fixed like the changes proposed by > Amit at [1]. In this case we ignore publications that are not found > for the purpose of computing RelSyncEntry attributes. We won't mark > such an entry as valid till all the publications are loaded without > anything missing. This means we won't publish operations on tables > corresponding to that publication till we found such a publication and > that seems okay. > > Tomas had raised a performance issue forcing us to reload it for every > replicated change/row in case the publications are invalid at [2]. >
Did you measure the performance impact? I think it should impact the performance only when there is a dropped/non-existent publication as per the subscription definition. Also, in the same email[2], Tomas raised another concern that in such cases it is better to break the replication. > How > about keeping the default option as it is and providing a new option > skip_not_exist_publication while creating/altering a subscription. In > this case if skip_not_exist_publication is specified we will ignore > the case if publication is not present and publications will be kept > in invalid and get validated later. > I don't know if this deserves a separate option or not but I think it is better to discuss this in a separate thread. To resolve the BF failure, I still feel, we should just recreate the subscription. This is a pre-existing problem and we can track it via a separate patch with a test case targetting such failures. > The attached patch has the changes for the same. Thoughts? > > [1] - > https://www.postgresql.org/message-id/CAA4eK1%2BT-ETXeRM4DHWzGxBpKafLCp__5bPA_QZfFQp7-0wj4Q%40mail.gmail.com > [2] - > https://www.postgresql.org/message-id/dc08add3-10a8-738b-983a-191c7406707b%40enterprisedb.com > -- With Regards, Amit Kapila.