pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: R, Siva <sivasubr@amazon.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Bug in gin insert redo code path during re-compression of empty gin data leaf pages
Date: Tue, 17 Jul 2018 22:40:12 +0000
Message-ID: <1531867212836.63354@amazon.com> (raw)

Hi,
We came across an issue during replay of a gin insert record on a pre-9.4 uncompressed data leaf page that does not have any items in it. The engine version where the replay is done is 9.6.3. The redo logic attempts to compress the existing page items before trying to insert  new items from the WAL record[1]. In this particular case, we noticed that if the data leaf page does not have any items in it (see below on how such a page can be present), the compression should not be attempted as the algorithm expects the page to have at least one item [2]. This will lead to incorrect data size set on the page and also makes the assertion that expects npacked and nuncompressed to be equal, false [3].

In Postgres 9.3, when the gin posting tree is vacuumed, the pages that are in the leftmost and rightmost branches are not deleted [4]. These empty pages can be part of the database after upgrading to 9.6.3. We verified this by doing the following test:

Step 1: On Postgresql 9.3, create a table with gin index, insert and delete some data followed by vacuum.

* CREATE EXTENSION IF NOT EXISTS pg_trgm;
* CREATE TABLE gin_test (first_name text, last_name text);
* INSERT INTO gin_test SELECT md5(random()::text), md5(random()::text) FROM (SELECT * FROM generate_series(1,100000) AS id) AS x;
* CREATE INDEX gin_t_idx ON gin_test USING gin (first_name gin_trgm_ops, last_name gin_trgm_ops) WITH (fastupdate = OFF);
* DELETE FROM gin_test;
* VACUUM gin_test;


Step 2: Upgrade the database to 9.6.3 using pg_upgrade

* pg_upgrade -b $93_POSTGRES_DIR/bin -B $96_POSTGRES_DIR/bin -d $93_DATADIR -D $96_DATADIR


Step 3: Check the gin page opaque information of a uncompressed data leaf page to see if it has no items

* CREATE EXTENSION IF NOT EXISTS pageinspect;
* SELECT * FROM gin_metapage_info(get_raw_page('gin_t_idx', 0));  >>> Get number of data pages
* SELECT * FROM gin_page_opaque_info(get_raw_page('gin_t_idx', 8000)); >>> some block number close to total number of blocks
    * This will return a page that has rightlink 2^32-1, maxoff 0 and flags {data,leaf}.

Attached is a patch that will skip compression of the posting list in the case that the data leaf page is empty.
Please let us know any comments or feedback. Thanks!

Best
Siva


*References:*
[1] - https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/access/gin/ginxlog.c;h=a40f...
[2] - https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/access/gin/ginpostinglist.c...
[3] - https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/access/gin/ginxlog.c;h=a40f...
[4] - https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/access/gin/ginvacuum.c;h=31...

Attachments:

skip-compression-of-pre-9.4-empty-gin-data-leaf-page-replay_v1.patch (application/octet-stream, 1.7K) download From 956a533c5ec3171485624461cc13f786dbfc15e5 Mon Sep 17 00:00:00 2001 From: Siva R <sivasubr@amazon.com> Date: Tue, 17 Jul 2018 22:01:42 +0000 Subject: [PATCH] Skip compression of pre-9.4 empty gin data leaf pages during replay --- src/backend/access/gin/ginxlog.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c index a40f168..0797b92 100644 --- a/src/backend/access/gin/ginxlog.c +++ b/src/backend/access/gin/ginxlog.c @@ -151,15 +151,28 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data) ItemPointer uncompressed = (ItemPointer) GinDataPageGetData(page); int nuncompressed = GinPageGetOpaque(page)->maxoff; int npacked; - GinPostingList *plist; - plist = ginCompressPostingList(uncompressed, nuncompressed, - BLCKSZ, &npacked); - Assert(npacked == nuncompressed); + totalsize = 0; - totalsize = SizeOfGinPostingList(plist); + /* + * In 9.3, when we delete data leaf pages as part of vacuum, we skip + * deletion of the leftmost and rightmost pages even when they are + * empty. So try to compress the posting list only if there are items + * in it. + */ + if (nuncompressed > 0) + { + GinPostingList *plist; + + plist = ginCompressPostingList(uncompressed, nuncompressed, + BLCKSZ, &npacked); + totalsize = SizeOfGinPostingList(plist); + + Assert(npacked == nuncompressed); + + memcpy(GinDataLeafPageGetPostingList(page), plist, totalsize); + } - memcpy(GinDataLeafPageGetPostingList(page), plist, totalsize); GinDataPageSetDataSize(page, totalsize); GinPageSetCompressed(page); GinPageGetOpaque(page)->maxoff = InvalidOffsetNumber; -- 2.7.3.AMZN

view thread (8+ messages)  latest in thread

Message-ID: <1531867212836.63354@amazon.com>
Permalink:  ../1531867212836.63354@amazon.com/
Also on:    postgresql.org/message-id/1531867212836.63354@amazon.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: sivasubr@amazon.com
  Subject: Re: Bug in gin insert redo code path during re-compression of empty gin data leaf pages
  In-Reply-To: <1531867212836.63354@amazon.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox