Skip to content

Commit ddfc556

Browse files
nmischnmisch
committed
Revert "Optimize pg_visibility with read streams."
This reverts commit ed1b1ee and its followup 1c61fd8. They rendered collect_corrupt_items() unable to detect corruption. Discussion: https://postgr.es/m/CAN55FZ1_Ru3XpMgTwsU67FTH2fs_FrRROmb7x6zs+F44QBEiww@mail.gmail.com Discussion: https://postgr.es/m/CAEudQAozv3wTY5TV2t29JcwPydbmKbiWQkZD42S2OgzdixPMDQ@mail.gmail.com
1 parent 82b07eb commit ddfc556

File tree

1 file changed

+21
-92
lines changed

1 file changed

+21
-92
lines changed

‎contrib/pg_visibility/pg_visibility.c

Lines changed: 21 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include "storage/bufmgr.h"
2222
#include "storage/proc.h"
2323
#include "storage/procarray.h"
24-
#include "storage/read_stream.h"
2524
#include "storage/smgr.h"
2625
#include "utils/rel.h"
2726
#include "utils/snapmgr.h"
@@ -42,17 +41,6 @@ typedef struct corrupt_items
4241
ItemPointer tids;
4342
} corrupt_items;
4443

45-
/* for collect_corrupt_items_read_stream_next_block */
46-
struct collect_corrupt_items_read_stream_private
47-
{
48-
bool all_frozen;
49-
bool all_visible;
50-
BlockNumber current_blocknum;
51-
BlockNumber last_exclusive;
52-
Relation rel;
53-
Buffer vmbuffer;
54-
};
55-
5644
PG_FUNCTION_INFO_V1(pg_visibility_map);
5745
PG_FUNCTION_INFO_V1(pg_visibility_map_rel);
5846
PG_FUNCTION_INFO_V1(pg_visibility);
@@ -490,8 +478,6 @@ collect_visibility_data(Oid relid, bool include_pd)
490478
BlockNumber blkno;
491479
Buffer vmbuffer = InvalidBuffer;
492480
BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
493-
BlockRangeReadStreamPrivate p;
494-
ReadStream *stream = NULL;
495481

496482
rel = relation_open(relid, AccessShareLock);
497483

@@ -503,20 +489,6 @@ collect_visibility_data(Oid relid, bool include_pd)
503489
info->next = 0;
504490
info->count = nblocks;
505491

506-
/* Create a stream if reading main fork. */
507-
if (include_pd)
508-
{
509-
p.current_blocknum = 0;
510-
p.last_exclusive = nblocks;
511-
stream = read_stream_begin_relation(READ_STREAM_FULL,
512-
bstrategy,
513-
rel,
514-
MAIN_FORKNUM,
515-
block_range_read_stream_cb,
516-
&p,
517-
0);
518-
}
519-
520492
for (blkno = 0; blkno < nblocks; ++blkno)
521493
{
522494
int32 mapbits;
@@ -541,7 +513,8 @@ collect_visibility_data(Oid relid, bool include_pd)
541513
Buffer buffer;
542514
Page page;
543515

544-
buffer = read_stream_next_buffer(stream, NULL);
516+
buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
517+
bstrategy);
545518
LockBuffer(buffer, BUFFER_LOCK_SHARE);
546519

547520
page = BufferGetPage(buffer);
@@ -552,12 +525,6 @@ collect_visibility_data(Oid relid, bool include_pd)
552525
}
553526
}
554527

555-
if (include_pd)
556-
{
557-
Assert(read_stream_next_buffer(stream, NULL) == InvalidBuffer);
558-
read_stream_end(stream);
559-
}
560-
561528
/* Clean up. */
562529
if (vmbuffer != InvalidBuffer)
563530
ReleaseBuffer(vmbuffer);
@@ -643,38 +610,6 @@ GetStrictOldestNonRemovableTransactionId(Relation rel)
643610
}
644611
}
645612

646-
/*
647-
* Callback function to get next block for read stream object used in
648-
* collect_corrupt_items() function.
649-
*/
650-
static BlockNumber
651-
collect_corrupt_items_read_stream_next_block(ReadStream *stream,
652-
void *callback_private_data,
653-
void *per_buffer_data)
654-
{
655-
struct collect_corrupt_items_read_stream_private *p = callback_private_data;
656-
657-
for (; p->current_blocknum < p->last_exclusive; p->current_blocknum++)
658-
{
659-
bool check_frozen = false;
660-
bool check_visible = false;
661-
662-
/* Make sure we are interruptible. */
663-
CHECK_FOR_INTERRUPTS();
664-
665-
if (p->all_frozen && VM_ALL_FROZEN(p->rel, p->current_blocknum, &p->vmbuffer))
666-
check_frozen = true;
667-
if (p->all_visible && VM_ALL_VISIBLE(p->rel, p->current_blocknum, &p->vmbuffer))
668-
check_visible = true;
669-
if (!check_visible && !check_frozen)
670-
continue;
671-
672-
return p->current_blocknum++;
673-
}
674-
675-
return InvalidBlockNumber;
676-
}
677-
678613
/*
679614
* Returns a list of items whose visibility map information does not match
680615
* the status of the tuples on the page.
@@ -693,13 +628,12 @@ static corrupt_items *
693628
collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
694629
{
695630
Relation rel;
631+
BlockNumber nblocks;
696632
corrupt_items *items;
633+
BlockNumber blkno;
697634
Buffer vmbuffer = InvalidBuffer;
698635
BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
699636
TransactionId OldestXmin = InvalidTransactionId;
700-
struct collect_corrupt_items_read_stream_private p;
701-
ReadStream *stream;
702-
Buffer buffer;
703637

704638
rel = relation_open(relid, AccessShareLock);
705639

@@ -709,6 +643,8 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
709643
if (all_visible)
710644
OldestXmin = GetStrictOldestNonRemovableTransactionId(rel);
711645

646+
nblocks = RelationGetNumberOfBlocks(rel);
647+
712648
/*
713649
* Guess an initial array size. We don't expect many corrupted tuples, so
714650
* start with a small array. This function uses the "next" field to track
@@ -722,46 +658,42 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
722658
items->count = 64;
723659
items->tids = palloc(items->count * sizeof(ItemPointerData));
724660

725-
p.current_blocknum = 0;
726-
p.last_exclusive = RelationGetNumberOfBlocks(rel);
727-
p.rel = rel;
728-
p.vmbuffer = InvalidBuffer;
729-
p.all_frozen = all_frozen;
730-
p.all_visible = all_visible;
731-
stream = read_stream_begin_relation(READ_STREAM_FULL,
732-
bstrategy,
733-
rel,
734-
MAIN_FORKNUM,
735-
collect_corrupt_items_read_stream_next_block,
736-
&p,
737-
0);
738-
739661
/* Loop over every block in the relation. */
740-
while ((buffer = read_stream_next_buffer(stream, NULL)) != InvalidBuffer)
662+
for (blkno = 0; blkno < nblocks; ++blkno)
741663
{
742664
bool check_frozen = false;
743665
bool check_visible = false;
666+
Buffer buffer;
744667
Page page;
745668
OffsetNumber offnum,
746669
maxoff;
747-
BlockNumber blkno;
748670

749671
/* Make sure we are interruptible. */
750672
CHECK_FOR_INTERRUPTS();
751673

674+
/* Use the visibility map to decide whether to check this page. */
675+
if (all_frozen && VM_ALL_FROZEN(rel, blkno, &vmbuffer))
676+
check_frozen = true;
677+
if (all_visible && VM_ALL_VISIBLE(rel, blkno, &vmbuffer))
678+
check_visible = true;
679+
if (!check_visible && !check_frozen)
680+
continue;
681+
682+
/* Read and lock the page. */
683+
buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL,
684+
bstrategy);
752685
LockBuffer(buffer, BUFFER_LOCK_SHARE);
753686

754687
page = BufferGetPage(buffer);
755688
maxoff = PageGetMaxOffsetNumber(page);
756-
blkno = BufferGetBlockNumber(buffer);
757689

758690
/*
759691
* The visibility map bits might have changed while we were acquiring
760692
* the page lock. Recheck to avoid returning spurious results.
761693
*/
762-
if (all_frozen && !VM_ALL_FROZEN(rel, blkno, &vmbuffer))
694+
if (check_frozen && !VM_ALL_FROZEN(rel, blkno, &vmbuffer))
763695
check_frozen = false;
764-
if (all_visible && !VM_ALL_VISIBLE(rel, blkno, &vmbuffer))
696+
if (check_visible && !VM_ALL_VISIBLE(rel, blkno, &vmbuffer))
765697
check_visible = false;
766698
if (!check_visible && !check_frozen)
767699
{
@@ -846,13 +778,10 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
846778

847779
UnlockReleaseBuffer(buffer);
848780
}
849-
read_stream_end(stream);
850781

851782
/* Clean up. */
852783
if (vmbuffer != InvalidBuffer)
853784
ReleaseBuffer(vmbuffer);
854-
if (p.vmbuffer != InvalidBuffer)
855-
ReleaseBuffer(p.vmbuffer);
856785
relation_close(rel, AccessShareLock);
857786

858787
/*

0 commit comments

Comments
 (0)