On Wed, Sep 5, 2018 at 12:26 PM Alexander Korotkov
<a.korot...@postgrespro.ru> wrote:
> On Wed, Sep 5, 2018 at 1:45 AM R, Siva <sivas...@amazon.com> wrote:
> > On Tue, Sep 4, 2018 at 09:16 PM, Alexander Korotkov
> > <a.korot...@postgrespro.ru> wrote:
> > > Do you have a test scenario for reproduction of this issue? We need
> > > it to ensure that fix is correct.
> >
> > Unfortunately, I do not have a way of reproducing this issue.
> > So far I have tried a workload consisting of inserts (of the
> > same attribute value that is indexed), batch deletes of rows
> > and vacuum interleaved with engine crash/restarts.
>
> Issue reproduction and testing is essential for bug fix. Remember
> last time you reported GIN bug [1], after issue reproduction it
> appears that we have more things to fix. I's quite clear for me that
> if segment list contains GIN_SEGMENT_INSERT before GIN_SEGMENT_DELETE,
> then it might lead to wrong behavior in ginRedoRecompress(). But it's
> not yet clear to understand what code patch could lead to such segment
> list... I'll explore code more and probably will come with some idea.
Aha, I've managed to reproduce this.
1. Apply ginRedoRecompress-asserts.patch, which add assertions to
ginRedoRecompress() detecting past opaque writes.
2. Setup streaming replication.
3. Execute following on the master.
create or replace function test () returns void as $$
declare
i int;
begin
FOR i IN 1..1000 LOOP
insert into test values ('{1}');
end loop;
end
$$ language plpgsql;
create table test (a int[]);
insert into test (select '{}'::int[] from generate_series(1,10000));
insert into test (select '{1}'::int[] from generate_series(1,100000));
create index test_idx on test using gin(a) with (fastupdate = off);
delete from test where a = '{}'::int[];
vacuum test;
select test();
So, since we managed to reproduce this, I'm going to test and review your fix.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c
index 7a1e94a1d56..301c527cab9 100644
--- a/src/backend/access/gin/ginxlog.c
+++ b/src/backend/access/gin/ginxlog.c
@@ -276,6 +276,7 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data)
break;
case GIN_SEGMENT_INSERT:
+ Assert(segptr + newsegsize + szleft < PageGetSpecialPointer(page));
/* make room for the new segment */
memmove(segptr + newsegsize, segptr, szleft);
/* copy the new segment in place */
@@ -285,6 +286,8 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data)
break;
case GIN_SEGMENT_REPLACE:
+ Assert(segptr + newsegsize + (szleft - segsize) < PageGetSpecialPointer(page));
+ Assert(segptr + szleft < PageGetSpecialPointer(page));
/* shift the segments that follow */
memmove(segptr + newsegsize,
segptr + segsize,
