From: "R, Siva" <sivasubr@amazon.com>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
CC: Sawada Masahiko <sawada.mshk@gmail.com>,
        pgsql-hackers
	<pgsql-hackers@postgresql.org>
Subject: Re: Bug in ginRedoRecompress that causes opaque data on page to be
 overrun
Thread-Topic: Bug in ginRedoRecompress that causes opaque data on page to be
 overrun
Thread-Index: 
 AQHURITWziSocye1S0ygUmXh1SVXsaTgquEAgAANUlGAALPkAIAAJPwAgAAo8QCAAHJWAIAACpEYgADO3wCAAId0ig==
Date: Thu, 6 Sep 2018 18:02:20 +0000
Message-ID: <1536256940595.4374@amazon.com>
References: <1536091151804.6588@amazon.com>
 <CAH2-WzneRxhYubPMh-x0L7h_1iH5kddVH5HWx2RFWKh=ELdgzw@mail.gmail.com>
 <1536101117759.63175@amazon.com>
 <CAPpHfdv1FkM3Lk7xGh1Wmo+u7xKV9GqNzLkc2mxiiX5OOqkN1Q@mail.gmail.com>
 <CAPpHfduH0P9onLnkGGMM_Pze5J47qjHFxFxouWkhYxBebq0mrQ@mail.gmail.com>
 <CAPpHfdtS2hHJBT4bOnk2FJDzE1yCvRDveAjrW_Oi3aAK1MtrGw@mail.gmail.com>
 <CAPpHfduRz9qeqxVQh+2FAj+uQ040qzijYOM6Lc2o7873UzVL3w@mail.gmail.com>
 <1536184145340.65860@amazon.com>,<CAPpHfdv3HLMWY8W8eh-djG6ar_coZzQnVmRVYh3-EtrtxccgAw@mail.gmail.com>
In-Reply-To: 
 <CAPpHfdv3HLMWY8W8eh-djG6ar_coZzQnVmRVYh3-EtrtxccgAw@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Precedence: bulk

On Tue, Sep 6, 2018 at 09:53 AM, Alexander Korotkov <a.korotkov@postgrespro=
.ru> wrote:=0A=
> With (fastupdate =3D on) GIN performs bulk update of posting lists,=0A=
> inserting multiple tuples at once if possible.  With (fastupdate =3D=0A=
> off) GIN always inserts tuples one-by-one.  It might be still possible=0A=
> to reproduce the issue with (fastupdate =3D off), but it seems even=0A=
> harder.=0A=
=0A=
Ah I see. This is cool, I will keep in mind for future testing. Thanks!=0A=
=0A=
> BTW, I've tried the patch you've posted. On my test case it fails=0A=
> with following assertion.=0A=
> TRAP: FailedAssertion("!(a_action =3D=3D 2)", File: "ginxlog.c", Line: 24=
3)=0A=
=0A=
> I thought about fixing this issue more, and I decided we can fix it in=0A=
> less invasive way.  Once modification is started we can copy tail of=0A=
> the page into separately allocated chunk of memory, and the use it as=0A=
> the source of original segments.  See the patch attached.=0A=
=0A=
I'm also running into this assert with the workload, I think my patch is=0A=
not handling the case where the action is add items on the last segment=0A=
of the page correctly. I'm still investigating the issue further to find th=
e=0A=
source of the bug.=0A=
=0A=
Meanwhile I reviewed your patch and it looks good to me. I agree that =0A=
copying out the entire tail out to the scratch space in one shot vs copying=
=0A=
out every segment reduces the number of memcpy calls and simplifies=0A=
the solution overall. Let us go ahead with this patch.=0A=
=0A=
Best=0A=
Siva=0A=