Re: Assertion failure in SnapBuildInitialSnapshot()

2024-02-01 Thread vignesh C
On Thu, 11 Jan 2024 at 19:55, vignesh C wrote: > > On Thu, 9 Feb 2023 at 12:02, Masahiko Sawada wrote: > > > > On Wed, Feb 8, 2023 at 1:13 PM Amit Kapila wrote: > > > > > > On Wed, Feb 8, 2023 at 1:19 AM Andres Freund wrote: > > > > > > > > On 2023-02-01 11:23:57 +0530, Amit Kapila wrote: > >

Re: Assertion failure in SnapBuildInitialSnapshot()

2024-01-11 Thread vignesh C
On Thu, 9 Feb 2023 at 12:02, Masahiko Sawada wrote: > > On Wed, Feb 8, 2023 at 1:13 PM Amit Kapila wrote: > > > > On Wed, Feb 8, 2023 at 1:19 AM Andres Freund wrote: > > > > > > On 2023-02-01 11:23:57 +0530, Amit Kapila wrote: > > > > On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada > > > >

Re: Assertion failure in SnapBuildInitialSnapshot()

2024-01-05 Thread Robert Haas
This thread has gone for about a year here without making any progress, which isn't great. On Tue, Feb 7, 2023 at 2:49 PM Andres Freund wrote: > Hm. It's worrysome to now hold ProcArrayLock exclusively while iterating over > the slots. ReplicationSlotsComputeRequiredXmin() can be called at a >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-07-20 Thread Daniel Gustafsson
> On 9 Feb 2023, at 07:32, Masahiko Sawada wrote: > I've attached the patch of this idea for discussion. Amit, Andres: have you had a chance to look at the updated version of this patch? -- Daniel Gustafsson

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-02-08 Thread Masahiko Sawada
On Wed, Feb 8, 2023 at 1:13 PM Amit Kapila wrote: > > On Wed, Feb 8, 2023 at 1:19 AM Andres Freund wrote: > > > > On 2023-02-01 11:23:57 +0530, Amit Kapila wrote: > > > On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada > > > wrote: > > > > > > > > Attached updated patches. > > > > > > > > > >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-02-07 Thread Amit Kapila
On Wed, Feb 8, 2023 at 1:35 AM Andres Freund wrote: > > On 2023-02-07 11:49:03 -0800, Andres Freund wrote: > > On 2023-02-01 11:23:57 +0530, Amit Kapila wrote: > > > On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada > > > wrote: > > > > > > > > Attached updated patches. > > > > > > > > > >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-02-07 Thread Amit Kapila
On Wed, Feb 8, 2023 at 1:19 AM Andres Freund wrote: > > On 2023-02-01 11:23:57 +0530, Amit Kapila wrote: > > On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada > > wrote: > > > > > > Attached updated patches. > > > > > > > Thanks, Andres, others, do you see a better way to fix this problem? I > >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-02-07 Thread Andres Freund
Hi, On 2023-02-07 11:49:03 -0800, Andres Freund wrote: > On 2023-02-01 11:23:57 +0530, Amit Kapila wrote: > > On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada > > wrote: > > > > > > Attached updated patches. > > > > > > > Thanks, Andres, others, do you see a better way to fix this problem? I > >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-02-07 Thread Andres Freund
Hi, On 2023-02-01 11:23:57 +0530, Amit Kapila wrote: > On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada wrote: > > > > Attached updated patches. > > > > Thanks, Andres, others, do you see a better way to fix this problem? I > have reproduced it manually and the steps are shared at [1] and >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-02-07 Thread Amit Kapila
On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada wrote: > > Attached updated patches. > In back-branch patches, the change is as below: + * + * NB: the caller must hold ProcArrayLock in an exclusive mode regardless of + * already_locked which is unused now but kept for ABI compatibility. */

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-31 Thread Amit Kapila
On Tue, Jan 31, 2023 at 6:08 PM Masahiko Sawada wrote: > > Attached updated patches. > Thanks, Andres, others, do you see a better way to fix this problem? I have reproduced it manually and the steps are shared at [1] and Sawada-San also reproduced it, see [2]. [1] -

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-31 Thread Masahiko Sawada
On Tue, Jan 31, 2023 at 3:59 PM Masahiko Sawada wrote: > > On Tue, Jan 31, 2023 at 3:56 PM Amit Kapila wrote: > > > > On Tue, Jan 31, 2023 at 11:12 AM Masahiko Sawada > > wrote: > > > > > > On Mon, Jan 30, 2023 at 9:41 PM Masahiko Sawada > > > wrote: > > > > > > I've attached patches for

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-30 Thread Masahiko Sawada
On Tue, Jan 31, 2023 at 3:56 PM Amit Kapila wrote: > > On Tue, Jan 31, 2023 at 11:12 AM Masahiko Sawada > wrote: > > > > On Mon, Jan 30, 2023 at 9:41 PM Masahiko Sawada > > wrote: > > > > I've attached patches for HEAD and backbranches. Please review them. > > > > Shall we add a comment like

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-30 Thread Amit Kapila
On Tue, Jan 31, 2023 at 11:12 AM Masahiko Sawada wrote: > > On Mon, Jan 30, 2023 at 9:41 PM Masahiko Sawada wrote: > > I've attached patches for HEAD and backbranches. Please review them. > Shall we add a comment like the one below in ReplicationSlotsComputeRequiredXmin()? diff --git

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-30 Thread Masahiko Sawada
On Mon, Jan 30, 2023 at 9:41 PM Masahiko Sawada wrote: > > On Mon, Jan 30, 2023 at 8:30 PM Amit Kapila wrote: > > > > On Fri, Jan 27, 2023 at 4:31 PM Hayato Kuroda (Fujitsu) > > wrote: > > > > > > Thank you for making the patch! I'm still considering whether this > > > approach is > > >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-30 Thread Masahiko Sawada
On Mon, Jan 30, 2023 at 8:24 PM Amit Kapila wrote: > > On Thu, Dec 8, 2022 at 8:17 AM Masahiko Sawada wrote: > > > > On Mon, Nov 21, 2022 at 4:31 PM Amit Kapila wrote: > > > > One idea to fix this issue is that in > > ReplicationSlotsComputeRequiredXmin(), we compute the minimum xmin > > while

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-30 Thread Masahiko Sawada
On Mon, Jan 30, 2023 at 8:30 PM Amit Kapila wrote: > > On Fri, Jan 27, 2023 at 4:31 PM Hayato Kuroda (Fujitsu) > wrote: > > > > Thank you for making the patch! I'm still considering whether this approach > > is > > correct, but I can put a comment to your patch anyway. > > > > ``` > > -

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-30 Thread Amit Kapila
On Fri, Jan 27, 2023 at 4:31 PM Hayato Kuroda (Fujitsu) wrote: > > Thank you for making the patch! I'm still considering whether this approach is > correct, but I can put a comment to your patch anyway. > > ``` > - Assert(!already_locked || LWLockHeldByMe(ProcArrayLock)); > - > - if

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-30 Thread Amit Kapila
On Mon, Jan 30, 2023 at 11:34 AM Amit Kapila wrote: > > I have reproduced it manually. For this, I had to manually make the > debugger call ReplicationSlotsComputeRequiredXmin(false) via path > SnapBuildProcessRunningXacts()->LogicalIncreaseXminForSlot()->LogicalConfirmReceivedLocation() >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-30 Thread Amit Kapila
On Thu, Dec 8, 2022 at 8:17 AM Masahiko Sawada wrote: > > On Mon, Nov 21, 2022 at 4:31 PM Amit Kapila wrote: > > One idea to fix this issue is that in > ReplicationSlotsComputeRequiredXmin(), we compute the minimum xmin > while holding both ProcArrayLock and ReplicationSlotControlLock, and >

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-29 Thread Amit Kapila
On Mon, Jan 30, 2023 at 10:27 AM Amit Kapila wrote: > > On Sun, Jan 29, 2023 at 9:15 PM Masahiko Sawada wrote: > > > > On Sat, Jan 28, 2023 at 11:54 PM Hayato Kuroda (Fujitsu) > > wrote: > > > > > > Dear Amit, Sawada-san, > > > > > > I have also reproduced the failure on PG15 with some debug

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-29 Thread Amit Kapila
On Sun, Jan 29, 2023 at 9:15 PM Masahiko Sawada wrote: > > On Sat, Jan 28, 2023 at 11:54 PM Hayato Kuroda (Fujitsu) > wrote: > > > > Dear Amit, Sawada-san, > > > > I have also reproduced the failure on PG15 with some debug log, and I > > agreed that > > somebody changed

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-29 Thread Masahiko Sawada
On Sat, Jan 28, 2023 at 11:54 PM Hayato Kuroda (Fujitsu) wrote: > > Dear Amit, Sawada-san, > > I have also reproduced the failure on PG15 with some debug log, and I agreed > that > somebody changed procArray->replication_slot_xmin to InvalidTransactionId. > > > > The same assertion failure has

RE: Assertion failure in SnapBuildInitialSnapshot()

2023-01-28 Thread Hayato Kuroda (Fujitsu)
Dear Amit, Sawada-san, I have also reproduced the failure on PG15 with some debug log, and I agreed that somebody changed procArray->replication_slot_xmin to InvalidTransactionId. > > The same assertion failure has been reported on another thread[1]. > > Since I could reproduce this issue

RE: Assertion failure in SnapBuildInitialSnapshot()

2023-01-27 Thread Hayato Kuroda (Fujitsu)
Dear Sawada-san, Thank you for making the patch! I'm still considering whether this approach is correct, but I can put a comment to your patch anyway. ``` - Assert(!already_locked || LWLockHeldByMe(ProcArrayLock)); - - if (!already_locked) - LWLockAcquire(ProcArrayLock,

Re: Assertion failure in SnapBuildInitialSnapshot()

2023-01-26 Thread Amit Kapila
On Thu, Dec 8, 2022 at 8:17 AM Masahiko Sawada wrote: > > The same assertion failure has been reported on another thread[1]. > Since I could reproduce this issue several times in my environment > I've investigated the root cause. > > I think there is a race condition of updating >

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-12-07 Thread Masahiko Sawada
On Mon, Nov 21, 2022 at 4:31 PM Amit Kapila wrote: > > On Sat, Nov 19, 2022 at 6:35 AM Andres Freund wrote: > > > > On 2022-11-18 11:20:36 +0530, Amit Kapila wrote: > > > Okay, updated the patch accordingly. > > > > Assuming it passes tests etc, this'd work for me. > > > > Thanks, Pushed. The

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-20 Thread Amit Kapila
On Sat, Nov 19, 2022 at 6:35 AM Andres Freund wrote: > > On 2022-11-18 11:20:36 +0530, Amit Kapila wrote: > > Okay, updated the patch accordingly. > > Assuming it passes tests etc, this'd work for me. > Thanks, Pushed. -- With Regards, Amit Kapila.

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-18 Thread Andres Freund
Hi, On 2022-11-18 11:20:36 +0530, Amit Kapila wrote: > Okay, updated the patch accordingly. Assuming it passes tests etc, this'd work for me. - Andres

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-17 Thread Amit Kapila
On Thu, Nov 17, 2022 at 11:15 PM Andres Freund wrote: > > On 2022-11-17 10:44:18 +0530, Amit Kapila wrote: > > On Wed, Nov 16, 2022 at 11:56 PM Andres Freund wrote: > > > On 2022-11-16 14:22:01 +0530, Amit Kapila wrote: > > > > On Wed, Nov 16, 2022 at 7:30 AM Andres Freund > > > > wrote: > > >

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-17 Thread Andres Freund
Hi, On 2022-11-17 10:44:18 +0530, Amit Kapila wrote: > On Wed, Nov 16, 2022 at 11:56 PM Andres Freund wrote: > > On 2022-11-16 14:22:01 +0530, Amit Kapila wrote: > > > On Wed, Nov 16, 2022 at 7:30 AM Andres Freund wrote: > > > > On 2022-11-15 16:20:00 +0530, Amit Kapila wrote: > > > > > On Tue,

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-16 Thread Amit Kapila
On Wed, Nov 16, 2022 at 11:56 PM Andres Freund wrote: > > On 2022-11-16 14:22:01 +0530, Amit Kapila wrote: > > On Wed, Nov 16, 2022 at 7:30 AM Andres Freund wrote: > > > > > > On 2022-11-15 16:20:00 +0530, Amit Kapila wrote: > > > > On Tue, Nov 15, 2022 at 8:08 AM Andres Freund > > > > wrote:

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-16 Thread Amit Kapila
On Wed, Nov 16, 2022 at 11:56 PM Andres Freund wrote: > > On 2022-11-16 14:22:01 +0530, Amit Kapila wrote: > > On Wed, Nov 16, 2022 at 7:30 AM Andres Freund wrote: > > > > > > On 2022-11-15 16:20:00 +0530, Amit Kapila wrote: > > > > On Tue, Nov 15, 2022 at 8:08 AM Andres Freund > > > > wrote:

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-16 Thread Andres Freund
Hi, On 2022-11-16 14:22:01 +0530, Amit Kapila wrote: > On Wed, Nov 16, 2022 at 7:30 AM Andres Freund wrote: > > > > On 2022-11-15 16:20:00 +0530, Amit Kapila wrote: > > > On Tue, Nov 15, 2022 at 8:08 AM Andres Freund wrote: > > > > nor do we enforce in an obvious place that we > > > > don't

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-16 Thread Amit Kapila
On Wed, Nov 16, 2022 at 7:30 AM Andres Freund wrote: > > On 2022-11-15 16:20:00 +0530, Amit Kapila wrote: > > On Tue, Nov 15, 2022 at 8:08 AM Andres Freund wrote: > > > nor do we enforce in an obvious place that we > > > don't already hold a snapshot. > > > > > > > We have a check for

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-15 Thread Andres Freund
Hi, On 2022-11-15 16:20:00 +0530, Amit Kapila wrote: > On Tue, Nov 15, 2022 at 8:08 AM Andres Freund wrote: > > nor do we enforce in an obvious place that we > > don't already hold a snapshot. > > > > We have a check for (FirstXactSnapshot == NULL) in >

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-15 Thread Amit Kapila
On Tue, Nov 15, 2022 at 6:55 AM Andres Freund wrote: > > On 2022-11-10 16:04:40 +0530, Amit Kapila wrote: > > I don't have any good ideas on how to proceed with this. Any thoughts > > on this would be helpful? > > One thing worth doing might be to convert the assertion path into an elog(), >

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-15 Thread Amit Kapila
On Tue, Nov 15, 2022 at 8:08 AM Andres Freund wrote: > > On 2022-11-14 17:25:31 -0800, Andres Freund wrote: > > Hm, also, shouldn't the patch adding CRS_USE_SNAPSHOT have copied more of > > SnapBuildExportSnapshot()? Why aren't the error checks for > > SnapBuildExportSnapshot() needed? Why don't

RE: Assertion failure in SnapBuildInitialSnapshot()

2022-11-14 Thread Takamichi Osumi (Fujitsu)
Hi, On Tuesday, November 15, 2022 10:26 AM Andres Freund wrote: > On 2022-11-10 16:04:40 +0530, Amit Kapila wrote: > > I don't have any good ideas on how to proceed with this. Any thoughts > > on this would be helpful? > > One thing worth doing might be to convert the assertion path into an

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-14 Thread Thomas Munro
On Tue, Nov 15, 2022 at 3:38 PM Andres Freund wrote: > On 2022-11-14 17:25:31 -0800, Andres Freund wrote: > Another theory: I dimly remember Thomas mentioning that there's some different > behaviour of xlogreader during shutdown as part of the v15 changes. I don't > quite remember what the

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-14 Thread Andres Freund
Hi, On 2022-11-14 17:25:31 -0800, Andres Freund wrote: > Hm, also, shouldn't the patch adding CRS_USE_SNAPSHOT have copied more of > SnapBuildExportSnapshot()? Why aren't the error checks for > SnapBuildExportSnapshot() needed? Why don't we need to set XactReadOnly? Which > transactions are we

Re: Assertion failure in SnapBuildInitialSnapshot()

2022-11-14 Thread Andres Freund
Hi, On 2022-11-10 16:04:40 +0530, Amit Kapila wrote: > I don't have any good ideas on how to proceed with this. Any thoughts > on this would be helpful? One thing worth doing might be to convert the assertion path into an elog(), mentioning both xids (or add a framework for things like

Assertion failure in SnapBuildInitialSnapshot()

2022-11-10 Thread Amit Kapila
Hi, Thomas has reported this failure in an email [1] and shared the following links offlist with me: https://cirrus-ci.com/task/5311549010083840 https://api.cirrus-ci.com/v1/artifact/task/5311549010083840/testrun/build/testrun/subscription/100_bugs/log/100_bugs_twoways.log