Skip to content

Commit 853d1c3

Browse files
feodorfeodor
committed
Fix bundle bugs of GIN:
- Fix possible deadlock between UPDATE and VACUUM queries. Bug never was observed in 8.2, but it still exist there. HEAD is more sensitive to bug after recent "ring" of buffer improvements. - Fix WAL creation: if parent page is stored as is after split then incomplete split isn't removed during replay. This happens rather rare, only on large tables with a lot of updates/inserts. - Fix WAL replay: there was wrong test of XLR_BKP_BLOCK_* for left page after deletion of page. That causes wrong rightlink field: it pointed to deleted page. - add checking of match of clearing incomplete split - cleanup incomplete split list after proceeding All of this chages doesn't change on-disk storage, so backpatch... But second point may be an issue for replaying logs from previous version.
1 parent aae5403 commit 853d1c3

File tree

5 files changed

+146
-54
lines changed

5 files changed

+146
-54
lines changed

‎src/backend/access/gin/gindatapage.c

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gin/gindatapage.c,v 1.6 2007/01/05 22:19:21 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gin/gindatapage.c,v 1.7 2007/06/04 15:56:28 teodor Exp $
1212
*-------------------------------------------------------------------------
1313
*/
1414

@@ -358,6 +358,7 @@ dataPlaceToPage(GinBtree btree, Buffer buf, OffsetNumber off, XLogRecData **prda
358358
static XLogRecData rdata[3];
359359
int sizeofitem = GinSizeOfItem(page);
360360
static ginxlogInsert data;
361+
int cnt=0;
361362

362363
*prdata = rdata;
363364
Assert(GinPageIsData(page));
@@ -372,21 +373,33 @@ dataPlaceToPage(GinBtree btree, Buffer buf, OffsetNumber off, XLogRecData **prda
372373
data.isData = TRUE;
373374
data.isLeaf = GinPageIsLeaf(page) ? TRUE : FALSE;
374375

375-
rdata[0].buffer = buf;
376-
rdata[0].buffer_std = FALSE;
377-
rdata[0].data = NULL;
378-
rdata[0].len = 0;
379-
rdata[0].next = &rdata[1];
376+
/*
377+
* Prevent full page write if child's split occurs. That is needed
378+
* to remove incomplete splits while replaying WAL
379+
*
380+
* data.updateBlkno contains new block number (of newly created right page)
381+
* for recently splited page.
382+
*/
383+
if ( data.updateBlkno == InvalidBlockNumber )
384+
{
385+
rdata[0].buffer = buf;
386+
rdata[0].buffer_std = FALSE;
387+
rdata[0].data = NULL;
388+
rdata[0].len = 0;
389+
rdata[0].next = &rdata[1];
390+
cnt++;
391+
}
380392

381-
rdata[1].buffer = InvalidBuffer;
382-
rdata[1].data = (char *) &data;
383-
rdata[1].len = sizeof(ginxlogInsert);
384-
rdata[1].next = &rdata[2];
393+
rdata[cnt].buffer = InvalidBuffer;
394+
rdata[cnt].data = (char *) &data;
395+
rdata[cnt].len = sizeof(ginxlogInsert);
396+
rdata[cnt].next = &rdata[cnt+1];
397+
cnt++;
385398

386-
rdata[2].buffer = InvalidBuffer;
387-
rdata[2].data = (GinPageIsLeaf(page)) ? ((char *) (btree->items + btree->curitem)) : ((char *) &(btree->pitem));
388-
rdata[2].len = sizeofitem;
389-
rdata[2].next = NULL;
399+
rdata[cnt].buffer = InvalidBuffer;
400+
rdata[cnt].data = (GinPageIsLeaf(page)) ? ((char *) (btree->items + btree->curitem)) : ((char *) &(btree->pitem));
401+
rdata[cnt].len = sizeofitem;
402+
rdata[cnt].next = NULL;
390403

391404
if (GinPageIsLeaf(page))
392405
{
@@ -402,7 +415,7 @@ dataPlaceToPage(GinBtree btree, Buffer buf, OffsetNumber off, XLogRecData **prda
402415
btree->curitem++;
403416
}
404417
data.nitem = btree->curitem - savedPos;
405-
rdata[2].len = sizeofitem * data.nitem;
418+
rdata[cnt].len = sizeofitem * data.nitem;
406419
}
407420
else
408421
{

‎src/backend/access/gin/ginentrypage.c

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gin/ginentrypage.c,v 1.6 2007/01/05 22:19:21 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gin/ginentrypage.c,v 1.7 2007/06/04 15:56:28 teodor Exp $
1212
*-------------------------------------------------------------------------
1313
*/
1414

@@ -354,6 +354,7 @@ entryPlaceToPage(GinBtree btree, Buffer buf, OffsetNumber off, XLogRecData **prd
354354
static XLogRecData rdata[3];
355355
OffsetNumber placed;
356356
static ginxlogInsert data;
357+
int cnt=0;
357358

358359
*prdata = rdata;
359360
data.updateBlkno = entryPreparePage(btree, page, off);
@@ -371,21 +372,33 @@ entryPlaceToPage(GinBtree btree, Buffer buf, OffsetNumber off, XLogRecData **prd
371372
data.isData = false;
372373
data.isLeaf = GinPageIsLeaf(page) ? TRUE : FALSE;
373374

374-
rdata[0].buffer = buf;
375-
rdata[0].buffer_std = TRUE;
376-
rdata[0].data = NULL;
377-
rdata[0].len = 0;
378-
rdata[0].next = &rdata[1];
375+
/*
376+
* Prevent full page write if child's split occurs. That is needed
377+
* to remove incomplete splits while replaying WAL
378+
*
379+
* data.updateBlkno contains new block number (of newly created right page)
380+
* for recently splited page.
381+
*/
382+
if ( data.updateBlkno == InvalidBlockNumber )
383+
{
384+
rdata[0].buffer = buf;
385+
rdata[0].buffer_std = TRUE;
386+
rdata[0].data = NULL;
387+
rdata[0].len = 0;
388+
rdata[0].next = &rdata[1];
389+
cnt++;
390+
}
379391

380-
rdata[1].buffer = InvalidBuffer;
381-
rdata[1].data = (char *) &data;
382-
rdata[1].len = sizeof(ginxlogInsert);
383-
rdata[1].next = &rdata[2];
384-
385-
rdata[2].buffer = InvalidBuffer;
386-
rdata[2].data = (char *) btree->entry;
387-
rdata[2].len = IndexTupleSize(btree->entry);
388-
rdata[2].next = NULL;
392+
rdata[cnt].buffer = InvalidBuffer;
393+
rdata[cnt].data = (char *) &data;
394+
rdata[cnt].len = sizeof(ginxlogInsert);
395+
rdata[cnt].next = &rdata[cnt+1];
396+
cnt++;
397+
398+
rdata[cnt].buffer = InvalidBuffer;
399+
rdata[cnt].data = (char *) btree->entry;
400+
rdata[cnt].len = IndexTupleSize(btree->entry);
401+
rdata[cnt].next = NULL;
389402

390403
btree->entry = NULL;
391404
}

‎src/backend/access/gin/ginget.c

Lines changed: 66 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gin/ginget.c,v 1.7 2007/02/01 04:16:08 neilc Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gin/ginget.c,v 1.8 2007/06/04 15:56:28 teodor Exp $
1212
*-------------------------------------------------------------------------
1313
*/
1414

@@ -17,22 +17,71 @@
1717
#include "catalog/index.h"
1818
#include "utils/memutils.h"
1919

20-
static OffsetNumber
21-
findItemInPage(Page page, ItemPointer item, OffsetNumber off)
20+
static bool
21+
findItemInPage(Page page, ItemPointer item, OffsetNumber *off)
2222
{
2323
OffsetNumber maxoff = GinPageGetOpaque(page)->maxoff;
2424
int res;
2525

26-
for (; off <= maxoff; off++)
26+
if ( GinPageGetOpaque(page)->flags & GIN_DELETED )
27+
/* page was deleted by concurrent vacuum */
28+
return false;
29+
30+
if ( *off > maxoff || *off == InvalidOffsetNumber )
31+
res = -1;
32+
else
33+
res = compareItemPointers(item, (ItemPointer) GinDataPageGetItem(page, *off));
34+
35+
if ( res == 0 )
36+
{
37+
/* page isn't changed */
38+
return true;
39+
}
40+
else if ( res > 0 )
2741
{
28-
res = compareItemPointers(item, (ItemPointer) GinDataPageGetItem(page, off));
29-
Assert(res >= 0);
42+
/*
43+
* some items was added before our position, look further to find
44+
* it or first greater
45+
*/
46+
47+
(*off)++;
48+
for (; *off <= maxoff; (*off)++)
49+
{
50+
res = compareItemPointers(item, (ItemPointer) GinDataPageGetItem(page, *off));
51+
52+
if (res == 0)
53+
return true;
3054

31-
if (res == 0)
32-
return off;
55+
if (res < 0)
56+
{
57+
(*off)--;
58+
return true;
59+
}
60+
}
3361
}
62+
else
63+
{
64+
/*
65+
* some items was deleted before our position, look from begining
66+
* to find it or first greater
67+
*/
68+
69+
for(*off = FirstOffsetNumber; *off<= maxoff; (*off)++)
70+
{
71+
res = compareItemPointers(item, (ItemPointer) GinDataPageGetItem(page, *off));
3472

35-
return InvalidOffsetNumber;
73+
if ( res == 0 )
74+
return true;
75+
76+
if (res < 0)
77+
{
78+
(*off)--;
79+
return true;
80+
}
81+
}
82+
}
83+
84+
return false;
3685
}
3786

3887
/*
@@ -111,7 +160,7 @@ startScanEntry(Relation index, GinState *ginstate, GinScanEntry entry, bool firs
111160
}
112161
else if (entry->buffer != InvalidBuffer)
113162
{
114-
/* we should find place were we was stopped */
163+
/* we should find place where we was stopped */
115164
BlockNumber blkno;
116165
Page page;
117166

@@ -125,7 +174,7 @@ startScanEntry(Relation index, GinState *ginstate, GinScanEntry entry, bool firs
125174
page = BufferGetPage(entry->buffer);
126175

127176
/* try to find curItem in current buffer */
128-
if ((entry->offset = findItemInPage(page, &entry->curItem, entry->offset)) != InvalidOffsetNumber)
177+
if ( findItemInPage(page, &entry->curItem, &entry->offset) )
129178
return;
130179

131180
/* walk to right */
@@ -136,11 +185,15 @@ startScanEntry(Relation index, GinState *ginstate, GinScanEntry entry, bool firs
136185
LockBuffer(entry->buffer, GIN_SHARE);
137186
page = BufferGetPage(entry->buffer);
138187

139-
if ((entry->offset = findItemInPage(page, &entry->curItem, FirstOffsetNumber)) != InvalidOffsetNumber)
188+
entry->offset = InvalidOffsetNumber;
189+
if ( findItemInPage(page, &entry->curItem, &entry->offset) )
140190
return;
141191
}
142192

143-
elog(ERROR, "Logic error: lost previously founded ItemId");
193+
/*
194+
* curItem and any greated items was deleted by concurrent vacuum,
195+
* so we finished scan with currrent entry
196+
*/
144197
}
145198
}
146199

‎src/backend/access/gin/ginvacuum.c

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gin/ginvacuum.c,v 1.13 2007/05/31 14:03:09 teodor Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gin/ginvacuum.c,v 1.14 2007/06/04 15:56:28 teodor Exp $
1212
*-------------------------------------------------------------------------
1313
*/
1414

@@ -160,14 +160,14 @@ ginVacuumPostingTreeLeaves(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
160160
/*
161161
* We should be sure that we don't concurrent with inserts, insert process
162162
* never release root page until end (but it can unlock it and lock
163-
* again). If we lock root with with LockBufferForCleanup, new scan
164-
* process can't begin, but previous may run. ginmarkpos/start* keeps
165-
* buffer pinned, so we will wait for it. We lock only one posting tree in
166-
* whole index, so, it's concurrent enough.. Side effect: after this is
167-
* full complete, tree is unused by any other process
163+
* again). New scan can't start but previously started
164+
* ones work concurrently.
168165
*/
169166

170-
LockBufferForCleanup(buffer);
167+
if ( isRoot )
168+
LockBufferForCleanup(buffer);
169+
else
170+
LockBuffer(buffer, GIN_EXCLUSIVE);
171171

172172
Assert(GinPageIsData(page));
173173

@@ -250,15 +250,15 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
250250
if (!isParentRoot) /* parent is already locked by
251251
* LockBufferForCleanup() */
252252
LockBuffer(pBuffer, GIN_EXCLUSIVE);
253+
if (leftBlkno != InvalidBlockNumber)
254+
LockBuffer(lBuffer, GIN_EXCLUSIVE);
253255

254256
START_CRIT_SECTION();
255257

256258
if (leftBlkno != InvalidBlockNumber)
257259
{
258260
BlockNumber rightlink;
259261

260-
LockBuffer(lBuffer, GIN_EXCLUSIVE);
261-
262262
page = BufferGetPage(dBuffer);
263263
rightlink = GinPageGetOpaque(page)->rightlink;
264264

@@ -276,6 +276,10 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
276276
PageDeletePostingItem(parentPage, myoff);
277277

278278
page = BufferGetPage(dBuffer);
279+
/*
280+
* we shouldn't change rightlink field to save
281+
* workability of running search scan
282+
*/
279283
GinPageGetOpaque(page)->flags = GIN_DELETED;
280284

281285
if (!gvs->index->rd_istemp)

‎src/backend/access/gin/ginxlog.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gin/ginxlog.c,v 1.6 2007/01/05 22:19:21 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gin/ginxlog.c,v 1.7 2007/06/04 15:56:28 teodor Exp $
1212
*-------------------------------------------------------------------------
1313
*/
1414
#include "postgres.h"
@@ -53,6 +53,7 @@ static void
5353
forgetIncompleteSplit(RelFileNode node, BlockNumber leftBlkno, BlockNumber updateBlkno)
5454
{
5555
ListCell *l;
56+
bool found = false;
5657

5758
foreach(l, incomplete_splits)
5859
{
@@ -61,9 +62,16 @@ forgetIncompleteSplit(RelFileNode node, BlockNumber leftBlkno, BlockNumber updat
6162
if (RelFileNodeEquals(node, split->node) && leftBlkno == split->leftBlkno && updateBlkno == split->rightBlkno)
6263
{
6364
incomplete_splits = list_delete_ptr(incomplete_splits, split);
65+
found = true;
6466
break;
6567
}
6668
}
69+
70+
if (!found)
71+
{
72+
elog(ERROR, "failed to identify corresponding split record for %u/%u/%u",
73+
node.relNode, leftBlkno, updateBlkno);
74+
}
6775
}
6876

6977
static void
@@ -416,7 +424,7 @@ ginRedoDeletePage(XLogRecPtr lsn, XLogRecord *record)
416424
UnlockReleaseBuffer(buffer);
417425
}
418426

419-
if (!(record->xl_info & XLR_BKP_BLOCK_2) && data->leftBlkno != InvalidBlockNumber)
427+
if (!(record->xl_info & XLR_BKP_BLOCK_3) && data->leftBlkno != InvalidBlockNumber)
420428
{
421429
buffer = XLogReadBuffer(reln, data->leftBlkno, false);
422430
page = BufferGetPage(buffer);
@@ -594,6 +602,7 @@ gin_xlog_cleanup(void)
594602

595603
MemoryContextSwitchTo(topCtx);
596604
MemoryContextDelete(opCtx);
605+
incomplete_splits = NIL;
597606
}
598607

599608
bool

0 commit comments

Comments
 (0)