Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-06-01 Thread Amit Langote
On Tue, Jun 1, 2021 at 6:56 PM Amit Kapila wrote: > On Mon, May 31, 2021 at 8:51 AM Amit Langote wrote: > > On Thu, May 27, 2021 at 3:36 PM Amit Kapila wrote: > > > Why do we need to move send_relation_and_attrs() call? I think it > > > doesn't matter much either way but OTOH, in the existing

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-06-01 Thread Amit Kapila
On Mon, May 31, 2021 at 8:51 AM Amit Langote wrote: > > On Thu, May 27, 2021 at 3:36 PM Amit Kapila wrote: > > On Fri, May 21, 2021 at 1:12 PM Amit Langote > > wrote: > > > > > > > Why do we need to move send_relation_and_attrs() call? I think it > > doesn't matter much either way but OTOH, in

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-30 Thread Amit Langote
On Thu, May 27, 2021 at 3:36 PM Amit Kapila wrote: > On Fri, May 21, 2021 at 1:12 PM Amit Langote wrote: > > > > Hmm, maybe get_rel_syn_entry() should explicitly set map to NULL when > > first initializing an entry. It's possible that without doing so, the > > map remains set to a garbage

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-27 Thread Amit Kapila
On Fri, May 21, 2021 at 1:12 PM Amit Langote wrote: > > Hmm, maybe get_rel_syn_entry() should explicitly set map to NULL when > first initializing an entry. It's possible that without doing so, the > map remains set to a garbage value, which causes the invalidation > callback that runs into such

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-24 Thread osumi.takami...@fujitsu.com
On Monday, May 24, 2021 12:57 PM I wrote: > On Monday, May 24, 2021 12:23 PM Amit Langote > wrote: > > On Mon, May 24, 2021 at 12:16 PM osumi.takami...@fujitsu.com > > wrote: > > > When I execute make check-world with v6 additionally, I've gotten > > > another failure. I get this about once in >

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-23 Thread osumi.takami...@fujitsu.com
On Monday, May 24, 2021 12:23 PM Amit Langote wrote: > On Mon, May 24, 2021 at 12:16 PM osumi.takami...@fujitsu.com > wrote: > > When I execute make check-world with v6 additionally, I've gotten > > another failure. I get this about once in > > 20 times of make check-world with v6. > Hmm, I

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-23 Thread Amit Langote
On Mon, May 24, 2021 at 12:16 PM osumi.takami...@fujitsu.com wrote: > On Saturday, May 22, 2021 11:58 AM Amit Langote > wrote: > > On Sat, May 22, 2021 at 11:00 AM osumi.takami...@fujitsu.com > > wrote: > > > I've checked the core file of v3's failure core and printed the entry > > > to get

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-23 Thread osumi.takami...@fujitsu.com
On Saturday, May 22, 2021 11:58 AM Amit Langote wrote: > On Sat, May 22, 2021 at 11:00 AM osumi.takami...@fujitsu.com > wrote: > > I've checked the core file of v3's failure core and printed the entry > > to get more confidence. Sorry for inappropriate measure to verify the > solution. > > > >

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-21 Thread Amit Langote
On Sat, May 22, 2021 at 11:00 AM osumi.takami...@fujitsu.com wrote: > On Friday, May 21, 2021 9:45 PM I worte: > > On Friday, May 21, 2021 4:43 PM Amit Langote > > wrote: > > > On Fri, May 21, 2021 at 3:55 PM osumi.takami...@fujitsu.com > > > wrote: > > > > But, I've detected segmentation

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-21 Thread osumi.takami...@fujitsu.com
On Friday, May 21, 2021 9:45 PM I worte: > On Friday, May 21, 2021 4:43 PM Amit Langote > wrote: > > On Fri, May 21, 2021 at 3:55 PM osumi.takami...@fujitsu.com > > wrote: > > > But, I've detected segmentation faults caused by the patch, which > > > can happen during 100_bugs.pl in

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-21 Thread osumi.takami...@fujitsu.com
On Friday, May 21, 2021 4:43 PM Amit Langote wrote: > On Fri, May 21, 2021 at 3:55 PM osumi.takami...@fujitsu.com > wrote: > > But, I've detected segmentation faults caused by the patch, which can > > happen during 100_bugs.pl in src/test/subscription. > > This happens more than one in ten

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-21 Thread Amit Langote
On Fri, May 21, 2021 at 3:55 PM osumi.takami...@fujitsu.com wrote: > On Thursday, May 20, 2021 9:59 PM Amit Langote > wrote: > > Here are updated/divided patches. > Thanks for your updates. > > But, I've detected segmentation faults caused by the patch, > which can happen during 100_bugs.pl in

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-21 Thread osumi.takami...@fujitsu.com
On Friday, May 21, 2021 3:55 PM I wrote: > On Thursday, May 20, 2021 9:59 PM Amit Langote > wrote: > > Here are updated/divided patches. > Thanks for your updates. > > But, I've detected segmentation faults caused by the patch, which can > happen during 100_bugs.pl in src/test/subscription. >

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-21 Thread osumi.takami...@fujitsu.com
On Thursday, May 20, 2021 9:59 PM Amit Langote wrote: > Here are updated/divided patches. Thanks for your updates. But, I've detected segmentation faults caused by the patch, which can happen during 100_bugs.pl in src/test/subscription. This happens more than one in ten times. This problem

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-20 Thread Amit Langote
On Thu, May 20, 2021 at 5:59 PM osumi.takami...@fujitsu.com wrote: > On Tuesday, May 18, 2021 3:30 PM Amit Langote wrote: > > While doing so, it occurred to me (maybe not for the first time) that we are > > *unnecessarily* doing send_relation_and_attrs() for a relation if the > > changes > >

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-20 Thread osumi.takami...@fujitsu.com
On Tuesday, May 18, 2021 3:30 PM Amit Langote wrote: > On Mon, May 17, 2021 at 9:45 PM osumi.takami...@fujitsu.com > wrote: > > On Monday, May 17, 2021 6:52 PM Amit Langote > wrote: > > > Both patches are attached. > > The patch for PG13 can be applied to it cleanly and the RT succeeded. > > >

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-20 Thread osumi.takami...@fujitsu.com
On Tuesday, May 18, 2021 3:30 PM Amit Langote wrote: > While doing so, it occurred to me (maybe not for the first time) that we are > *unnecessarily* doing send_relation_and_attrs() for a relation if the changes > will be published using an ancestor's schema. In that case, sending only the >

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-19 Thread osumi.takami...@fujitsu.com
On Wednesday, May 19, 2021 1:52 PM Amit Kapila wrote: > > > I am not sure but I > > > > think we should prohibit truncate on user_catalog_tables as we > > > > prohibit truncate on system catalog tables (see below [1]) if we > > > > want plugin to lock them, otherwise, as you said it might lead to

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-18 Thread Amit Kapila
On Wed, May 19, 2021 at 8:28 AM osumi.takami...@fujitsu.com wrote: > > On Wednesday, May 19, 2021 11:33 AM Amit Kapila > wrote: > > On Wed, May 19, 2021 at 7:59 AM Amit Kapila > > wrote: > > > > > > On Tue, May 18, 2021 at 5:29 PM Amit Kapila > > wrote: > > > > > > > > On Tue, May 18, 2021 at

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-18 Thread osumi.takami...@fujitsu.com
On Wednesday, May 19, 2021 11:33 AM Amit Kapila wrote: > On Wed, May 19, 2021 at 7:59 AM Amit Kapila > wrote: > > > > On Tue, May 18, 2021 at 5:29 PM Amit Kapila > wrote: > > > > > > On Tue, May 18, 2021 at 1:29 PM osumi.takami...@fujitsu.com > > > wrote: > > > > > > > > On Monday, May 17,

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-18 Thread Amit Kapila
On Wed, May 19, 2021 at 7:59 AM Amit Kapila wrote: > > On Tue, May 18, 2021 at 5:29 PM Amit Kapila wrote: > > > > On Tue, May 18, 2021 at 1:29 PM osumi.takami...@fujitsu.com > > wrote: > > > > > > On Monday, May 17, 2021 6:45 PM Amit Kapila > > > wrote: > > > > > > > > We allow taking locks

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-18 Thread Amit Kapila
On Tue, May 18, 2021 at 5:29 PM Amit Kapila wrote: > > On Tue, May 18, 2021 at 1:29 PM osumi.takami...@fujitsu.com > wrote: > > > > On Monday, May 17, 2021 6:45 PM Amit Kapila wrote: > > > > > > We allow taking locks on system catalogs, so why prohibit > > > user_catalog_tables? However, I

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-18 Thread Amit Kapila
On Tue, May 18, 2021 at 1:29 PM osumi.takami...@fujitsu.com wrote: > > On Monday, May 17, 2021 6:45 PM Amit Kapila wrote: > > > > We allow taking locks on system catalogs, so why prohibit > > user_catalog_tables? However, I agree that if we want plugins to acquire the > > lock on

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-18 Thread osumi.takami...@fujitsu.com
On Monday, May 17, 2021 6:45 PM Amit Kapila wrote: > On Fri, May 14, 2021 at 2:20 PM osumi.takami...@fujitsu.com > wrote: > > > > On Thursday, May 13, 2021 7:21 PM Amit Kapila > wrote: > > > I don't think we can reproduce it with core plugins as they don't > > > lock user catalog tables. > >

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-18 Thread Amit Langote
On Mon, May 17, 2021 at 9:45 PM osumi.takami...@fujitsu.com wrote: > On Monday, May 17, 2021 6:52 PM Amit Langote wrote: > > Both patches are attached. > The patch for PG13 can be applied to it cleanly and the RT succeeded. > > I have few really minor comments on your comments in the patch. > >

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-17 Thread osumi.takami...@fujitsu.com
On Monday, May 17, 2021 6:52 PM Amit Langote wrote: > On Mon, May 17, 2021 at 6:13 PM Amit Kapila > wrote: > > On Fri, May 14, 2021 at 12:44 PM Amit Langote > wrote: > > > On Thu, May 13, 2021 at 7:43 PM Amit Kapila > wrote: > > > > > > > Also, don't we need to free the > > > > entire map as

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-17 Thread Amit Langote
On Mon, May 17, 2021 at 6:13 PM Amit Kapila wrote: > On Fri, May 14, 2021 at 12:44 PM Amit Langote wrote: > > On Thu, May 13, 2021 at 7:43 PM Amit Kapila wrote: > > > > > Also, don't we need to free the > > > entire map as suggested by me? > > > > Yes, I had missed that too. Addressed in the

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-17 Thread Amit Kapila
On Fri, May 14, 2021 at 2:20 PM osumi.takami...@fujitsu.com wrote: > > On Thursday, May 13, 2021 7:21 PM Amit Kapila wrote: > > I don't think we can reproduce it with core plugins as they don't lock user > > catalog tables. > OK. My current understanding about how the deadlock happens is below.

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-17 Thread Amit Kapila
On Fri, May 14, 2021 at 12:44 PM Amit Langote wrote: > > On Thu, May 13, 2021 at 7:43 PM Amit Kapila wrote: > > > Also, don't we need to free the > > entire map as suggested by me? > > Yes, I had missed that too. Addressed in the updated patch. > + relentry->map =

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-14 Thread osumi.takami...@fujitsu.com
On Thursday, May 13, 2021 7:21 PM Amit Kapila wrote: > On Thu, May 13, 2021 at 11:15 AM osumi.takami...@fujitsu.com > wrote: > > > > I tried the following scenarios for trying to reproduce this. > > Scenario2: > > (1) set up 1 publisher and 1 subscriber > > (2) create table with

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-14 Thread Amit Langote
Takamichi-san, On Fri, May 14, 2021 at 11:19 AM osumi.takami...@fujitsu.com wrote: > On Thursday, May 13, 2021 7:43 PM Amit Kapila wrote: > > On Tue, Apr 20, 2021 at 8:36 AM Amit Langote > > wrote: > > > Back in: > > https://www.postgresql.org/message-id/CA%2BHiwqEeU19iQgjN6HF1HTP > > U0L5%2 >

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-14 Thread Amit Langote
On Thu, May 13, 2021 at 7:43 PM Amit Kapila wrote: > On Tue, Apr 20, 2021 at 8:36 AM Amit Langote wrote: > > On Sat, Apr 17, 2021 at 1:30 PM Amit Kapila wrote: > > > On Fri, Apr 16, 2021 at 11:24 PM Andres Freund > > > wrote:> > This made me take a brief look at pgoutput.c - maybe I am > > >

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-13 Thread osumi.takami...@fujitsu.com
On Thursday, May 13, 2021 7:43 PM Amit Kapila wrote: > On Tue, Apr 20, 2021 at 8:36 AM Amit Langote > wrote: > > Back in: > https://www.postgresql.org/message-id/CA%2BHiwqEeU19iQgjN6HF1HTP > U0L5%2 > > BJxyS5CmxaOVGNXBAfUY06Q%40mail.gmail.com > > > > I had proposed to move the map creation from

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-13 Thread Amit Kapila
On Tue, Apr 20, 2021 at 8:36 AM Amit Langote wrote: > > On Sat, Apr 17, 2021 at 1:30 PM Amit Kapila wrote: > > On Fri, Apr 16, 2021 at 11:24 PM Andres Freund wrote:> > > > This made me take a brief look at pgoutput.c - maybe I am missing > > > something, but how is the following not a memory

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-13 Thread Amit Kapila
On Thu, May 13, 2021 at 11:15 AM osumi.takami...@fujitsu.com wrote: > > On Thursday, April 29, 2021 2:31 PM Amit Kapila > wrote: > > I am not so sure about it because I think we don't have any example of > > user_catalog_tables in the core code. This is the reason I was kind of > > looking > >

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-12 Thread osumi.takami...@fujitsu.com
On Thursday, April 29, 2021 2:31 PM Amit Kapila wrote: > I am not so sure about it because I think we don't have any example of > user_catalog_tables in the core code. This is the reason I was kind of looking > towards Andres to clarify this. Right now, if the user performs TRUNCATE on >

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-05-12 Thread Amit Langote
Takamichi-san, On Tue, Apr 27, 2021 at 9:37 PM osumi.takami...@fujitsu.com wrote: > On Tuesday, April 20, 2021 12:07 PM Amit Langote > wrote: > > On Sat, Apr 17, 2021 at 1:30 PM Amit Kapila > > wrote: > > > On Fri, Apr 16, 2021 at 11:24 PM Andres Freund > > > wrote:> > This made me take a

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-28 Thread Amit Kapila
On Wed, Apr 28, 2021 at 5:36 PM osumi.takami...@fujitsu.com wrote: > > On Monday, April 26, 2021 2:05 PM Amit Kapila wrote: > > On Fri, Apr 23, 2021 at 8:03 PM osumi.takami...@fujitsu.com > > wrote: > > I think we are allowed to decode the operations on user catalog tables > > because we are

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-28 Thread osumi.takami...@fujitsu.com
On Monday, April 26, 2021 2:05 PM Amit Kapila wrote: > On Fri, Apr 23, 2021 at 8:03 PM osumi.takami...@fujitsu.com > wrote: > > > > On Saturday, April 17, 2021 4:13 PM Amit Kapila > wrote: > > > > I also don't find a test for this. It is introduced in > > > > 5dfd1e5a6696, wrote by Simon

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-27 Thread osumi.takami...@fujitsu.com
On Tuesday, April 20, 2021 12:07 PM Amit Langote wrote: > On Sat, Apr 17, 2021 at 1:30 PM Amit Kapila > wrote: > > On Fri, Apr 16, 2021 at 11:24 PM Andres Freund > > wrote:> > This made me take a brief look at pgoutput.c - maybe I am > > missing > > > something, but how is the following not a

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-25 Thread Amit Kapila
On Fri, Apr 23, 2021 at 8:03 PM osumi.takami...@fujitsu.com wrote: > > On Saturday, April 17, 2021 4:13 PM Amit Kapila > wrote: > > > I also don't find a test for this. It is introduced in 5dfd1e5a6696, > > > wrote by Simon Riggs, Marco Nenciarini and Peter Eisentraut. Maybe > > > they can

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-24 Thread Japin Li
On Fri, 23 Apr 2021 at 22:33, osumi.takami...@fujitsu.com wrote: > On Saturday, April 17, 2021 4:13 PM Amit Kapila > wrote: >> On Sat, Apr 17, 2021 at 12:05 PM Japin Li wrote: >> > >> > On Sat, 17 Apr 2021 at 14:09, Amit Kapila >> wrote: >> > > On Thu, Apr 15, 2021 at 4:53 PM Amit Kapila

RE: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-23 Thread osumi.takami...@fujitsu.com
On Saturday, April 17, 2021 4:13 PM Amit Kapila wrote: > On Sat, Apr 17, 2021 at 12:05 PM Japin Li wrote: > > > > On Sat, 17 Apr 2021 at 14:09, Amit Kapila > wrote: > > > On Thu, Apr 15, 2021 at 4:53 PM Amit Kapila > wrote: > > >> > > >> On Thu, Apr 15, 2021 at 4:00 PM Japin Li wrote: > > >>

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-19 Thread Amit Langote
On Sat, Apr 17, 2021 at 1:30 PM Amit Kapila wrote: > On Fri, Apr 16, 2021 at 11:24 PM Andres Freund wrote:> > > This made me take a brief look at pgoutput.c - maybe I am missing > > something, but how is the following not a memory leak? > > > > static void > >

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-19 Thread Amit Kapila
On Sat, Apr 17, 2021 at 12:01 PM Amit Kapila wrote: > > On Fri, Apr 16, 2021 at 11:24 PM Andres Freund wrote: > > > > > > > I think it is also important to *not* acquire any lock on relation > > > otherwise it can lead to some sort of deadlock or infinite wait in the > > > decoding process.

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-17 Thread Amit Kapila
On Sat, Apr 17, 2021 at 12:05 PM Japin Li wrote: > > On Sat, 17 Apr 2021 at 14:09, Amit Kapila wrote: > > On Thu, Apr 15, 2021 at 4:53 PM Amit Kapila wrote: > >> > >> On Thu, Apr 15, 2021 at 4:00 PM Japin Li wrote: > >> > > >> > The RelationIdGetRelation() comment says: > >> > > >> > > Caller

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-17 Thread Japin Li
On Sat, 17 Apr 2021 at 14:09, Amit Kapila wrote: > On Thu, Apr 15, 2021 at 4:53 PM Amit Kapila wrote: >> >> On Thu, Apr 15, 2021 at 4:00 PM Japin Li wrote: >> > >> > The RelationIdGetRelation() comment says: >> > >> > > Caller should eventually decrement count. (Usually, >> > > that happens

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-17 Thread Amit Kapila
On Fri, Apr 16, 2021 at 11:24 PM Andres Freund wrote: > > > > I think it is also important to *not* acquire any lock on relation > > otherwise it can lead to some sort of deadlock or infinite wait in the > > decoding process. Consider a case for operations like Truncate (or if > > the user has

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-17 Thread Amit Kapila
On Thu, Apr 15, 2021 at 4:53 PM Amit Kapila wrote: > > On Thu, Apr 15, 2021 at 4:00 PM Japin Li wrote: > > > > The RelationIdGetRelation() comment says: > > > > > Caller should eventually decrement count. (Usually, > > > that happens by calling RelationClose().) > > > > However, it doesn't do it

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-16 Thread Amit Kapila
On Fri, Apr 16, 2021 at 11:24 PM Andres Freund wrote: > > > I think it is also important to *not* acquire any lock on relation > > otherwise it can lead to some sort of deadlock or infinite wait in the > > decoding process. Consider a case for operations like Truncate (or if > > the user has

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-16 Thread Andres Freund
Hi, On 2021-04-16 08:42:40 +0530, Amit Kapila wrote: > I think it is because relation_open expects either caller to have a > lock on the relation or don't use 'NoLock' lockmode. AFAIU, we don't > need to acquire a lock on relation while decoding changes from WAL > because it uses a historic

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-15 Thread Amit Kapila
On Thu, Apr 15, 2021 at 10:56 PM Tom Lane wrote: > > Amit Kapila writes: > > On Thu, Apr 15, 2021 at 4:00 PM Japin Li wrote: > >> > >> The RelationIdGetRelation() comment says: > >> > > Caller should eventually decrement count. (Usually, > > that happens by calling RelationClose().) > >> > >>

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-15 Thread Tom Lane
Amit Kapila writes: > On Thu, Apr 15, 2021 at 4:00 PM Japin Li wrote: >> >> The RelationIdGetRelation() comment says: >> > Caller should eventually decrement count. (Usually, > that happens by calling RelationClose().) >> >> However, it doesn't do it in ReorderBufferProcessTXN(). >> I think

Re: Forget close an open relation in ReorderBufferProcessTXN()

2021-04-15 Thread Amit Kapila
On Thu, Apr 15, 2021 at 4:00 PM Japin Li wrote: > > The RelationIdGetRelation() comment says: > > > Caller should eventually decrement count. (Usually, > > that happens by calling RelationClose().) > > However, it doesn't do it in ReorderBufferProcessTXN(). > I think we should close it, here is a