Re: [HACKERS] opportunistic tuple freezing
On Tue, 2010-02-23 at 17:49 -0500, Bruce Momjian wrote: > I assume no progress has been made on testing the performance of this > patch. > That's correct. As of right now, the potential benefits of the patch do not seem to justify the performance testing effort. Others are welcome to try, of course. Regards, Jeff Davis -- 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] opportunistic tuple freezing
I assume no progress has been made on testing the performance of this patch. --- Jeff Davis wrote: > Attached is a patch to implement the idea discussed here: > > http://archives.postgresql.org/pgsql-hackers/2009-08/msg01137.php > > If VACUUM freezes one tuple on a page, it's likely that there are others > on the same page that are close to vacuum_freeze_min_age, but not quite. > Because the page is already dirty from freezing one tuple, it makes > sense to be more aggressive about freezing the rest, in the hope that > all the tuples will be frozen, and we will not have to dirty the page > again later. > > This patch introduces a GUC vacuum_freeze_opportunistic_ratio. If one > tuple on a page is frozen by vacuum, it effectively multiplies > vacuum_freeze_min_age by vacuum_freeze_opportunistic_ratio and uses that > lower (more aggressive) value only for the current page. > > The reason we don't just freeze all the tuples we can (effectively > setting the vacuum_freeze_opportunistic_ratio to zero) is to preserve > transaction ID information for diagnosing problems. > > Regards, > Jeff Davis [ 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 http://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] opportunistic tuple freezing
On Thu, 2009-09-17 at 12:36 -0400, Robert Haas wrote: > Despite my recent screw-up in this department, it should really be the > patch author's responsibility to test the patch first. Then the > reviewing process can involve additional testing. So I would say this > should be moved to Returned With Feedback, and then it can be > resubmitted later with test results. Fine with me. I already suspected that this patch wouldn't make it to the September commitfest: http://archives.postgresql.org/pgsql-hackers/2009-09/msg00798.php Regards, Jeff Davis -- 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] opportunistic tuple freezing
On Thu, Sep 17, 2009 at 12:24 PM, Tom Lane wrote: > Jeff Janes writes: >> Does the community think that experimental performance testing is a >> must-have in order for this patch to be acceptable? > > Dunno about others, but I think so. It's complicating both the > implementation and the users-eye view of VACUUM, and I want more than > a hypothesis that we're going to get something useful out of that. > > If we can't test it in a reasonable time frame for this commitfest, > then we should move it to the queue for the next one. Despite my recent screw-up in this department, it should really be the patch author's responsibility to test the patch first. Then the reviewing process can involve additional testing. So I would say this should be moved to Returned With Feedback, and then it can be resubmitted later with test results. The problem with bumping things to the next CommitFest is that it then becomes the CommitFest management team's problem to sort out which patches were bumped but the necessary to-do items weren't completed, versus being the patch author's problem to let us know when they have completed the necessary to-do items. So I am in favor of a policy that things should only be moved to the next CommitFest when they have ALREADY satisfied the requirements for being reviewed during that CommitFest. ...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] opportunistic tuple freezing
Jeff Janes writes: > Does the community think that experimental performance testing is a > must-have in order for this patch to be acceptable? Dunno about others, but I think so. It's complicating both the implementation and the users-eye view of VACUUM, and I want more than a hypothesis that we're going to get something useful out of that. If we can't test it in a reasonable time frame for this commitfest, then we should move it to the queue for the next one. regards, tom lane -- 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] opportunistic tuple freezing
On Mon, Sep 14, 2009 at 2:07 AM, Jeff Davis wrote: > On Mon, 2009-08-17 at 10:22 -0400, Tom Lane wrote: > > As always with patches that are meant to improve performance, > > some experimental evidence would be a good thing. > > I haven't had time to performance test this patch yet, and it looks like > it will take a significant amount of effort to do so. I'm focusing on my > other work, so I don't know if this one is going to be in shape for the > September commitfest. > > If someone is interested in doing some performance testing for this > patch, let me know. I still think it has potential. > > Does the community think that experimental performance testing is a must-have in order for this patch to be acceptable? If so, it sounds like this should be marked as rejected or RwF, and no longer considered for this commit fest, and I should move on to a different patch. I'll do some work on benchmarking it, but since it takes hundreds of millions of transactions to make a plausible scenario, this will not be done any time soon. Also, I see that the number of frozen tuples is not logged. I'd like to add that to the info reported under Log_autovacuum_min_duration, both to help evaluate this patch and because it seems like something that should be reported. Cheers, Jeff Janes
Re: [HACKERS] opportunistic tuple freezing
On Tue, 2009-09-15 at 20:56 -0700, Jeff Janes wrote: > Under what kind of circumstances/workload to you think this patch is > most likely to show its full potential? I can try to test it out, but > would like some guidance. I am guessing it is when the anti-wrap > around vacuums come due, but that is such a rare event, it could both > be hard to test for and also be of limited real-world applicability. I would expect the benefit to come when tuples start to reach vacuum_freeze_min_age, and the vacuums start freezing them. Without the patch, I expect that, on average, vacuum will freeze the same page multiple times even if the tuples are all quite old during the first round of freezing. So this would really only be a problem in a long steady state with a high volume of transactions. The patch will hopefully reduce the write volume going on in the background. I expect the biggest benefit comes when the tuples on a given page may have been inserted/updated over a few million transactions. Under normal circumstances, it won't be a huge win, but it's not a huge patch, either ;) Regards, Jeff Davis -- 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] opportunistic tuple freezing
On Mon, Sep 14, 2009 at 2:07 AM, Jeff Davis wrote: > On Mon, 2009-08-17 at 10:22 -0400, Tom Lane wrote: > > As always with patches that are meant to improve performance, > > some experimental evidence would be a good thing. > > I haven't had time to performance test this patch yet, and it looks like > it will take a significant amount of effort to do so. I'm focusing on my > other work, so I don't know if this one is going to be in shape for the > September commitfest. > > If someone is interested in doing some performance testing for this > patch, let me know. I still think it has potential. > Under what kind of circumstances/workload to you think this patch is most likely to show its full potential? I can try to test it out, but would like some guidance. I am guessing it is when the anti-wrap around vacuums come due, but that is such a rare event, it could both be hard to test for and also be of limited real-world applicability. Cheers, Jeff (Janes)
Re: [HACKERS] opportunistic tuple freezing
On Mon, 2009-08-17 at 10:22 -0400, Tom Lane wrote: > As always with patches that are meant to improve performance, > some experimental evidence would be a good thing. I haven't had time to performance test this patch yet, and it looks like it will take a significant amount of effort to do so. I'm focusing on my other work, so I don't know if this one is going to be in shape for the September commitfest. If someone is interested in doing some performance testing for this patch, let me know. I still think it has potential. Regards, Jeff Davis -- 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] opportunistic tuple freezing
Peter Eisentraut writes: > The patch might make sense anyway, but I think it might not be such an > overwhelming winner in practice. As always with patches that are meant to improve performance, some experimental evidence would be a good thing. regards, tom lane -- 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] opportunistic tuple freezing
On Sun, 2009-08-16 at 18:32 -0700, Jeff Davis wrote: > If VACUUM freezes one tuple on a page, it's likely that there are others > on the same page that are close to vacuum_freeze_min_age, but not quite. > Because the page is already dirty from freezing one tuple, it makes > sense to be more aggressive about freezing the rest, in the hope that > all the tuples will be frozen, and we will not have to dirty the page > again later. In the old days, where all new tuples were put at the end, this would have made a lot of sense. But nowadays, with fillfacter, HOT, and so on, it's quite likely that all the stuff around an outdated tuple are newer versions of the same tuple or newer versions of other tuples close by. The patch might make sense anyway, but I think it might not be such an overwhelming winner in practice. -- 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] opportunistic tuple freezing
On Mon, 2009-08-17 at 03:43 +0100, Greg Stark wrote: > I thought Josh's idea to apply this opportunistic threshold if the > page is already dirty for any reason was a good idea. Is there a good way to tell if a buffer is dirty or not? I don't see an exported function to do that. I could check by looking at the flags directly, but I don't see that being done in any file that doesn't have the word "buf" in it, so I assume it's breaking an abstraction. > Ie, if some > other dml or hint bit was set since the page was loaded even if vacuum > doesn't find any tuples are freezable. For this patch, I think this optimization might be useful, but I don't see how it would be important. The primary optimization in my patch is that VACUUM is likely to freeze all of the xids on a page at once. Hint bits are likely to be set before the tuples are eligible for freezing (unless the new GUC is set close to zero), and assorted dml is likely to mean that there are still non-freezable xids on the page (meaning that we still need to write it again, anyway). Regards, Jeff Davis -- 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] opportunistic tuple freezing
On Mon, 2009-08-17 at 04:01 +0100, Greg Stark wrote: > If it's convenient to have a function to handle the scan then use it > for both scans. You could have a flag that indicates it should abort > after the first freeze. I think it would be simpler to have a return > value indicating the oldest transaction found and then just call it > again if that's old enough for the opportunistic threshold. Thanks for the suggestions. I think it will take some significant refactoring. It needs to work from both scan_heap and lazy_scan_heap, so I would need to make a loop that works for both of those cases. Additionally, the second scan needs to avoid all of the side effects (like double-counting), which might mean some ugly branching. For instance, the big switch statement is completely unnecessary during the second scan. I'll come up with a refactored version of the patch. Regards, Jeff Davis -- 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] opportunistic tuple freezing
Looking at the patch I didn't like that duplicated the page scan in a second function without refactoring out the first scan. I think this comes from the usual noncommiter-itus of trying to modify the existing code as little as possible. At least that's the problem I've had which led to things like this. Instead try to figure how you would have written it if you were writing it from scratch. If it's convenient to have a function to handle the scan then use it for both scans. You could have a flag that indicates it should abort after the first freeze. I think it would be simpler to have a return value indicating the oldest transaction found and then just call it again if that's old enough for the opportunistic threshold. -- 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] opportunistic tuple freezing
On Mon, Aug 17, 2009 at 2:32 AM, Jeff Davis wrote: > > This patch introduces a GUC vacuum_freeze_opportunistic_ratio. If one > tuple on a page is frozen by vacuum, it effectively multiplies > vacuum_freeze_min_age by vacuum_freeze_opportunistic_ratio and uses that > lower (more aggressive) value only for the current page. I thought Josh's idea to apply this opportunistic threshold if the page is already dirty for any reason was a good idea. Ie, if some other dml or hint bit was set since the page was loaded even if vacuum doesn't find any tuples are freezable. So basically I think the logic should be: normal-vacuum-processing if (page-is-clean) try-to-freeze(normal-threshold) if (page-is-dirty) try-to-freeze(opportunistic-threshold) Sure it's duplicated work but I don't think it will add up to much. The normal pass could remember the oldest xid found and we could skip the second pass if the oldest xid is still too young. -- greg http://mit.edu/~gsstark/resume.pdf -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] opportunistic tuple freezing
Attached is a patch to implement the idea discussed here:
http://archives.postgresql.org/pgsql-hackers/2009-08/msg01137.php
If VACUUM freezes one tuple on a page, it's likely that there are others
on the same page that are close to vacuum_freeze_min_age, but not quite.
Because the page is already dirty from freezing one tuple, it makes
sense to be more aggressive about freezing the rest, in the hope that
all the tuples will be frozen, and we will not have to dirty the page
again later.
This patch introduces a GUC vacuum_freeze_opportunistic_ratio. If one
tuple on a page is frozen by vacuum, it effectively multiplies
vacuum_freeze_min_age by vacuum_freeze_opportunistic_ratio and uses that
lower (more aggressive) value only for the current page.
The reason we don't just freeze all the tuples we can (effectively
setting the vacuum_freeze_opportunistic_ratio to zero) is to preserve
transaction ID information for diagnosing problems.
Regards,
Jeff Davis
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 2034fdc..1d71abf 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4060,6 +4060,27 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
+
+ vacuum_freeze_opportunistic_ratio (floating point)
+
+ vacuum_freeze_opportunistic_ratio configuration parameter
+
+
+
+ While VACUUM is scanning a table, if it replaces
+ some transaction IDs with FrozenXID on a page, it
+ is cheaper to do so for other transaction IDs on the same page
+ at the same time. This value, which must be between 0 and 1,
+ is multiplied by to
+ determine a lower (more aggressive) cutoff for use during this
+ opportunity. A lower setting may reduce writes for
+ rarely-updated data, while a higher setting will preserve
+ transaction ID information longer (which is important when
+ diagnosing problems). The default setting is 0.5.
+
+
+
+
bytea_output (enum)
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index a6ba2ec..4e86e97 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -771,6 +771,7 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex)
bool use_wal;
TransactionId OldestXmin;
TransactionId FreezeXid;
+ TransactionId OpportunisticFreezeXid;
RewriteState rwstate;
/*
@@ -808,7 +809,8 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex)
* plain VACUUM would.
*/
vacuum_set_xid_limits(-1, -1, OldHeap->rd_rel->relisshared,
- &OldestXmin, &FreezeXid, NULL);
+ &OldestXmin, &FreezeXid, &OpportunisticFreezeXid,
+ NULL);
/*
* FreezeXid will become the table's new relfrozenxid, and that mustn't go
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 03c5edc..854d184 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -62,6 +62,7 @@
* GUC parameters
*/
int vacuum_freeze_min_age;
+double vacuum_freeze_opportunistic_ratio;
int vacuum_freeze_table_age;
/*
@@ -208,6 +209,7 @@ static int elevel = -1;
static TransactionId OldestXmin;
static TransactionId FreezeLimit;
+static TransactionId OpportunisticFreezeLimit;
static BufferAccessStrategy vac_strategy;
@@ -596,10 +598,12 @@ vacuum_set_xid_limits(int freeze_min_age,
bool sharedRel,
TransactionId *oldestXmin,
TransactionId *freezeLimit,
+ TransactionId *opportunisticFreezeLimit,
TransactionId *freezeTableLimit)
{
int freezemin;
TransactionId limit;
+ TransactionId opportunistic_limit;
TransactionId safeLimit;
/*
@@ -634,6 +638,11 @@ vacuum_set_xid_limits(int freeze_min_age,
if (!TransactionIdIsNormal(limit))
limit = FirstNormalTransactionId;
+ opportunistic_limit = *oldestXmin - (freezemin *
+ vacuum_freeze_opportunistic_ratio);
+ if (!TransactionIdIsNormal(opportunistic_limit))
+ opportunistic_limit = FirstNormalTransactionId;
+
/*
* If oldestXmin is very far back (in practice, more than
* autovacuum_freeze_max_age / 2 XIDs old), complain and force a minimum
@@ -648,10 +657,11 @@ vacuum_set_xid_limits(int freeze_min_age,
ereport(WARNING,
(errmsg("oldest xmin is far in the past"),
errhint("Close open transactions soon to avoid wraparound problems.")));
- limit = *oldestXmin;
+ limit = opportunistic_limit = *oldestXmin;
}
*freezeLimit = limit;
+ *opportunisticFreezeLimit = opportunistic_limit;
if (freezeTableLimit != NULL)
{
@@ -1253,7 +1263,8 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
vacuum_set_xid_limits(vacstmt->freeze_min_age, vacstmt->freeze_table_age,
onerel->rd_rel->relisshared,
- &OldestXmin, &FreezeLimit, NULL);
+ &OldestXmin, &FreezeLimit, &OpportunisticFreezeLimit,
+ NULL);
/*
* Flush any previous async-commit transactions. This does not guarantee
@@ -1396,6 +1407,8
