Re: Assertion failure in SnapBuildInitialSnapshot()

2024-06-11 Thread Alexander Lakhin
Hello, 01.02.2024 21:20, vignesh C wrote: The patch which you submitted has been awaiting your attention for quite some time now. As such, we have moved it to "Returned with Feedback" and removed it from the reviewing queue. Depending on timing, this may be reversible. Kindly address the

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