Re: Index corruption with CREATE INDEX CONCURRENTLY

Pavan Deolasee wrote:

> Looking at the history and some past discussions, it seems Tomas reported
> somewhat similar problem and Andres proposed a patch here
> https://www.postgresql.org/message-id/20140514155204.GE23943@awork2.anarazel.de
> which got committed via b23b0f5588d2e2. Not exactly the same issue, but
> related to relcache flush happening in index_open().
> 
> I wonder if we should just add a recheck logic
> in RelationGetIndexAttrBitmap() such that if rd_indexvalid is seen as 0 at
> the end, we go back and start again from RelationGetIndexList(). Or is
> there any code path that relies on finding the old information?

Good find on that old report.  It occured to me to re-run Tomas' test
case with this second patch you proposed (the "ddl" test takes 11
minutes in my laptop), and lo and behold -- your "recheck" code enters
an infinite loop.  Not good.  I tried some more tricks that came to
mind, but I didn't get any good results.  Pavan and I discussed it
further and he came up with the idea of returning the bitmapset we
compute, but if an invalidation occurs in index_open, simply do not
cache the bitmapsets so that next time this is called they are all
computed again.  Patch attached.

This appears to have appeased both test_decoding's ddl test under
CACHE_CLOBBER_ALWAYS, as well as the CREATE INDEX CONCURRENTLY bug.

I intend to commit this soon to all branches, to ensure it gets into the
next set of minors.

Here is a detailed reproducer for the CREATE INDEX CONCURRENTLY bug.
Line numbers are as of today's master; comments are supposed to help
locating good spots in other branches.  Given the use of gdb
breakpoints, there's no good way to integrate this with regression
tests, which is a pity because this stuff looks a little brittle to me,
and if it breaks it is hard to detect.

Prep:
DROP TABLE IF EXISTS testtab;
CREATE TABLE testtab (a int unique, b int, c int);
INSERT INTO testtab VALUES (1, 100, 10);
CREATE INDEX testindx ON testtab (b);

S1:
SELECT * FROM testtab;
UPDATE testtab SET b = b + 1 WHERE a = 1;
SELECT pg_backend_pid();
  attach GDB to this session.
  break relcache.c:4800
      # right before entering the per-index loop
  cont

S2:
SELECT pg_backend_pid();
DROP INDEX testindx;
  attach GDB to this session
  handle SIGUSR1 nostop
  break indexcmds.c:666
     # right after index_create
  break indexcmds.c:790
     # right before the second CommitTransactionCommand
  cont
CREATE INDEX CONCURRENTLY testindx ON testtab (b);
  -- this blocks in breakpoint 1.  Leave it there.

S1:
UPDATE testtab SET b = b + 1 WHERE a = 1;
  -- this blocks in breakpoint 1. Leave it there.

S2:
  gdb -> cont
  -- This blocks waiting for S1

S1:
  gdb -> cont
  -- S1 finishes the update.  S2 awakens and blocks again in breakpoint 2.

S1:
  -- Now run more UPDATEs:
UPDATE testtab SET b = b + 1 WHERE a = 1;
  This produces a broken HOT chain.
  This can be done several times; all these updates should be non-HOT
  because of the index created by CIC, but they are marked HOT.

S2: "cont"
  From this point onwards, update are correct.

SELECT * FROM testtab;
SET enable_seqscan TO 0;
SET enable_bitmapscan TO 0;
SELECT * FROM testtab WHERE b between 100 and 110;

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services