Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd6c9e05

Browse files
committed
Fix assorted bugs in contrib/bloom.
In blinsert(), cope with the possibility that a page we pull from thenotFullPage list is marked BLOOM_DELETED. This could happen if VACUUMrecently marked it deleted but hasn't (yet) updated the metapage.We can re-use such a page safely, but we *must* reinitialize it so thatit's no longer marked deleted.Fix blvacuum() so that it updates the notFullPage list even if it'sgoing to update it to empty. The previous "optimization" of skippingthe update seems pretty dubious, since it means that the next blinsert()will uselessly visit whatever pages we left in the list.Uniformly treat PageIsNew pages the same as deleted pages. This shouldallow proper recovery if a crash occurs just after relation extension.Properly use vacuum_delay_point, not assorted ad-hoc CHECK_FOR_INTERRUPTScalls, in the blvacuum() main loop.Fix broken tuple-counting logic: blvacuum.c counted the number of liveindex tuples over again in each scan, leading to VACUUM VERBOSE reportingsome multiple of the actual number of surviving index tuples after anyvacuum that removed any tuples (since they'd be counted in blvacuum, maybemore than once, and then again in blvacuumcleanup, without ever zeroing thecounter). It's sufficient to count them in blvacuumcleanup.stats->estimated_count is a boolean, not a counter, and we don't wantto set it true, so don't add tuple counts to it.Add a couple of Asserts that we don't overrun available space on a bloompage. I don't think there's any bug there today, but the way theFreeBlockNumberArray size calculation is set up is scarily fragile, andBloomPageGetFreeSpace isn't much better. The Asserts should help catchany future mistakes.Per investigation of a report from Jeff Janes. I think the first itemabove may explain his report; the other changes were things I noticedwhile casting about for an explanation.Report: <CAMkU=1xEUuBphDwDmB1WjN4+td4kpnEniFaTBxnk1xzHCw8_OQ@mail.gmail.com>
1 parented0097e commitd6c9e05

File tree

4 files changed

+60
-41
lines changed

4 files changed

+60
-41
lines changed

‎contrib/bloom/blinsert.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,13 @@ blinsert(Relation index, Datum *values, bool *isnull,
237237
state=GenericXLogStart(index);
238238
page=GenericXLogRegisterBuffer(state,buffer,0);
239239

240+
/*
241+
* We might have found a page that was recently deleted by VACUUM. If
242+
* so, we can reuse it, but we must reinitialize it.
243+
*/
244+
if (PageIsNew(page)||BloomPageIsDeleted(page))
245+
BloomInitPage(page,0);
246+
240247
if (BloomPageAddItem(&blstate,page,itup))
241248
{
242249
/* Success! Apply the change, clean up, and exit */
@@ -295,6 +302,10 @@ blinsert(Relation index, Datum *values, bool *isnull,
295302
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
296303
page=GenericXLogRegisterBuffer(state,buffer,0);
297304

305+
/* Basically same logic as above */
306+
if (PageIsNew(page)||BloomPageIsDeleted(page))
307+
BloomInitPage(page,0);
308+
298309
if (BloomPageAddItem(&blstate,page,itup))
299310
{
300311
/* Success! Apply the changes, clean up, and exit */

‎contrib/bloom/blscan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
135135
page=BufferGetPage(buffer);
136136
TestForOldSnapshot(scan->xs_snapshot,scan->indexRelation,page);
137137

138-
if (!BloomPageIsDeleted(page))
138+
if (!PageIsNew(page)&& !BloomPageIsDeleted(page))
139139
{
140140
OffsetNumberoffset,
141141
maxOffset=BloomPageGetMaxOffset(page);

‎contrib/bloom/blutils.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ BloomFormTuple(BloomState *state, ItemPointer iptr, Datum *values, bool *isnull)
299299

300300
/*
301301
* Add new bloom tuple to the page. Returns true if new tuple was successfully
302-
* added to the page. Returns false if it doesn't fit the page.
302+
* added to the page. Returns false if it doesn't fitonthe page.
303303
*/
304304
bool
305305
BloomPageAddItem(BloomState*state,Pagepage,BloomTuple*tuple)
@@ -308,7 +308,10 @@ BloomPageAddItem(BloomState *state, Page page, BloomTuple *tuple)
308308
BloomPageOpaqueopaque;
309309
Pointerptr;
310310

311-
/* Does new tuple fit the page */
311+
/* We shouldn't be pointed to an invalid page */
312+
Assert(!PageIsNew(page)&& !BloomPageIsDeleted(page));
313+
314+
/* Does new tuple fit on the page? */
312315
if (BloomPageGetFreeSpace(state,page)<state->sizeOfBloomTuple)
313316
return false;
314317

@@ -322,6 +325,9 @@ BloomPageAddItem(BloomState *state, Page page, BloomTuple *tuple)
322325
ptr= (Pointer)BloomPageGetTuple(state,page,opaque->maxoff+1);
323326
((PageHeader)page)->pd_lower=ptr-page;
324327

328+
/* Assert we didn't overrun available space */
329+
Assert(((PageHeader)page)->pd_lower <= ((PageHeader)page)->pd_upper);
330+
325331
return true;
326332
}
327333

@@ -424,6 +430,9 @@ BloomFillMetapage(Relation index, Page metaPage)
424430
metadata->magickNumber=BLOOM_MAGICK_NUMBER;
425431
metadata->opts=*opts;
426432
((PageHeader)metaPage)->pd_lower+=sizeof(BloomMetaPageData);
433+
434+
/* If this fails, probably FreeBlockNumberArray size calc is wrong: */
435+
Assert(((PageHeader)metaPage)->pd_lower <= ((PageHeader)metaPage)->pd_upper);
427436
}
428437

429438
/*

‎contrib/bloom/blvacuum.c

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include"postgres.h"
1414

1515
#include"access/genam.h"
16+
#include"bloom.h"
1617
#include"catalog/storage.h"
1718
#include"commands/vacuum.h"
1819
#include"miscadmin.h"
@@ -21,7 +22,6 @@
2122
#include"storage/indexfsm.h"
2223
#include"storage/lmgr.h"
2324

24-
#include"bloom.h"
2525

2626
/*
2727
* Bulk deletion of all index entries pointing to a set of heap tuples.
@@ -42,6 +42,7 @@ blbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
4242
BloomStatestate;
4343
Bufferbuffer;
4444
Pagepage;
45+
BloomMetaPageData*metaData;
4546
GenericXLogState*gxlogState;
4647

4748
if (stats==NULL)
@@ -60,22 +61,27 @@ blbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
6061
*itupPtr,
6162
*itupEnd;
6263

64+
vacuum_delay_point();
65+
6366
buffer=ReadBufferExtended(index,MAIN_FORKNUM,blkno,
6467
RBM_NORMAL,info->strategy);
6568

6669
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
6770
gxlogState=GenericXLogStart(index);
6871
page=GenericXLogRegisterBuffer(gxlogState,buffer,0);
6972

70-
if (BloomPageIsDeleted(page))
73+
/* Ignore empty/deleted pages until blvacuumcleanup() */
74+
if (PageIsNew(page)||BloomPageIsDeleted(page))
7175
{
7276
UnlockReleaseBuffer(buffer);
7377
GenericXLogAbort(gxlogState);
74-
CHECK_FOR_INTERRUPTS();
7578
continue;
7679
}
7780

78-
/* Iterate over the tuples */
81+
/*
82+
* Iterate over the tuples. itup points to current tuple being
83+
* scanned, itupPtr points to where to save next non-deleted tuple.
84+
*/
7985
itup=itupPtr=BloomPageGetTuple(&state,page,FirstOffsetNumber);
8086
itupEnd=BloomPageGetTuple(&state,page,
8187
OffsetNumberNext(BloomPageGetMaxOffset(page)));
@@ -84,36 +90,32 @@ blbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
8490
/* Do we have to delete this tuple? */
8591
if (callback(&itup->heapPtr,callback_state))
8692
{
87-
stats->tuples_removed+=1;
93+
/* Yes; adjust count of tuples that will be left on page */
8894
BloomPageGetOpaque(page)->maxoff--;
95+
stats->tuples_removed+=1;
8996
}
9097
else
9198
{
99+
/* No; copy it to itupPtr++, but skip copy if not needed */
92100
if (itupPtr!=itup)
93-
{
94-
/*
95-
* If we already delete something before, we have to move
96-
* this tuple backward.
97-
*/
98101
memmove((Pointer)itupPtr, (Pointer)itup,
99102
state.sizeOfBloomTuple);
100-
}
101-
stats->num_index_tuples++;
102103
itupPtr=BloomPageGetNextTuple(&state,itupPtr);
103104
}
104105

105106
itup=BloomPageGetNextTuple(&state,itup);
106107
}
107108

109+
/* Assert that we counted correctly */
108110
Assert(itupPtr==BloomPageGetTuple(&state,page,
109111
OffsetNumberNext(BloomPageGetMaxOffset(page))));
110112

111113
/*
112-
* Add page to notFullPage list if we will not mark page as deleted
113-
* and there is a free space on it
114+
* Add page tonewnotFullPage list if we will not mark page as
115+
*deletedand there is free space on it
114116
*/
115117
if (BloomPageGetMaxOffset(page)!=0&&
116-
BloomPageGetFreeSpace(&state,page)>state.sizeOfBloomTuple&&
118+
BloomPageGetFreeSpace(&state,page) >=state.sizeOfBloomTuple&&
117119
countPage<BloomMetaBlockN)
118120
notFullPage[countPage++]=blkno;
119121

@@ -134,27 +136,26 @@ blbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
134136
GenericXLogAbort(gxlogState);
135137
}
136138
UnlockReleaseBuffer(buffer);
137-
CHECK_FOR_INTERRUPTS();
138139
}
139140

140-
if (countPage>0)
141-
{
142-
BloomMetaPageData*metaData;
143-
144-
buffer=ReadBuffer(index,BLOOM_METAPAGE_BLKNO);
145-
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
141+
/*
142+
* Update the metapage's notFullPage list with whatever we found. Our
143+
* info could already be out of date at this point, but blinsert() will
144+
* cope if so.
145+
*/
146+
buffer=ReadBuffer(index,BLOOM_METAPAGE_BLKNO);
147+
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
146148

147-
gxlogState=GenericXLogStart(index);
148-
page=GenericXLogRegisterBuffer(gxlogState,buffer,0);
149+
gxlogState=GenericXLogStart(index);
150+
page=GenericXLogRegisterBuffer(gxlogState,buffer,0);
149151

150-
metaData=BloomPageGetMeta(page);
151-
memcpy(metaData->notFullPage,notFullPage,sizeof(BlockNumber)*countPage);
152-
metaData->nStart=0;
153-
metaData->nEnd=countPage;
152+
metaData=BloomPageGetMeta(page);
153+
memcpy(metaData->notFullPage,notFullPage,sizeof(BlockNumber)*countPage);
154+
metaData->nStart=0;
155+
metaData->nEnd=countPage;
154156

155-
GenericXLogFinish(gxlogState);
156-
UnlockReleaseBuffer(buffer);
157-
}
157+
GenericXLogFinish(gxlogState);
158+
UnlockReleaseBuffer(buffer);
158159

159160
returnstats;
160161
}
@@ -170,7 +171,6 @@ blvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
170171
Relationindex=info->index;
171172
BlockNumbernpages,
172173
blkno;
173-
BlockNumbertotFreePages;
174174

175175
if (info->analyze_only)
176176
returnstats;
@@ -183,7 +183,9 @@ blvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
183183
* statistics.
184184
*/
185185
npages=RelationGetNumberOfBlocks(index);
186-
totFreePages=0;
186+
stats->num_pages=npages;
187+
stats->pages_free=0;
188+
stats->num_index_tuples=0;
187189
for (blkno=BLOOM_HEAD_BLKNO;blkno<npages;blkno++)
188190
{
189191
Bufferbuffer;
@@ -196,23 +198,20 @@ blvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
196198
LockBuffer(buffer,BUFFER_LOCK_SHARE);
197199
page= (Page)BufferGetPage(buffer);
198200

199-
if (BloomPageIsDeleted(page))
201+
if (PageIsNew(page)||BloomPageIsDeleted(page))
200202
{
201203
RecordFreeIndexPage(index,blkno);
202-
totFreePages++;
204+
stats->pages_free++;
203205
}
204206
else
205207
{
206208
stats->num_index_tuples+=BloomPageGetMaxOffset(page);
207-
stats->estimated_count+=BloomPageGetMaxOffset(page);
208209
}
209210

210211
UnlockReleaseBuffer(buffer);
211212
}
212213

213214
IndexFreeSpaceMapVacuum(info->index);
214-
stats->pages_free=totFreePages;
215-
stats->num_pages=RelationGetNumberOfBlocks(index);
216215

217216
returnstats;
218217
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp