Re: Add an option to skip loading missing publication to avoid logical replication failure

On Sat, May 10, 2025 at 5:15 PM Amit Kapila wrote: > > I am planning to proceed with the test-fix proposed by Vignesh [1] > early next week unless we want to discuss more on this issue. > Pushed. > [1] - > https://www.postgresql.org/message-id/CALDaNm3TH3J8fwj%2BNcdjdN8DZCdrnmm1kzBsHBW2nkN%2Bh6

Re: Add an option to skip loading missing publication to avoid logical replication failure

If the presumed theory regarding the cause of the issue is correct — as outlined in this email — and no data replication occurs in this scenario

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Sun, May 11, 2025 at 8:49 PM Xuneng Zhou wrote: > > Hi, I was able to reproduce the failure by adding a 1-second sleep in the > LogicalRepApplyLoop function > However, I noticed that the tests under src/test/subscription run > significantly slower— is this normal? > Yes, because you made app

Re: Add an option to skip loading missing publication to avoid logical replication failure

Hi, I was able to reproduce the failure by adding a 1-second sleep in the LogicalRepApplyLoop function However, I noticed that the tests under src/test/subscription run significantly slower— is this normal? Also, it looks like the patch mentioned in this thread addresses the issue: https://www.pos

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, May 6, 2025 at 5:17 PM Amit Kapila wrote: > > On Tue, May 6, 2025 at 3:33 PM Xuneng Zhou wrote: > > > > A clear benefit of addressing this in code is to ensure that the user sees > > the log message, which can be valuable for trouble-shooting—even under race > > conditions. > > > > I do

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, May 6, 2025 at 3:33 PM Xuneng Zhou wrote: > > A clear benefit of addressing this in code is to ensure that the user sees > the log message, which can be valuable for trouble-shooting—even under race > conditions. > I don't think we can take that guarantee because if the Insert is concur

Re: Add an option to skip loading missing publication to avoid logical replication failure

Hi, A clear benefit of addressing this in code is to ensure that the user sees the log message, which can be valuable for trouble-shooting—even under race conditions. ereport(WARNING, errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Fri, May 2, 2025 at 10:24 PM Tom Lane wrote: > > vignesh C writes: > > Due to the asynchronous nature of these processes, the ALTER > > SUBSCRIPTION command may not be immediately observed by the apply > > worker. Meanwhile, the walsender may process and decode an INSERT > > statement. > > If

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Fri, 2 May 2025 at 09:23, vignesh C wrote: > > On Fri, 2 May 2025 at 06:30, Tom Lane wrote: > > > > vignesh C writes: > > > I agree with your analysis. I was able to reproduce the issue by > > > delaying the invalidation of the subscription until the walsender > > > finished decoding the INSE

Re: Add an option to skip loading missing publication to avoid logical replication failure

vignesh C writes: > Due to the asynchronous nature of these processes, the ALTER > SUBSCRIPTION command may not be immediately observed by the apply > worker. Meanwhile, the walsender may process and decode an INSERT > statement. > If the insert targets a table (e.g., tab_3) that does not belong t

Re: Add an option to skip loading missing publication to avoid logical replication failure

Yeh, tks for your clarification. I have a basic understanding of it now. I mean is this considered a bug or design defect in the codebase? If so, should we prevent it from occuring in general, not just for this specific test. vignesh C > > We have three processes involved in this scenario: > A

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Fri, 2 May 2025 at 10:11, Xuneng Zhou wrote: > > Hi, > Is this an expected behavior? > > A race between subscriber LSN feedback and publisher subscription change > processing allows the walsender to restart decoding past relevant WAL > records, bypassing the updated subscription rules for tho

Re: Add an option to skip loading missing publication to avoid logical replication failure

Xuneng Zhou writes: > Is this an expected behavior? I'm wondering that too. I don't see how the repro method Vignesh describes could correspond to a simple timing issue. It smells like there's a bug here somewhere. regards, tom lane

Re: Add an option to skip loading missing publication to avoid logical replication failure

Hi, Is this an expected behavior? A race between subscriber LSN feedback and publisher subscription change processing allows the walsender to restart decoding past relevant WAL records, bypassing the updated subscription rules for those records. On Wed, 30 Apr 2025 at 17:41, Amit Kapila wrote:

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Fri, 2 May 2025 at 06:30, Tom Lane wrote: > > vignesh C writes: > > I agree with your analysis. I was able to reproduce the issue by > > delaying the invalidation of the subscription until the walsender > > finished decoding the INSERT operation following the ALTER > > SUBSCRIPTION through a d

Re: Add an option to skip loading missing publication to avoid logical replication failure

+1, I was unable to reproduce this with lldb, not sure my way is appropriate or not. > > Can you be a little more specific about how you reproduced this? > I tried inserting sleep() calls in various likely-looking spots > and could not get a failure that way. > > regards,

Re: Add an option to skip loading missing publication to avoid logical replication failure

vignesh C writes: > I agree with your analysis. I was able to reproduce the issue by > delaying the invalidation of the subscription until the walsender > finished decoding the INSERT operation following the ALTER > SUBSCRIPTION through a debugger and using the lsn from the pg_waldump > of the INS

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Wed, 30 Apr 2025 at 17:41, Amit Kapila wrote: > > On Wed, Apr 30, 2025 at 11:22 AM Tom Lane wrote: > > > > Xuneng Zhou pointed out on Discord that the test case added by > > 7c99dc587 has caused repeated failures in CI --- though oddly, > > it's not failed in the buildfarm so far as I can find

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Wed, Apr 30, 2025 at 11:22 AM Tom Lane wrote: > > Xuneng Zhou pointed out on Discord that the test case added by > 7c99dc587 has caused repeated failures in CI --- though oddly, > it's not failed in the buildfarm so far as I can find. The > failures look like > > timed out waiting for match: (

Re: Add an option to skip loading missing publication to avoid logical replication failure

Amit Kapila writes: > Thanks, Dilip and Sawada-San, for the inputs, and Vignesh for the > patch. I have pushed the change. Xuneng Zhou pointed out on Discord that the test case added by 7c99dc587 has caused repeated failures in CI --- though oddly, it's not failed in the buildfarm so far as I can

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Thu, Mar 13, 2025 at 7:26 PM Dilip Kumar wrote: > > On Thu, Mar 13, 2025 at 3:20 PM Amit Kapila wrote: > > > > > > I think that is too much related to pub-sub model, and ideally, > > pgoutput should not care about it. I have written a comment > > considering somebody using pgoutput decoding mo

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Thu, Mar 13, 2025 at 3:20 PM Amit Kapila wrote: > > On Thu, Mar 13, 2025 at 11:43 AM Dilip Kumar wrote: > > > > Looks fine, shall we add the missing publication point as well > > something like below > > > > /* > > * In operations like 'ALTER SUBSCRIPTION ... ADD/SET PUBLICATION' and > > * 'CR

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Thu, Mar 13, 2025 at 11:43 AM Dilip Kumar wrote: > > Looks fine, shall we add the missing publication point as well > something like below > > /* > * In operations like 'ALTER SUBSCRIPTION ... ADD/SET PUBLICATION' and > * 'CREATE SUBSCRIPTION', if the specified publication does not exist or > *

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Thu, Mar 13, 2025 at 10:49 AM vignesh C wrote: > > On Thu, 13 Mar 2025 at 09:18, Dilip Kumar wrote: > > > > Thanks looks good to me. > > > > While looking at the patch, I have a few comments/questions > > > > + if (pub) > > + result = lappend(result, pub); > > + else > > + { > > + /* > > + * W

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Wed, 12 Mar 2025 at 16:15, Dilip Kumar wrote: > > Thanks, Vignesh, for adding the test. I believe you've tested the > effect of DROP PUBLICATION. However, I think we should also test the > behavior of ALTER SUBSCRIPTION...SET PUBLICATION before creating the > PUBLICATION, and then create the PU

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Thu, 13 Mar 2025 at 09:18, Dilip Kumar wrote: > > Thanks looks good to me. > > While looking at the patch, I have a few comments/questions > > + if (pub) > + result = lappend(result, pub); > + else > + { > + /* > + * When executing 'ALTER SUBSCRIPTION ... SET PUBLICATION', the > + * apply worke

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Thu, Mar 13, 2025 at 7:38 AM vignesh C wrote: > > On Wed, 12 Mar 2025 at 16:15, Dilip Kumar wrote: > > > > Thanks, Vignesh, for adding the test. I believe you've tested the > > effect of DROP PUBLICATION. However, I think we should also test the > > behavior of ALTER SUBSCRIPTION...SET PUBLICA

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Wed, Mar 12, 2025 at 3:34 AM Amit Kapila wrote: > > On Tue, Mar 11, 2025 at 9:48 AM Dilip Kumar wrote: > > > > On Mon, Mar 10, 2025 at 10:54 AM Amit Kapila > > wrote: > > > > > > > > >> BTW, I am planning to commit this only on HEAD as this is a behavior > > > >> change. Please let me know i

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, Mar 11, 2025 at 4:01 PM vignesh C wrote: > > On Mon, 10 Mar 2025 at 09:33, Amit Kapila wrote: > > > > On Tue, Mar 4, 2025 at 6:54 PM vignesh C wrote: > > > > > > On further thinking, I felt the use of publications_updated variable > > > is not required we can use publications_valid itsel

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, Mar 11, 2025 at 9:48 AM Dilip Kumar wrote: > > On Mon, Mar 10, 2025 at 10:54 AM Amit Kapila wrote: > > > > > >> BTW, I am planning to commit this only on HEAD as this is a behavior > > >> change. Please let me know if you guys think otherwise. > > > > > > > > > Somehow this looks like a b

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Mon, Mar 10, 2025 at 10:15 AM Dilip Kumar wrote: > > On Mon, Mar 10, 2025 at 9:33 AM Amit Kapila wrote: >> >> On Tue, Mar 4, 2025 at 6:54 PM vignesh C wrote: >> > >> > On further thinking, I felt the use of publications_updated variable >> > is not required we can use publications_valid itsel

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Mon, 10 Mar 2025 at 09:33, Amit Kapila wrote: > > On Tue, Mar 4, 2025 at 6:54 PM vignesh C wrote: > > > > On further thinking, I felt the use of publications_updated variable > > is not required we can use publications_valid itself which will be set > > if the publication system table is inval

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Mon, Mar 10, 2025 at 10:54 AM Amit Kapila wrote: > > On Mon, Mar 10, 2025 at 10:15 AM Dilip Kumar wrote: > > > > On Mon, Mar 10, 2025 at 9:33 AM Amit Kapila wrote: > >> > >> On Tue, Mar 4, 2025 at 6:54 PM vignesh C wrote: > >> > > >> > On further thinking, I felt the use of publications_upda

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, Mar 4, 2025 at 6:54 PM vignesh C wrote: > > On further thinking, I felt the use of publications_updated variable > is not required we can use publications_valid itself which will be set > if the publication system table is invalidated. Here is a patch for > the same. > The patch relies on

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Mon, Mar 10, 2025 at 9:33 AM Amit Kapila wrote: > On Tue, Mar 4, 2025 at 6:54 PM vignesh C wrote: > > > > On further thinking, I felt the use of publications_updated variable > > is not required we can use publications_valid itself which will be set > > if the publication system table is inva

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Sun, Mar 9, 2025 at 9:00 AM Masahiko Sawada wrote: > > On Tue, Mar 4, 2025 at 9:04 PM Amit Kapila wrote: > > > > > I see the point of adding such an option to avoid breaking the current > > applications (if there are any) that are relying on current behaviour. > > But OTOH, I am not sure if us

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, Mar 4, 2025 at 9:04 PM Amit Kapila wrote: > > On Tue, Mar 4, 2025 at 12:23 PM vignesh C wrote: > > > > There is almost negligible dip with the above suggested way, the test > > results for the same is given below(execution time is in milli > > seconds): > > Brach/records | 100 | 10

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, Mar 4, 2025 at 12:23 PM vignesh C wrote: > > There is almost negligible dip with the above suggested way, the test > results for the same is given below(execution time is in milli > seconds): > Brach/records | 100 | 1000 | 1| 10 | 100 > Head | 10

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, 4 Mar 2025 at 12:22, vignesh C wrote: > > On Mon, 3 Mar 2025 at 16:41, Amit Kapila wrote: > > > > On Mon, Mar 3, 2025 at 2:30 PM vignesh C wrote: > > > > > > On Tue, 25 Feb 2025 at 15:32, vignesh C wrote: > > > > > > > > The attached script has the script that was used for testing. Here

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Mon, 3 Mar 2025 at 16:41, Amit Kapila wrote: > > On Mon, Mar 3, 2025 at 2:30 PM vignesh C wrote: > > > > On Tue, 25 Feb 2025 at 15:32, vignesh C wrote: > > > > > > The attached script has the script that was used for testing. Here the > > > NUM_RECORDS count should be changed accordingly for

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Mon, Mar 3, 2025 at 2:30 PM vignesh C wrote: > > On Tue, 25 Feb 2025 at 15:32, vignesh C wrote: > > > > The attached script has the script that was used for testing. Here the > > NUM_RECORDS count should be changed accordingly for each of the tests > > and while running the test with the patch

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, 25 Feb 2025 at 15:32, vignesh C wrote: > > The attached script has the script that was used for testing. Here the > NUM_RECORDS count should be changed accordingly for each of the tests > and while running the test with the patch change uncomment the drop > publication command. I have don

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, 18 Feb 2025 at 16:53, Amit Kapila wrote: > > On Tue, Feb 18, 2025 at 2:24 PM vignesh C wrote: > > > > On Fri, 14 Feb 2025 at 15:36, Amit Kapila wrote: > > > > > > Did you try to measure the performance impact of this change? We can > > > try a few cases where DDL and DMLs are involved, m

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Tue, Feb 18, 2025 at 2:24 PM vignesh C wrote: > > On Fri, 14 Feb 2025 at 15:36, Amit Kapila wrote: > > > > Did you try to measure the performance impact of this change? We can > > try a few cases where DDL and DMLs are involved, missing publication > > (drop publication and recreate after a va

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Fri, 14 Feb 2025 at 15:36, Amit Kapila wrote: > > Did you try to measure the performance impact of this change? We can > try a few cases where DDL and DMLs are involved, missing publication > (drop publication and recreate after a varying number of records to > check the impact). Since we don'

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Mon, Feb 19, 2024 at 12:49 PM vignesh C wrote: > > Currently ALTER SUBSCRIPTION ... SET PUBLICATION will break the > logical replication in certain cases. This can happen as the apply > worker will get restarted after SET PUBLICATION, the apply worker will > use the existing slot and replicatio

Re: Add an option to skip loading missing publication to avoid logical replication failure

On Mon, 19 Feb 2024 at 12:48, vignesh C wrote: > > Hi, > > Currently ALTER SUBSCRIPTION ... SET PUBLICATION will break the > logical replication in certain cases. This can happen as the apply > worker will get restarted after SET PUBLICATION, the apply worker will > use the existing slot and repli

Add an option to skip loading missing publication to avoid logical replication failure

Hi, Currently ALTER SUBSCRIPTION ... SET PUBLICATION will break the logical replication in certain cases. This can happen as the apply worker will get restarted after SET PUBLICATION, the apply worker will use the existing slot and replication origin corresponding to the subscription. Now, it is p