Re: [HACKERS] visibility maps and heap_prune
Bruce Momjian wrote: Pavan Deolasee wrote: On Fri, Feb 26, 2010 at 8:19 AM, Bruce Momjian br...@momjian.us wrote: Whatever happened to this? It was in the first 9.0 commitfest but was returned with feedback but never updated: Though Alex did some useful tests and review, and in fact confirmed that the VACUUM time dropped from 16494 msec to 366 msec, I somehow kept waiting for Heikki's decision on the general direction of the patch and lost interest in between. If we are still interested in this, I can work out a patch and submit for next release if not this. OK, TODO added: Have single-page pruning update the visibility map * https://commitfest.postgresql.org/action/patch_view?id=75 Hopefully Heikki can comment on this. I think I was worried about the possible performance impact of having to clear the bit in visibility map again. If you're frequently updating a tuple so that HOT and page pruning is helping you, setting the bit in visibility map seems counter-productive; it's going to be cleared soon again by another UPDATE. That's just a hunch, though. Maybe the overhead is negligible. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
Heikki Linnakangas wrote: Bruce Momjian wrote: Pavan Deolasee wrote: On Fri, Feb 26, 2010 at 8:19 AM, Bruce Momjian br...@momjian.us wrote: Whatever happened to this? It was in the first 9.0 commitfest but was returned with feedback but never updated: Though Alex did some useful tests and review, and in fact confirmed that the VACUUM time dropped from 16494 msec to 366 msec, I somehow kept waiting for Heikki's decision on the general direction of the patch and lost interest in between. If we are still interested in this, I can work out a patch and submit for next release if not this. OK, TODO added: Have single-page pruning update the visibility map * https://commitfest.postgresql.org/action/patch_view?id=75 Hopefully Heikki can comment on this. I think I was worried about the possible performance impact of having to clear the bit in visibility map again. If you're frequently updating a tuple so that HOT and page pruning is helping you, setting the bit in visibility map seems counter-productive; it's going to be cleared soon again by another UPDATE. That's just a hunch, though. Maybe the overhead is negligible. Should I remove the TODO item? I updated the text to: Consider having single-page pruning update the visibility map and added a URL to Heikki's new comment. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
Pavan Deolasee wrote: On Fri, Feb 26, 2010 at 8:19 AM, Bruce Momjian br...@momjian.us wrote: Whatever happened to this? It was in the first 9.0 commitfest but was returned with feedback but never updated: Though Alex did some useful tests and review, and in fact confirmed that the VACUUM time dropped from 16494 msec to 366 msec, I somehow kept waiting for Heikki's decision on the general direction of the patch and lost interest in between. If we are still interested in this, I can work out a patch and submit for next release if not this. OK, TODO added: Have single-page pruning update the visibility map * https://commitfest.postgresql.org/action/patch_view?id=75 Hopefully Heikki can comment on this. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
Whatever happened to this? It was in the first 9.0 commitfest but was returned with feedback but never updated: https://commitfest.postgresql.org/action/patch_view?id=75 --- Pavan Deolasee wrote: ISTM that the PD_ALL_VISIBLE flag and the visibility map bit can be set at the end of pruning operation if we know that there are only tuples visible to all transactions left in the page. The way pruning is done, I think it would be straight forward to get this information. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
On Thu, Feb 25, 2010 at 9:49 PM, Bruce Momjian br...@momjian.us wrote: Whatever happened to this? It was in the first 9.0 commitfest but was returned with feedback but never updated: https://commitfest.postgresql.org/action/patch_view?id=75 Well, the patch author chose not to pursue it. It's clearly far too late now, at least for 9.0. I'm pleased to see that you're not finding many patches that just completely slipped through the cracks - seems like most things were withdrawn on purpose, had problems, and/or were not pursued by the author. I think the CommitFest process has done a pretty good job of making sure everything gets looked at. The only small chink I see is that there may be some patches (especially small ones or from first-time contributors) which escaped getting added to a CommitFest in the first place; and we don't really have a way of policing that. Usually someone replies to the patch author and suggests adding it to the next CF, but I can't swear that that happens in every case. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
Robert Haas wrote: On Thu, Feb 25, 2010 at 9:49 PM, Bruce Momjian br...@momjian.us wrote: Whatever happened to this? ?It was in the first 9.0 commitfest but was returned with feedback but never updated: ? ? ? ?https://commitfest.postgresql.org/action/patch_view?id=75 Well, the patch author chose not to pursue it. It's clearly far too late now, at least for 9.0. I'm pleased to see that you're not finding many patches that just completely slipped through the cracks - seems like most things were withdrawn on purpose, had problems, and/or were not pursued by the author. I think the CommitFest process has done a pretty good job of making sure everything gets looked at. The only small chink I see is that there may be some patches (especially small ones or from first-time contributors) which escaped getting added to a CommitFest in the first place; and we don't really have a way of policing that. Usually someone replies to the patch author and suggests adding it to the next CF, but I can't swear that that happens in every case. Yea, the complex issues are often lost, and I stopped tracking commitfest items so I don't actually know if anything that got into the commit fest was eventually just dropped by the author. We can say we don't need to persue those but they might be valuable/important. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
On Thu, Feb 25, 2010 at 10:32 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: On Thu, Feb 25, 2010 at 9:49 PM, Bruce Momjian br...@momjian.us wrote: Whatever happened to this? ?It was in the first 9.0 commitfest but was returned with feedback but never updated: ? ? ? ?https://commitfest.postgresql.org/action/patch_view?id=75 Well, the patch author chose not to pursue it. It's clearly far too late now, at least for 9.0. I'm pleased to see that you're not finding many patches that just completely slipped through the cracks - seems like most things were withdrawn on purpose, had problems, and/or were not pursued by the author. I think the CommitFest process has done a pretty good job of making sure everything gets looked at. The only small chink I see is that there may be some patches (especially small ones or from first-time contributors) which escaped getting added to a CommitFest in the first place; and we don't really have a way of policing that. Usually someone replies to the patch author and suggests adding it to the next CF, but I can't swear that that happens in every case. Yea, the complex issues are often lost, and I stopped tracking commitfest items so I don't actually know if anything that got into the commit fest was eventually just dropped by the author. We can say we don't need to persue those but they might be valuable/important. Yes, they could be valuable/important - anything that falls into that category is probably going to turn into a TODO list item at this point. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
On Fri, Feb 26, 2010 at 8:19 AM, Bruce Momjian br...@momjian.us wrote: Whatever happened to this? It was in the first 9.0 commitfest but was returned with feedback but never updated: Though Alex did some useful tests and review, and in fact confirmed that the VACUUM time dropped from 16494 msec to 366 msec, I somehow kept waiting for Heikki's decision on the general direction of the patch and lost interest in between. If we are still interested in this, I can work out a patch and submit for next release if not this. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] visibility maps and heap_prune
On Tue, Jul 21, 2009 at 2:37 AM, Pavan Deolaseepavan.deola...@gmail.com wrote: On Tue, Jul 21, 2009 at 10:38 AM, Robert Haasrobertmh...@gmail.com wrote: Pavan, are you planning to respond to Alex's comments and/or update this patch? Yes, I will. Hopefully by end of this week. Since it has now been 10 days since this patch was reviewed, I think that it is more than fair to move this from Waiting on Author to Returned with Feedback. As I've said on other threads, we want to give everyone a fair chance to respond to review comments, but we also don't want to tie up reviewers indefinitely on patches that aren't being updated in a timely fashion, and we don't want to be left with a crush of patches that need to be re-reviewed at the very end of the CommitFest when suddenly everyone updates them. So I'm going to go make this change. I hope, though, that this will be resubmitted, after appropriate updating, for a future CommitFest. I haven't read the code so I can't speak at all to whether it works (in which I'm including crash-safe, deadlock-proof, and correct with respect to locking), but if so it sounds like a nice improvement. Thanks, ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
On Tue, Jul 21, 2009 at 10:38 AM, Robert Haasrobertmh...@gmail.com wrote: Pavan, are you planning to respond to Alex's comments and/or update this patch? Yes, I will. Hopefully by end of this week. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
On Wed, Jul 15, 2009 at 11:44 PM, Alex Hunsakerbada...@gmail.com wrote: On Mon, Dec 8, 2008 at 06:56, Pavan Deolaseepavan.deola...@gmail.com wrote: Here is a patch which implements this. The PD_ALL_VISIBLE flag is set if all tuples in the page are visible to all transactions and there are no DEAD line pointers in the page. The second check is required so that VACUUM takes up the page. We could slightly distinguish the two cases (one where the page requires vacuuming only because of DEAD line pointers and the other where the page-tuples do not require any visibility checks), but I thought its not worth the complexity. Hi! I was round robin assigned to review this. So take my comments with the grain of salt (or novice HOT salt) they deserve. Pavan, are you planning to respond to Alex's comments and/or update this patch? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
On Mon, Dec 8, 2008 at 06:56, Pavan Deolaseepavan.deola...@gmail.com wrote:
Here is a patch which implements this. The PD_ALL_VISIBLE flag is set if all
tuples in the page are visible to all transactions and there are no DEAD
line pointers in the page. The second check is required so that VACUUM takes
up the page. We could slightly distinguish the two cases (one where the page
requires vacuuming only because of DEAD line pointers and the other where
the page-tuples do not require any visibility checks), but I thought its not
worth the complexity.
Hi!
I was round robin assigned to review this. So take my comments with
the grain of salt (or novice HOT salt) they deserve.
I did some quick performance testing that basically boiled down to:
insert
(hot) update
select
to see if I could detect any noticeable performance difference (see
attachments for more detail for exact queries ran, all run with
autovac off).
The only major difference was with this patch vacuum time (after the
first select after some hot updates) was significantly reduced for my
test case (366ms vs 16494ms).
There was no noticeable (within noise) select or update slow down.
I was able to trigger WARNING: PD_ALL_VISIBLE flag once while running
pgbench but have not be able to re-create it... (should I keep
trying?)
See comments on patch below...
Index: src/backend/access/heap/pruneheap.c
snip
*** heap_page_prune_opt(Relation relation, B
*** 118,125
(void) heap_page_prune(relation, buffer, OldestXmin, false, true);
}
! /* And release buffer lock */
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
}
}
--- 124,150
(void) heap_page_prune(relation, buffer, OldestXmin, false, true);
}
! /*
! * Since the visibility map page may require an I/O,release the buffer
! * lock before updating the visibility map.
! */
Would it be worth having heap_page_prune() return or pass in a ptr so
we can saw we need to update the visibility map because we set/changed
PageIsAllVisible?
! if (PageIsAllVisible(page))
! {
! Buffer vmbuffer = InvalidBuffer;
! /* Release buffer lock */
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
!
! visibilitymap_pin(relation, BufferGetBlockNumber(buffer),
vmbuffer);
! LockBuffer(buffer, BUFFER_LOCK_SHARE);
! if (PageIsAllVisible(page))
! visibilitymap_set(relation, BufferGetBlockNumber(buffer),
!PageGetLSN(page), vmbuffer);
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! if (BufferIsValid(vmbuffer))
! ReleaseBuffer(vmbuffer);
! }
! else
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
}
}
snip
*** heap_page_prune(Relation relation, Buffe
*** 245,250
--- 277,291
*/
PageClearFull(page);
+ /* Update the all-visible flag on the page */
+ if (!PageIsAllVisible(page) prstate.all_visible)
+ PageSetAllVisible(page);
+ else if (PageIsAllVisible(page) !prstate.all_visible)
+ {
+ elog(WARNING, PD_ALL_VISIBLE flag was incorrectly set);
+ PageClearAllVisible(page);
Hrm do we need to update the visibility map ? AFAICT this wont update
it it because we only check for PageIsAllVisible() in
heap_page_prune_opt().
+ }
+
MarkBufferDirty(buffer);
/*
*** heap_page_prune(Relation relation, Buffe
*** 282,287
--- 323,341
PageClearFull(page);
SetBufferCommitInfoNeedsSave(buffer);
}
+
+ /* Update the all-visible flag on the page */
+ if (!PageIsAllVisible(page) prstate.all_visible)
+ {
+ PageSetAllVisible(page);
+ SetBufferCommitInfoNeedsSave(buffer);
+ }
+ else if (PageIsAllVisible(page) !prstate.all_visible)
+ {
+ elog(WARNING, PD_ALL_VISIBLE flag was incorrectly set);
+ PageClearAllVisible(page);
+ SetBufferCommitInfoNeedsSave(buffer);
Same question as above.
+ }
}
END_CRIT_SECTION();
snip
*** heap_prune_chain(Relation relation, Buff
*** 495,503
--- 557,596
*/
heap_prune_record_prunable(prstate,
HeapTupleHeaderGetXmax(htup));
+ prstate-all_visible = false;
break;
case HEAPTUPLE_LIVE:
+ /*
+ * Is the tuple definitely visible to all
transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't
set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more
info. Check
+
Re: [HACKERS] visibility maps and heap_prune
Pavan Deolasee wrote: On Mon, Dec 8, 2008 at 11:33 AM, Pavan Deolasee pavan.deola...@gmail.comwrote: On Sat, Dec 6, 2008 at 8:08 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: If you see a straightforward way, please submit a patch! Will do that. Here is a patch which implements this. The PD_ALL_VISIBLE flag is set if all tuples in the page are visible to all transactions and there are no DEAD line pointers in the page. The second check is required so that VACUUM takes up the page. We could slightly distinguish the two cases (one where the page requires vacuuming only because of DEAD line pointers and the other where the page-tuples do not require any visibility checks), but I thought its not worth the complexity. Is this patch for 8.4? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
Bruce Momjian wrote: Pavan Deolasee wrote: On Mon, Dec 8, 2008 at 11:33 AM, Pavan Deolasee pavan.deola...@gmail.comwrote: On Sat, Dec 6, 2008 at 8:08 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: If you see a straightforward way, please submit a patch! Will do that. Here is a patch which implements this. The PD_ALL_VISIBLE flag is set if all tuples in the page are visible to all transactions and there are no DEAD line pointers in the page. The second check is required so that VACUUM takes up the page. We could slightly distinguish the two cases (one where the page requires vacuuming only because of DEAD line pointers and the other where the page-tuples do not require any visibility checks), but I thought its not worth the complexity. Is this patch for 8.4? We already went through this: http://archives.postgresql.org/message-id/496f6a8e.8020...@enterprisedb.com I guess I'll follow Robert's advice and add this to the first 8.5 commit fest page. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
On Thu, Jan 15, 2009 at 7:10 AM, Bruce Momjian br...@momjian.us wrote: Is this something for 8.4 CVS? I worked out the patch as per Heikki's suggestion. So I think he needs to review and decide it's fate. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
Pavan Deolasee wrote: On Thu, Jan 15, 2009 at 7:10 AM, Bruce Momjian br...@momjian.us wrote: Is this something for 8.4 CVS? I worked out the patch as per Heikki's suggestion. So I think he needs to review and decide it's fate. Yeah, I dropped the ball on that one. It's been knocking in the back of my head since, but I've never gotten around. I'm feeling reluctant to review it since it's not really a high priority thing, and I'm not sure whether we want it or not. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
Yeah, I dropped the ball on that one. It's been knocking in the back of my head since, but I've never gotten around. I'm feeling reluctant to review it since it's not really a high priority thing, and I'm not sure whether we want it or not. In that case perhaps we should add it to http://wiki.postgresql.org/wiki/CommitFest_2009-First and let it go for 8.4. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
Is this something for 8.4 CVS? --- Pavan Deolasee wrote: On Mon, Dec 8, 2008 at 11:33 AM, Pavan Deolasee pavan.deola...@gmail.comwrote: On Sat, Dec 6, 2008 at 8:08 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: If you see a straightforward way, please submit a patch! Will do that. Here is a patch which implements this. The PD_ALL_VISIBLE flag is set if all tuples in the page are visible to all transactions and there are no DEAD line pointers in the page. The second check is required so that VACUUM takes up the page. We could slightly distinguish the two cases (one where the page requires vacuuming only because of DEAD line pointers and the other where the page-tuples do not require any visibility checks), but I thought its not worth the complexity. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com [ Attachment, skipping... ] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] visibility maps and heap_prune
On Mon, Dec 8, 2008 at 11:33 AM, Pavan Deolasee [EMAIL PROTECTED]wrote:
On Sat, Dec 6, 2008 at 8:08 PM, Heikki Linnakangas
[EMAIL PROTECTED] wrote:
If you see a straightforward way, please submit a patch!
Will do that.
Here is a patch which implements this. The PD_ALL_VISIBLE flag is set if all
tuples in the page are visible to all transactions and there are no DEAD
line pointers in the page. The second check is required so that VACUUM takes
up the page. We could slightly distinguish the two cases (one where the page
requires vacuuming only because of DEAD line pointers and the other where
the page-tuples do not require any visibility checks), but I thought its not
worth the complexity.
Thanks,
Pavan
--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com
Index: src/backend/access/heap/pruneheap.c
===
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/access/heap/pruneheap.c,v
retrieving revision 1.16
diff -c -p -r1.16 pruneheap.c
*** src/backend/access/heap/pruneheap.c 13 Jul 2008 20:45:47 - 1.16
--- src/backend/access/heap/pruneheap.c 8 Dec 2008 13:24:53 -
***
*** 17,22
--- 17,23
#include access/heapam.h
#include access/htup.h
#include access/transam.h
+ #include access/visibilitymap.h
#include miscadmin.h
#include pgstat.h
#include storage/bufmgr.h
*** typedef struct
*** 39,44
--- 40,50
OffsetNumber nowunused[MaxHeapTuplesPerPage];
/* marked[i] is TRUE if item i is entered in one of the above arrays */
bool marked[MaxHeapTuplesPerPage + 1];
+ /*
+ * all_visible is TRUE if all tuples in the page are visible to all
+ * transactions and there are no DEAD line pointers in the page.
+ */
+ bool all_visible;
} PruneState;
/* Local functions */
*** heap_page_prune_opt(Relation relation, B
*** 118,125
(void) heap_page_prune(relation, buffer, OldestXmin, false, true);
}
! /* And release buffer lock */
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
}
}
--- 124,150
(void) heap_page_prune(relation, buffer, OldestXmin, false, true);
}
! /*
! * Since the visibility map page may require an I/O, release the buffer
! * lock before updating the visibility map.
! */
! if (PageIsAllVisible(page))
! {
! Buffer vmbuffer = InvalidBuffer;
! /* Release buffer lock */
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
!
! visibilitymap_pin(relation, BufferGetBlockNumber(buffer), vmbuffer);
! LockBuffer(buffer, BUFFER_LOCK_SHARE);
! if (PageIsAllVisible(page))
! visibilitymap_set(relation, BufferGetBlockNumber(buffer),
! PageGetLSN(page), vmbuffer);
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
! if (BufferIsValid(vmbuffer))
! ReleaseBuffer(vmbuffer);
! }
! else
! LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
}
}
*** heap_page_prune(Relation relation, Buffe
*** 178,183
--- 203,209
prstate.new_prune_xid = InvalidTransactionId;
prstate.nredirected = prstate.ndead = prstate.nunused = 0;
memset(prstate.marked, 0, sizeof(prstate.marked));
+ prstate.all_visible = true;
/* Scan the page */
maxoff = PageGetMaxOffsetNumber(page);
*** heap_page_prune(Relation relation, Buffe
*** 193,200
/* Nothing to do if slot is empty or already dead */
itemid = PageGetItemId(page, offnum);
! if (!ItemIdIsUsed(itemid) || ItemIdIsDead(itemid))
continue;
/* Process this item or chain of items */
ndeleted += heap_prune_chain(relation, buffer, offnum,
--- 219,232
/* Nothing to do if slot is empty or already dead */
itemid = PageGetItemId(page, offnum);
! if (!ItemIdIsUsed(itemid))
! continue;
! if (ItemIdIsDead(itemid))
! {
! /* VACUUM is required to reclaim DEAD line pointers */
! prstate.all_visible = false;
continue;
+ }
/* Process this item or chain of items */
ndeleted += heap_prune_chain(relation, buffer, offnum,
*** heap_page_prune(Relation relation, Buffe
*** 245,250
--- 277,291
*/
PageClearFull(page);
+ /* Update the all-visible flag on the page */
+ if (!PageIsAllVisible(page) prstate.all_visible)
+ PageSetAllVisible(page);
+ else if (PageIsAllVisible(page) !prstate.all_visible)
+ {
+ elog(WARNING, PD_ALL_VISIBLE flag was incorrectly set);
+ PageClearAllVisible(page);
+ }
+
MarkBufferDirty(buffer);
/*
*** heap_page_prune(Relation relation, Buffe
*** 282,287
--- 323,341
PageClearFull(page);
SetBufferCommitInfoNeedsSave(buffer);
}
+
+ /* Update the all-visible flag on the page */
+ if (!PageIsAllVisible(page) prstate.all_visible)
+ {
+ PageSetAllVisible(page);
+ SetBufferCommitInfoNeedsSave(buffer);
+ }
+ else if (PageIsAllVisible(page) !prstate.all_visible)
+ {
+ elog(WARNING, PD_ALL_VISIBLE flag
Re: [HACKERS] visibility maps and heap_prune
On Sat, Dec 6, 2008 at 8:08 PM, Heikki Linnakangas [EMAIL PROTECTED] wrote: If you see a straightforward way, please submit a patch! Will do that. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] visibility maps and heap_prune
Pavan Deolasee wrote: ISTM that the PD_ALL_VISIBLE flag and the visibility map bit can be set at the end of pruning operation if we know that there are only tuples visible to all transactions left in the page. Right. The way pruning is done, I think it would be straight forward to get this information. Is it? I thought about that a bit while writing the patch, but didn't see any obvious way to do it. Except by adding a loop through all tuples on the page, but that's extra overhead. I think we're looping through all tuples in the pruning, but it's not quite obvious. If you see a straightforward way, please submit a patch! -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] visibility maps and heap_prune
ISTM that the PD_ALL_VISIBLE flag and the visibility map bit can be set at the end of pruning operation if we know that there are only tuples visible to all transactions left in the page. The way pruning is done, I think it would be straight forward to get this information. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
