Обсуждение: Bug in gin insert redo code path during re-compression of empty gindata leaf pages

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:

On Wed, Jul 18, 2018 at 1:40 AM R, Siva <sivasubr@amazon.com> wrote:

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:

Thank you for reporting this bug.  At the first glance it seems that your proposed fix is correct.  I'll review it in more details and commit.

On Wed, Jul 18, 2018 at 11:59 AM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:

On Wed, Jul 18, 2018 at 1:40 AM R, Siva <sivasubr@amazon.com> wrote:

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:

Thank you for reporting this bug.  At the first glance it seems that your proposed fix is correct.  I'll review it in more details and commit.

I've managed to reproduce assertion failure while upgrading from 9.3 to master.  The steps to reproduction are following.

1. On 9.3 run following.

* create table t as (select array[1] as a from generate_series(1,2000));

* create index t_idx on t using gin(a) with (fastupdate= off);

* delete from t;

* vacuum t;

2. pg_upgrade from 9.3 to master

3. On upgraded master set full_page_writes = off and run it

4. On master run following

* insert into t (select array[1] as a from generate_series(1,2000));

5. kill postmaster with -9

2018-07-19 14:30:11.437 MSK [57946] CONTEXT:  WAL redo at 0/7000190 for Gin/INSERT: isdata: T isleaf: T 1 segments: 0 (replace)

TRAP: FailedAssertion("!(npacked == nuncompressed)", File: "ginxlog.c", Line: 161)

After applying your patch, I still have following assertion failure.

2018-07-19 14:40:34.449 MSK [60372] LOG:  redo starts at 0/7000140

TRAP: FailedAssertion("!(a_action == 2)", File: "ginxlog.c", Line: 262)

So, the issue is still persists.  I'm going to investigate this bug more and come up with updated patch.

> On Thu, Jul 19, 2018 at 2:43 PM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> It appears that we need to handle empty posting list pages also  in disassembleLeaf().  Updated patch is attached.  I'm
> going to commit  it.
Thank you so much for reviewing the patch! You are right, we need to handle the regular insert/vacuum path for the empty page as well, thanks for fixing that. The updated patch looks good to me.

>BTW, what is your full name for the commit message?
My full name is "Sivasubramanian Ramasubramanian".
Email : sivasubr@amazon.com

On 2018-Jul-19, Alexander Korotkov wrote:

> Pushed, thanks!

I think you missed branch REL_11_STABLE ...