Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-25 Thread Michael Paquier
On Tue, Oct 25, 2022 at 09:37:08AM +0200, Alvaro Herrera wrote: > Okay, so if we follow this argument, then the logical conclusion is that > this *should* be backpatched, after all. After sleeping on it and looking at all the stable branches involved, backpatched down to v10. -- Michael signatur

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-25 Thread Alvaro Herrera
On 2022-Oct-25, Tom Lane wrote: > Michael Paquier writes: > > On Mon, Oct 24, 2022 at 02:22:16PM +0200, Alvaro Herrera wrote: > >> I confess I don't understand why is it important that XLogBeginInsert is > >> called inside the critical section. It seems to me that that part is > >> only a side-e

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-24 Thread Tom Lane
Michael Paquier writes: > On Mon, Oct 24, 2022 at 02:22:16PM +0200, Alvaro Herrera wrote: >> I confess I don't understand why is it important that XLogBeginInsert is >> called inside the critical section. It seems to me that that part is >> only a side-effect of having to acquire the buffer locks

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-24 Thread Michael Paquier
On Mon, Oct 24, 2022 at 02:22:16PM +0200, Alvaro Herrera wrote: > I suppose it's a matter of whether any bugs are possible outside of > Neon. If yes, then definitely this should be backpatched. Offhand, I > don't see any. On the other hand, even if no bugs are known, then it's > still valuable t

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-24 Thread Alvaro Herrera
On 2022-Oct-13, Michael Paquier wrote: > On Wed, Oct 12, 2022 at 08:54:34PM -0400, Tom Lane wrote: > > Don't we need to back-patch these fixes? > > I guess I should, though I have finished by not doing due to the > unlikeliness of the problem, where we would need the combination of a > page evict

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-12 Thread Michael Paquier
On Wed, Oct 12, 2022 at 08:54:34PM -0400, Tom Lane wrote: > Don't we need to back-patch these fixes? I guess I should, though I have finished by not doing due to the unlikeliness of the problem, where we would need the combination of a page eviction with an error in the critical section to force a

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-12 Thread Tom Lane
Michael Paquier writes: > Thanks. This looks fine on a second look, so applied. Don't we need to back-patch these fixes? regards, tom lane

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-12 Thread Michael Paquier
On Wed, Oct 12, 2022 at 09:39:11PM +0800, Zhang Mingli wrote: > Patch added. Thanks. This looks fine on a second look, so applied. -- Michael signature.asc Description: PGP signature

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-12 Thread Zhang Mingli
HI, On Oct 12, 2022, 10:09 +0800, Michael Paquier , wrote: > > Nice catches, both of you. Let's adjust everything spotted in one > shot. Matthias' patch makes the ordering right, but the > initialization path a bit harder to follow when using a separate list. > The code is short so it does not str

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-10-11 Thread Michael Paquier
On Fri, Sep 30, 2022 at 06:22:02PM +0800, Zhang Mingli wrote: > In the same function, there is disorder of XLogBeginInsert and > START_CRIT_SECTION. > > ``` > collectordata = ptr = (char *) palloc(collector->sumsize); > >  data.ntuples = collector->ntuples; > >  if (needWal) >    XLogBeginInser

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-09-30 Thread Zhang Mingli
Hi, On Sep 8, 2022, 19:08 +0800, Matthias van de Meent , wrote: > > PFA a patch that rectifies this issue, by moving the XLogBeginInsert() > down to where 1.) we have all relevant buffers pinned and locked, and > 2.) we're in a critical section, making that part of the code > consistent with the

Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-09-30 Thread Zhang Mingli
HI, On Sep 8, 2022, 19:08 +0800, Matthias van de Meent , wrote: >  In general, this works fine, except that in ginHeapTupleFastInsert we > call XLogBeginInsert() before the last of the buffers for the eventual > record was read, thus creating a path where eviction is possible in a > `begininsert_

Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

2022-09-08 Thread Matthias van de Meent
Hi, In Neon, we've had to modify the GIN fast insertion path as attached, due to an unexpected XLOG insertion and buffer locking ordering issue. The xlog readme [0] mentions that the common order of operations is 1) pin and lock any buffers you need for the log record, then 2) start a critical se