Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9ffc8ed

Browse files
tglsfdctglsfdc
committed
Repair possible failure to update hint bits back to disk, per
http://archives.postgresql.org/pgsql-hackers/2004-10/msg00464.php. This fix is intended to be permanent: it moves the responsibility for calling SetBufferCommitInfoNeedsSave() into the tqual.c routines, eliminating the requirement for callers to test whether t_infomask changed. Also, tighten validity checking on buffer IDs in bufmgr.c --- several routines were paranoid about out-of-range shared buffer numbers but not about out-of-range local ones, which seems a tad pointless.
1 parent db9e2fd commit 9ffc8ed

File tree

13 files changed

+241
-176
lines changed

13 files changed

+241
-176
lines changed

‎contrib/pgstattuple/pgstattuple.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* $PostgreSQL: pgsql/contrib/pgstattuple/pgstattuple.c,v 1.16 2004/08/29 05:06:37 momjian Exp $
2+
* $PostgreSQL: pgsql/contrib/pgstattuple/pgstattuple.c,v 1.17 2004/10/15 22:39:38 tgl Exp $
33
*
44
* Copyright (c) 2001,2002 Tatsuo Ishii
55
*
@@ -134,7 +134,10 @@ pgstattuple_real(Relation rel)
134134
/* scan the relation */
135135
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
136136
{
137-
if (HeapTupleSatisfiesNow(tuple->t_data))
137+
/* must hold a buffer lock to call HeapTupleSatisfiesNow */
138+
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
139+
140+
if (HeapTupleSatisfiesNow(tuple->t_data, scan->rs_cbuf))
138141
{
139142
tuple_len += tuple->t_len;
140143
tuple_count++;
@@ -145,6 +148,8 @@ pgstattuple_real(Relation rel)
145148
dead_tuple_count++;
146149
}
147150

151+
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
152+
148153
/*
149154
* To avoid physically reading the table twice, try to do the
150155
* free-space scan in parallel with the heap scan. However,
@@ -156,7 +161,9 @@ pgstattuple_real(Relation rel)
156161
while (block <= tupblock)
157162
{
158163
buffer = ReadBuffer(rel, block);
164+
LockBuffer(buffer, BUFFER_LOCK_SHARE);
159165
free_space += PageGetFreeSpace((Page) BufferGetPage(buffer));
166+
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
160167
ReleaseBuffer(buffer);
161168
block++;
162169
}

‎src/backend/access/heap/heapam.c

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.178 2004/10/12 21:54:34 petere Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.179 2004/10/15 22:39:42 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1314,7 +1314,7 @@ heap_delete(Relation relation, ItemPointer tid,
13141314
tp.t_tableOid = relation->rd_id;
13151315

13161316
l1:
1317-
result = HeapTupleSatisfiesUpdate(tp.t_data, cid);
1317+
result = HeapTupleSatisfiesUpdate(tp.t_data, cid, buffer);
13181318

13191319
if (result == HeapTupleInvisible)
13201320
{
@@ -1331,7 +1331,7 @@ heap_delete(Relation relation, ItemPointer tid,
13311331
XactLockTableWait(xwait);
13321332

13331333
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
1334-
if (TransactionIdDidAbort(xwait))
1334+
if (!TransactionIdDidCommit(xwait))
13351335
goto l1;
13361336

13371337
/*
@@ -1356,7 +1356,7 @@ heap_delete(Relation relation, ItemPointer tid,
13561356
if (crosscheck != InvalidSnapshot && result == HeapTupleMayBeUpdated)
13571357
{
13581358
/* Perform additional check for serializable RI updates */
1359-
if (!HeapTupleSatisfiesSnapshot(tp.t_data, crosscheck))
1359+
if (!HeapTupleSatisfiesSnapshot(tp.t_data, crosscheck, buffer))
13601360
result = HeapTupleUpdated;
13611361
}
13621362

@@ -1543,7 +1543,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
15431543
*/
15441544

15451545
l2:
1546-
result = HeapTupleSatisfiesUpdate(oldtup.t_data, cid);
1546+
result = HeapTupleSatisfiesUpdate(oldtup.t_data, cid, buffer);
15471547

15481548
if (result == HeapTupleInvisible)
15491549
{
@@ -1560,7 +1560,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
15601560
XactLockTableWait(xwait);
15611561

15621562
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
1563-
if (TransactionIdDidAbort(xwait))
1563+
if (!TransactionIdDidCommit(xwait))
15641564
goto l2;
15651565

15661566
/*
@@ -1585,7 +1585,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
15851585
if (crosscheck != InvalidSnapshot && result == HeapTupleMayBeUpdated)
15861586
{
15871587
/* Perform additional check for serializable RI updates */
1588-
if (!HeapTupleSatisfiesSnapshot(oldtup.t_data, crosscheck))
1588+
if (!HeapTupleSatisfiesSnapshot(oldtup.t_data, crosscheck, buffer))
15891589
result = HeapTupleUpdated;
15901590
}
15911591

@@ -1871,7 +1871,7 @@ heap_mark4update(Relation relation, HeapTuple tuple, Buffer *buffer,
18711871
tuple->t_len = ItemIdGetLength(lp);
18721872

18731873
l3:
1874-
result = HeapTupleSatisfiesUpdate(tuple->t_data, cid);
1874+
result = HeapTupleSatisfiesUpdate(tuple->t_data, cid, *buffer);
18751875

18761876
if (result == HeapTupleInvisible)
18771877
{
@@ -1888,7 +1888,7 @@ heap_mark4update(Relation relation, HeapTuple tuple, Buffer *buffer,
18881888
XactLockTableWait(xwait);
18891889

18901890
LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
1891-
if (TransactionIdDidAbort(xwait))
1891+
if (!TransactionIdDidCommit(xwait))
18921892
goto l3;
18931893

18941894
/*

‎src/backend/access/index/indexam.c

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.75 2004/09/30 23:21:14 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.76 2004/10/15 22:39:46 tgl Exp $
1212
*
1313
* INTERFACE ROUTINES
1414
* index_open - open an index relation by relation OID
@@ -497,7 +497,6 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
497497
for (;;)
498498
{
499499
bool found;
500-
uint16 sv_infomask;
501500

502501
pgstat_count_index_scan(&scan->xs_pgstat_info);
503502

@@ -541,19 +540,14 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
541540
* index AM to not return it on future indexscans.
542541
*
543542
* We told heap_release_fetch to keep a pin on the buffer, so we can
544-
* re-access the tuple here. But we must re-lock the buffer
545-
* first. Also, it's just barely possible for an update of hint
546-
* bits to occur here.
543+
* re-access the tuple here. But we must re-lock the buffer first.
547544
*/
548545
LockBuffer(scan->xs_cbuf, BUFFER_LOCK_SHARE);
549-
sv_infomask = heapTuple->t_data->t_infomask;
550546

551-
if (HeapTupleSatisfiesVacuum(heapTuple->t_data, RecentGlobalXmin) ==
552-
HEAPTUPLE_DEAD)
547+
if (HeapTupleSatisfiesVacuum(heapTuple->t_data, RecentGlobalXmin,
548+
scan->xs_cbuf) == HEAPTUPLE_DEAD)
553549
scan->kill_prior_tuple = true;
554550

555-
if (sv_infomask != heapTuple->t_data->t_infomask)
556-
SetBufferCommitInfoNeedsSave(scan->xs_cbuf);
557551
LockBuffer(scan->xs_cbuf, BUFFER_LOCK_UNLOCK);
558552
}
559553

‎src/backend/access/nbtree/nbtinsert.c

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.116 2004/08/29 05:06:40 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.117 2004/10/15 22:39:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -261,19 +261,13 @@ _bt_check_unique(Relation rel, BTItem btitem, Relation heapRel,
261261
* marked killed. This logic should match
262262
* index_getnext and btgettuple.
263263
*/
264-
uint16 sv_infomask;
265-
266264
LockBuffer(hbuffer, BUFFER_LOCK_SHARE);
267-
sv_infomask = htup.t_data->t_infomask;
268-
if (HeapTupleSatisfiesVacuum(htup.t_data,
269-
RecentGlobalXmin) ==
270-
HEAPTUPLE_DEAD)
265+
if (HeapTupleSatisfiesVacuum(htup.t_data, RecentGlobalXmin,
266+
hbuffer) == HEAPTUPLE_DEAD)
271267
{
272268
curitemid->lp_flags |= LP_DELETE;
273269
SetBufferCommitInfoNeedsSave(buf);
274270
}
275-
if (sv_infomask != htup.t_data->t_infomask)
276-
SetBufferCommitInfoNeedsSave(hbuffer);
277271
LockBuffer(hbuffer, BUFFER_LOCK_UNLOCK);
278272
ReleaseBuffer(hbuffer);
279273
}

‎src/backend/catalog/index.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.240 2004/10/01 17:11:49 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.241 2004/10/15 22:39:53 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1472,18 +1472,16 @@ IndexBuildHeapScan(Relation heapRelation,
14721472
{
14731473
/* do our own time qual check */
14741474
bool indexIt;
1475-
uint16 sv_infomask;
14761475

14771476
/*
1478-
* HeapTupleSatisfiesVacuum may update tuple's hint status
1479-
* bits. We could possibly get away with not locking the
1480-
* buffer here, since caller should hold ShareLock on the
1481-
* relation, but let's be conservative about it.
1477+
* We could possibly get away with not locking the buffer here,
1478+
* since caller should hold ShareLock on the relation, but let's
1479+
* be conservative about it.
14821480
*/
14831481
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
1484-
sv_infomask = heapTuple->t_data->t_infomask;
14851482

1486-
switch (HeapTupleSatisfiesVacuum(heapTuple->t_data, OldestXmin))
1483+
switch (HeapTupleSatisfiesVacuum(heapTuple->t_data, OldestXmin,
1484+
scan->rs_cbuf))
14871485
{
14881486
case HEAPTUPLE_DEAD:
14891487
indexIt = false;
@@ -1544,10 +1542,6 @@ IndexBuildHeapScan(Relation heapRelation,
15441542
break;
15451543
}
15461544

1547-
/* check for hint-bit update by HeapTupleSatisfiesVacuum */
1548-
if (sv_infomask != heapTuple->t_data->t_infomask)
1549-
SetBufferCommitInfoNeedsSave(scan->rs_cbuf);
1550-
15511545
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
15521546

15531547
if (!indexIt)

‎src/backend/commands/vacuum.c

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.294 2004/10/07 14:19:58 momjian Exp $
16+
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.295 2004/10/15 22:39:56 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -1190,6 +1190,12 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
11901190
buf = ReadBuffer(onerel, blkno);
11911191
page = BufferGetPage(buf);
11921192

1193+
/*
1194+
* We don't bother to do LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE)
1195+
* because we assume that holding exclusive lock on the relation
1196+
* will keep other backends from looking at the page.
1197+
*/
1198+
11931199
vacpage->blkno = blkno;
11941200
vacpage->offsets_used = 0;
11951201
vacpage->offsets_free = 0;
@@ -1235,7 +1241,6 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
12351241
offnum <= maxoff;
12361242
offnum = OffsetNumberNext(offnum))
12371243
{
1238-
uint16 sv_infomask;
12391244
ItemId itemid = PageGetItemId(page, offnum);
12401245
bool tupgone = false;
12411246

@@ -1255,9 +1260,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
12551260
tuple.t_len = ItemIdGetLength(itemid);
12561261
ItemPointerSet(&(tuple.t_self), blkno, offnum);
12571262

1258-
sv_infomask = tuple.t_data->t_infomask;
1259-
1260-
switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin))
1263+
switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin, buf))
12611264
{
12621265
case HEAPTUPLE_DEAD:
12631266
tupgone = true; /* we can delete the tuple */
@@ -1348,10 +1351,6 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
13481351
break;
13491352
}
13501353

1351-
/* check for hint-bit update by HeapTupleSatisfiesVacuum */
1352-
if (sv_infomask != tuple.t_data->t_infomask)
1353-
pgchanged = true;
1354-
13551354
if (tupgone)
13561355
{
13571356
ItemId lpp;

‎src/backend/commands/vacuumlazy.c

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
*
3232
*
3333
* IDENTIFICATION
34-
* $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.46 2004/09/30 23:21:19 tgl Exp $
34+
* $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.47 2004/10/15 22:39:56 tgl Exp $
3535
*
3636
*-------------------------------------------------------------------------
3737
*/
@@ -291,7 +291,6 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
291291
offnum = OffsetNumberNext(offnum))
292292
{
293293
ItemId itemid;
294-
uint16 sv_infomask;
295294

296295
itemid = PageGetItemId(page, offnum);
297296

@@ -307,9 +306,8 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
307306
ItemPointerSet(&(tuple.t_self), blkno, offnum);
308307

309308
tupgone = false;
310-
sv_infomask = tuple.t_data->t_infomask;
311309

312-
switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin))
310+
switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin, buf))
313311
{
314312
case HEAPTUPLE_DEAD:
315313
tupgone = true; /* we can delete the tuple */
@@ -364,10 +362,6 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
364362
break;
365363
}
366364

367-
/* check for hint-bit update by HeapTupleSatisfiesVacuum */
368-
if (sv_infomask != tuple.t_data->t_infomask)
369-
pgchanged = true;
370-
371365
if (tupgone)
372366
{
373367
lazy_record_dead_tuple(vacrelstats, &(tuple.t_self));
@@ -399,9 +393,9 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
399393
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
400394

401395
if (pgchanged)
402-
SetBufferCommitInfoNeedsSave(buf);
403-
404-
ReleaseBuffer(buf);
396+
WriteBuffer(buf);
397+
else
398+
ReleaseBuffer(buf);
405399
}
406400

407401
/* save stats for use later */
@@ -790,8 +784,7 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
790784
Page page;
791785
OffsetNumber offnum,
792786
maxoff;
793-
bool pgchanged,
794-
tupgone,
787+
bool tupgone,
795788
hastup;
796789

797790
vacuum_delay_point();
@@ -813,15 +806,13 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
813806
continue;
814807
}
815808

816-
pgchanged = false;
817809
hastup = false;
818810
maxoff = PageGetMaxOffsetNumber(page);
819811
for (offnum = FirstOffsetNumber;
820812
offnum <= maxoff;
821813
offnum = OffsetNumberNext(offnum))
822814
{
823815
ItemId itemid;
824-
uint16 sv_infomask;
825816

826817
itemid = PageGetItemId(page, offnum);
827818

@@ -834,9 +825,8 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
834825
ItemPointerSet(&(tuple.t_self), blkno, offnum);
835826

836827
tupgone = false;
837-
sv_infomask = tuple.t_data->t_infomask;
838828

839-
switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin))
829+
switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin, buf))
840830
{
841831
case HEAPTUPLE_DEAD:
842832
tupgone = true; /* we can delete the tuple */
@@ -862,10 +852,6 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
862852
break;
863853
}
864854

865-
/* check for hint-bit update by HeapTupleSatisfiesVacuum */
866-
if (sv_infomask != tuple.t_data->t_infomask)
867-
pgchanged = true;
868-
869855
if (!tupgone)
870856
{
871857
hastup = true;
@@ -875,10 +861,7 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
875861

876862
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
877863

878-
if (pgchanged)
879-
WriteBuffer(buf);
880-
else
881-
ReleaseBuffer(buf);
864+
ReleaseBuffer(buf);
882865

883866
/* Done scanning if we found a tuple here */
884867
if (hastup)

0 commit comments

Comments
 (0)