On Sat, Jan 12, 2013 at 02:14:03PM -0500, Kevin Grittner wrote:
> Amit Kapila wrote:
> > On Thursday, January 10, 2013 6:09 AM Josh Berkus wrote:
>
> >> Surely VACUUM FULL should rebuild the visibility map, and make
> >> tuples in the new relation all-visible, no?
>
> Certainly it seems odd to me that VACUUM FULL leaves the the table
> in a less-well maintained state in terms of visibility than a
> "normal" vacuum. VACUUM FULL should not need to be followed by
> another VACUUM.
>
> > I think it cannot made all visible.
>
> I don't think all tuples in the relation are necessarily visible to
> all transactions, but the ones which are should probably be flagged
> that way.
I have developed the attached proof-of-concept patch to fix the problem
of having no visibility map after CLUSTER or VACUUM FULL. I tested with
these queries:
CREATE TABLE test(x INT PRIMARY KEY);
INSERT INTO test VALUES (1);
VACUUM FULL test; -- or CLUSTER
SELECT relfilenode FROM pg_class WHERE relname = 'test';
relfilenode
-------------
16399
Then 'ls -l data/base/16384/16399*' to see the *_vm file. I am not sure
how to test that the vm contents are valid.
This patch is fairly tricky because our CLUSTER/VACUUM FULL behavior
does not do writes through the shared buffer cache, as outlined in this
C comment block:
* We can't use the normal heap_insert function to insert into the new
* heap, because heap_insert overwrites the visibility information.
* We use a special-purpose raw_heap_insert function instead, which
* is optimized for bulk inserting a lot of tuples, knowing that we have
* exclusive access to the heap. raw_heap_insert builds new pages in
* local storage. When a page is full, or at the end of the process,
* we insert it to WAL as a single record and then write it to disk
* directly through smgr. Note, however, that any data sent to the new
* heap's TOAST table will go through the normal bufmgr.
I originally tried to do this higher up in the stack but ran into
problems because I couldn't access the new heap page so I had to do it
at the non-shared-buffer page level. I reused the lazy vacuum routines.
I need to know this is the right approach, and need to know what things
are wrong or missing.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
new file mode 100644
index 951894c..c01a6a8
*** a/src/backend/access/heap/rewriteheap.c
--- b/src/backend/access/heap/rewriteheap.c
***************
*** 107,112 ****
--- 107,114 ----
#include "access/rewriteheap.h"
#include "access/transam.h"
#include "access/tuptoaster.h"
+ #include "access/visibilitymap.h"
+ #include "commands/vacuum.h"
#include "storage/bufmgr.h"
#include "storage/smgr.h"
#include "utils/memutils.h"
*************** typedef OldToNewMappingData *OldToNewMap
*** 172,177 ****
--- 174,180 ----
/* prototypes for internal functions */
static void raw_heap_insert(RewriteState state, HeapTuple tup);
+ static void update_page_vm(Relation relation, Page page, BlockNumber blkno);
/*
*************** end_heap_rewrite(RewriteState state)
*** 281,286 ****
--- 284,290 ----
RelationOpenSmgr(state->rs_new_rel);
PageSetChecksumInplace(state->rs_buffer, state->rs_blockno);
+ update_page_vm(state->rs_new_rel, state->rs_buffer, state->rs_blockno);
smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM, state->rs_blockno,
(char *) state->rs_buffer, true);
*************** raw_heap_insert(RewriteState state, Heap
*** 633,638 ****
--- 637,643 ----
RelationOpenSmgr(state->rs_new_rel);
PageSetChecksumInplace(page, state->rs_blockno);
+ update_page_vm(state->rs_new_rel, page, state->rs_blockno);
smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM,
state->rs_blockno, (char *) page, true);
*************** raw_heap_insert(RewriteState state, Heap
*** 677,679 ****
--- 682,704 ----
if (heaptup != tup)
heap_freetuple(heaptup);
}
+
+ static void
+ update_page_vm(Relation relation, Page page, BlockNumber blkno)
+ {
+ Buffer vmbuffer = InvalidBuffer;
+ TransactionId visibility_cutoff_xid;
+
+ visibilitymap_pin(relation, blkno, &vmbuffer);
+ Assert(BufferIsValid(vmbuffer));
+
+ if (!visibilitymap_test(relation, blkno, &vmbuffer) &&
+ heap_page_is_all_visible(relation, InvalidBuffer, page,
+ &visibility_cutoff_xid))
+ {
+ PageSetAllVisible(page);
+ visibilitymap_set(relation, blkno, InvalidBlockNumber,
+ InvalidXLogRecPtr, vmbuffer, visibility_cutoff_xid);
+ }
+ ReleaseBuffer(vmbuffer);
+ }
diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
new file mode 100644
index 7f40d89..a42511b
*** a/src/backend/access/heap/visibilitymap.c
--- b/src/backend/access/heap/visibilitymap.c
*************** visibilitymap_set(Relation rel, BlockNum
*** 278,284 ****
map[mapByte] |= (1 << mapBit);
MarkBufferDirty(vmBuf);
! if (RelationNeedsWAL(rel))
{
if (XLogRecPtrIsInvalid(recptr))
{
--- 278,284 ----
map[mapByte] |= (1 << mapBit);
MarkBufferDirty(vmBuf);
! if (RelationNeedsWAL(rel) && BufferIsValid(heapBuf))
{
if (XLogRecPtrIsInvalid(recptr))
{
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
new file mode 100644
index 6688ab3..ea349a5
*** a/src/backend/commands/vacuumlazy.c
--- b/src/backend/commands/vacuumlazy.c
*************** static void lazy_record_dead_tuple(LVRel
*** 151,158 ****
ItemPointer itemptr);
static bool lazy_tid_reaped(ItemPointer itemptr, void *state);
static int vac_cmp_itemptr(const void *left, const void *right);
- static bool heap_page_is_all_visible(Relation rel, Buffer buf,
- TransactionId *visibility_cutoff_xid);
/*
--- 151,156 ----
*************** lazy_vacuum_page(Relation onerel, BlockN
*** 1187,1193 ****
* check if the page has become all-visible.
*/
if (!visibilitymap_test(onerel, blkno, vmbuffer) &&
! heap_page_is_all_visible(onerel, buffer, &visibility_cutoff_xid))
{
Assert(BufferIsValid(*vmbuffer));
PageSetAllVisible(page);
--- 1185,1191 ----
* check if the page has become all-visible.
*/
if (!visibilitymap_test(onerel, blkno, vmbuffer) &&
! heap_page_is_all_visible(onerel, buffer, NULL, &visibility_cutoff_xid))
{
Assert(BufferIsValid(*vmbuffer));
PageSetAllVisible(page);
*************** vac_cmp_itemptr(const void *left, const
*** 1694,1707 ****
* transactions. Also return the visibility_cutoff_xid which is the highest
* xmin amongst the visible tuples.
*/
! static bool
! heap_page_is_all_visible(Relation rel, Buffer buf, TransactionId *visibility_cutoff_xid)
{
- Page page = BufferGetPage(buf);
OffsetNumber offnum,
maxoff;
bool all_visible = true;
*visibility_cutoff_xid = InvalidTransactionId;
/*
--- 1692,1708 ----
* transactions. Also return the visibility_cutoff_xid which is the highest
* xmin amongst the visible tuples.
*/
! bool
! heap_page_is_all_visible(Relation rel, Buffer buf, Page page,
! TransactionId *visibility_cutoff_xid)
{
OffsetNumber offnum,
maxoff;
bool all_visible = true;
+ if (BufferIsValid(buf))
+ page = BufferGetPage(buf);
+
*visibility_cutoff_xid = InvalidTransactionId;
/*
*************** heap_page_is_all_visible(Relation rel, B
*** 1722,1728 ****
if (!ItemIdIsUsed(itemid) || ItemIdIsRedirected(itemid))
continue;
! ItemPointerSet(&(tuple.t_self), BufferGetBlockNumber(buf), offnum);
/*
* Dead line pointers can have index pointers pointing to them. So
--- 1723,1731 ----
if (!ItemIdIsUsed(itemid) || ItemIdIsRedirected(itemid))
continue;
! /* XXX use 0 or real offset? */
! ItemPointerSet(&(tuple.t_self), BufferIsValid(buf) ?
! BufferGetBlockNumber(buf) : 0, offnum);
/*
* Dead line pointers can have index pointers pointing to them. So
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
new file mode 100644
index ed66c49..7ad8b58
*** a/src/backend/utils/time/tqual.c
--- b/src/backend/utils/time/tqual.c
*************** static inline void
*** 108,113 ****
--- 108,117 ----
SetHintBits(HeapTupleHeader tuple, Buffer buffer,
uint16 infomask, TransactionId xid)
{
+ /* we might not have a buffer if we are doing raw_heap_insert() */
+ if (!BufferIsValid(buffer))
+ return;
+
if (TransactionIdIsValid(xid))
{
/* NB: xid must be known committed here! */
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
new file mode 100644
index 08bec25..04cd4b7
*** a/src/include/commands/vacuum.h
--- b/src/include/commands/vacuum.h
***************
*** 19,24 ****
--- 19,25 ----
#include "catalog/pg_type.h"
#include "nodes/parsenodes.h"
#include "storage/buf.h"
+ #include "storage/bufpage.h"
#include "storage/lock.h"
#include "utils/relcache.h"
*************** extern void vacuum_delay_point(void);
*** 167,172 ****
--- 168,175 ----
/* in commands/vacuumlazy.c */
extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
BufferAccessStrategy bstrategy);
+ extern bool heap_page_is_all_visible(Relation rel, Buffer buf, Page page,
+ TransactionId *visibility_cutoff_xid);
/* in commands/analyze.c */
extern void analyze_rel(Oid relid, VacuumStmt *vacstmt,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
