Обсуждение: Status of 8.3 patches
Here is the current 8.3 patch status: http://developer.postgresql.org/index.php/Todo:PatchStatus As you can see we have two major patches remaining, tsearch2 and HOT. Tom is working on tsearch2 now and Paven just posted an updated HOT patch. (Only the GIT (group index tuples) patch didn't make it into 8.3.) There are a few minor patches left: o Automatic adjustment of bgwriter_lru_maxpages We show this as waiting for performance results. I am thinking we should hold this for 8.4. o Error correction for n_dead_tuples This shows as waiting on another patch. Again, I am thinking to keep it for 8.4. o Per function search_path Tom will complete this one. o Table function support Neil, where are you on this? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > o Automatic adjustment of bgwriter_lru_maxpages > > We show this as waiting for performance results. I am thinking we > should hold this for 8.4. Agreed. I spent close to a week trying different benchmarks and configurations and simple test cases on a test server and my laptop, and couldn't demonstrate bgwriter making a positive impact in any configuration I tried. The theory behind the patch is sound, but it looks like a lot more testing and analysis is needed. > o Error correction for n_dead_tuples > > This shows as waiting on another patch. Again, I am thinking to > keep it for 8.4. It was waiting on the "vacuum oldestxmin" patch, which didn't make it to 8.3. I don't care for the patch myself, but it was submitted well before feature freeze and deserves a review. It looks good to me at first glance. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Heikki Linnakangas wrote: > Bruce Momjian wrote: >> o Automatic adjustment of bgwriter_lru_maxpages >> >> We show this as waiting for performance results. I am thinking we >> should hold this for 8.4. > > Agreed. I spent close to a week trying different benchmarks and > configurations and simple test cases on a test server and my laptop, and > couldn't demonstrate bgwriter making a positive impact in any > configuration I tried. The theory behind the patch is sound, but it > looks like a lot more testing and analysis is needed. Wouldn't real world testing be needed to actually gain insight to this patch? Joshua D. Drake > >> o Error correction for n_dead_tuples >> >> This shows as waiting on another patch. Again, I am thinking to >> keep it for 8.4. > > It was waiting on the "vacuum oldestxmin" patch, which didn't make it to > 8.3. I don't care for the patch myself, but it was submitted well before > feature freeze and deserves a review. It looks good to me at first glance. > - -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240 PostgreSQL solutions since 1997 http://www.commandprompt.com/ UNIQUE NOT NULL Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL Replication: http://www.commandprompt.com/products/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFGyfZjATb/zqfZUUQRAltsAKClIbgh+r2ktW9MM8EO/rfp/TrsWACgq34w RdjYafRztMrnxztwlpRhWzQ= =Yfiq -----END PGP SIGNATURE-----
Heikki Linnakangas wrote: > Bruce Momjian wrote: > > o Error correction for n_dead_tuples > > > > This shows as waiting on another patch. Again, I am thinking to > > keep it for 8.4. > > It was waiting on the "vacuum oldestxmin" patch, which didn't make it to > 8.3. I don't care for the patch myself, but it was submitted well before > feature freeze and deserves a review. It looks good to me at first glance. My opinion as well. -- Alvaro Herrera http://www.advogato.org/person/alvherre "La fuerza no está en los medios físicos sino que reside en una voluntad indomable" (Gandhi)
Joshua D. Drake wrote: > Heikki Linnakangas wrote: >> Bruce Momjian wrote: >>> o Automatic adjustment of bgwriter_lru_maxpages >>> >>> We show this as waiting for performance results. I am thinking we >>> should hold this for 8.4. >> Agreed. I spent close to a week trying different benchmarks and >> configurations and simple test cases on a test server and my laptop, and >> couldn't demonstrate bgwriter making a positive impact in any >> configuration I tried. The theory behind the patch is sound, but it >> looks like a lot more testing and analysis is needed. > > Wouldn't real world testing be needed to actually gain insight to this > patch? I would expect a fairly static benchmark workload to benefit from having a bgwriter, more so than more unpredictable real world applications. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Heikki Linnakangas wrote: > Joshua D. Drake wrote: >> Heikki Linnakangas wrote: >>> Bruce Momjian wrote: >>>> o Automatic adjustment of bgwriter_lru_maxpages >>>> >>>> We show this as waiting for performance results. I am thinking we >>>> should hold this for 8.4. >>> Agreed. I spent close to a week trying different benchmarks and >>> configurations and simple test cases on a test server and my laptop, and >>> couldn't demonstrate bgwriter making a positive impact in any >>> configuration I tried. The theory behind the patch is sound, but it >>> looks like a lot more testing and analysis is needed. >> Wouldn't real world testing be needed to actually gain insight to this >> patch? > > I would expect a fairly static benchmark workload to benefit from having > a bgwriter, more so than more unpredictable real world applications. Hmmm, I find that real world applications are quite predictable over time. Certainly you have spikes (good pr, whatever) but in general with a little bit of monitoring it is quite possible to evaluate a generally expected result. I guess my point is, if the patch looks good and does not appear to hurt anything, why not apply it? At least that way we can start to review the progress of the feature itself as it starts to see use. Sincerely, Joshua D. Drake - -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240 PostgreSQL solutions since 1997 http://www.commandprompt.com/ UNIQUE NOT NULL Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL Replication: http://www.commandprompt.com/products/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFGyftwATb/zqfZUUQRAkwjAJ9xGlzYci6dT3jJy5PrPMYxZOmt0ACffeUm Br+UQp+k4XVjpyQDSIba4hk= =jDki -----END PGP SIGNATURE-----
Joshua D. Drake wrote: > Heikki Linnakangas wrote: >> Joshua D. Drake wrote: >>> Heikki Linnakangas wrote: >>>> Bruce Momjian wrote: >>>>> o Automatic adjustment of bgwriter_lru_maxpages >>>>> >>>>> We show this as waiting for performance results. I am thinking we >>>>> should hold this for 8.4. >>>> Agreed. I spent close to a week trying different benchmarks and >>>> configurations and simple test cases on a test server and my laptop, and >>>> couldn't demonstrate bgwriter making a positive impact in any >>>> configuration I tried. The theory behind the patch is sound, but it >>>> looks like a lot more testing and analysis is needed. >>> Wouldn't real world testing be needed to actually gain insight to this >>> patch? >> I would expect a fairly static benchmark workload to benefit from having >> a bgwriter, more so than more unpredictable real world applications. > > Hmmm, I find that real world applications are quite predictable over > time. Certainly you have spikes (good pr, whatever) but in general with > a little bit of monitoring it is quite possible to evaluate a generally > expected result. > > I guess my point is, if the patch looks good and does not appear to hurt > anything, why not apply it? At least that way we can start to review the > progress of the feature itself as it starts to see use. If it doesn't appear to have any positive effect, why would we apply it? If we apply the patch, how would you monitor it's effectiveness in a live database? There's nothing to compare against. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Joshua D. Drake wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Heikki Linnakangas wrote: > > Joshua D. Drake wrote: > >> Heikki Linnakangas wrote: > >>> Bruce Momjian wrote: > >>>> o Automatic adjustment of bgwriter_lru_maxpages > >>>> > >>>> We show this as waiting for performance results. I am thinking we > >>>> should hold this for 8.4. > >>> Agreed. I spent close to a week trying different benchmarks and > >>> configurations and simple test cases on a test server and my laptop, and > >>> couldn't demonstrate bgwriter making a positive impact in any > >>> configuration I tried. The theory behind the patch is sound, but it > >>> looks like a lot more testing and analysis is needed. > >> Wouldn't real world testing be needed to actually gain insight to this > >> patch? > > > > I would expect a fairly static benchmark workload to benefit from having > > a bgwriter, more so than more unpredictable real world applications. > > Hmmm, I find that real world applications are quite predictable over > time. Certainly you have spikes (good pr, whatever) but in general with > a little bit of monitoring it is quite possible to evaluate a generally > expected result. > > I guess my point is, if the patch looks good and does not appear to hurt > anything, why not apply it? At least that way we can start to review the > progress of the feature itself as it starts to see use. Yeah, you mean like commit_delay. It really worked great, that reviewing of a feature, you know. It only took 3 years until someone realized that it didn't work as advertised. -- Alvaro Herrera http://www.amazon.com/gp/registry/5ZYLFMCVHXC "En las profundidades de nuestro inconsciente hay una obsesiva necesidad de un universo lógico y coherente. Pero el universo real se halla siempre un paso más allá de la lógica" (Irulan)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Alvaro Herrera wrote: > Joshua D. Drake wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> Heikki Linnakangas wrote: >>> Joshua D. Drake wrote: >> I guess my point is, if the patch looks good and does not appear to hurt >> anything, why not apply it? At least that way we can start to review the >> progress of the feature itself as it starts to see use. > > Yeah, you mean like commit_delay. It really worked great, that > reviewing of a feature, you know. It only took 3 years until someone > realized that it didn't work as advertised. You can not compare the relevant smallness of use from three years ago to the explosion of use at present. It is certainly unfortunate that commit_delay didn't work as advertised, but then again had we not applied it, we would have never known in the first place and now we have the opportunity to fix or remove it. You can compare other such features that many people don't touch that are starting to show promise over time such as cpu_tuple_cost. Sincerely, Joshua D. Drake - -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240 PostgreSQL solutions since 1997 http://www.commandprompt.com/ UNIQUE NOT NULL Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL Replication: http://www.commandprompt.com/products/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFGyf4bATb/zqfZUUQRAnPOAKCP8wLnXIJ5B7cYdHGBudZC+bALrACeL6HI 6ZYXLsKD/MSgdnqVkGK/THY= =HqYq -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Heikki Linnakangas wrote: > Joshua D. Drake wrote: >> Heikki Linnakangas wrote: >>> Joshua D. Drake wrote: >>>> Heikki Linnakangas wrote: >>>>> Bruce Momjian wrote: >>>>>> o Automatic adjustment of bgwriter_lru_maxpages \ >>> I would expect a fairly static benchmark workload to benefit from having >>> a bgwriter, more so than more unpredictable real world applications. >> Hmmm, I find that real world applications are quite predictable over >> time. Certainly you have spikes (good pr, whatever) but in general with >> a little bit of monitoring it is quite possible to evaluate a generally >> expected result. >> >> I guess my point is, if the patch looks good and does not appear to hurt >> anything, why not apply it? At least that way we can start to review the >> progress of the feature itself as it starts to see use. > > If it doesn't appear to have any positive effect, why would we apply it? We don't know if it has a positive effect. My understanding is that your testing shows that it does not appear to have a negative effect. Those are different things don't you think? > > If we apply the patch, how would you monitor it's effectiveness in a > live database? There's nothing to compare against. > Well that's valid. A production database you really don't want to be fiddling. *shrug* I hate to see potential lost, but if it isn't a good fit.... Sincerely, Joshua D. Drake - -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240 PostgreSQL solutions since 1997 http://www.commandprompt.com/ UNIQUE NOT NULL Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate PostgreSQL Replication: http://www.commandprompt.com/products/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFGyf6oATb/zqfZUUQRAjLrAJ9lknJRIoAe7EsFWlD3PQeXPZXjMgCdE2ze bGNC5JpSE2DMQuKWrOf9fqI= =rL19 -----END PGP SIGNATURE-----
Joshua D. Drake wrote: > > I guess my point is, if the patch looks good and does not appear to hurt > anything, why not apply it? At least that way we can start to review the > progress of the feature itself as it starts to see use. > > I don't think that's a very good criterion. We need to have good evidence that the change has positive benefit. We don't want to be doing speculative changes. cheers andrew
Joshua D. Drake wrote: > Alvaro Herrera wrote: >> Joshua D. Drake wrote: >>> Heikki Linnakangas wrote: >>>> Joshua D. Drake wrote: > >>> I guess my point is, if the patch looks good and does not appear to hurt >>> anything, why not apply it? At least that way we can start to review the >>> progress of the feature itself as it starts to see use. >> Yeah, you mean like commit_delay. It really worked great, that >> reviewing of a feature, you know. It only took 3 years until someone >> realized that it didn't work as advertised. > > You can not compare the relevant smallness of use from three years ago > to the explosion of use at present. It is certainly unfortunate that > commit_delay didn't work as advertised, but then again had we not > applied it, we would have never known in the first place and now we have > the opportunity to fix or remove it. I don't think we can work like that anymore. For a performance patch, you ought to have at least a one repeatable test case where the patch improves performance. Then you can start talking about tradeoffs with code complexity, possible performance losses in other less common use cases etc, but if you can't demonstrate any meaningful benefit whatsoever in any use case, it's dead on arrival. > You can compare other such features that many people don't touch that > are starting to show promise over time such as cpu_tuple_cost. That's different. Even though choosing the right plan surely has an effect on performance, cpu_tuple_cost more like a functional feature than a performance feature.. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki@enterprisedb.com> wrote: > Bruce Momjian wrote: > > o Automatic adjustment of bgwriter_lru_maxpages > > > > We show this as waiting for performance results. I am thinking we > > should hold this for 8.4. > > Agreed. I spent close to a week trying different benchmarks and > configurations and simple test cases on a test server and my laptop, and > couldn't demonstrate bgwriter making a positive impact in any > configuration I tried. The theory behind the patch is sound, but it > looks like a lot more testing and analysis is needed. Agreed, too. However, I don't think it is a performance feature practically; it is just for an advertisement: "We will be freed from the tuning of bgwriter in 8.3!" Does anyone have a way to measure the performance difference by bgwriter_lru_xxx ? I have no performance results not only of the patch but also of those parameters. I'd like to use those test cases to compare manual and automatic tunings of lru parameters for 8.4. > > o Error correction for n_dead_tuples > > > > This shows as waiting on another patch. Again, I am thinking to > > keep it for 8.4. > > It was waiting on the "vacuum oldestxmin" patch, which didn't make it to > 8.3. I don't care for the patch myself, but it was submitted well before > feature freeze and deserves a review. It looks good to me at first glance. I think there is no stopper to the patch, too. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
Heikki Linnakangas <heikki@enterprisedb.com> writes: > Bruce Momjian wrote: >> o Error correction for n_dead_tuples >> >> This shows as waiting on another patch. Again, I am thinking to >> keep it for 8.4. > It was waiting on the "vacuum oldestxmin" patch, which didn't make it to > 8.3. I don't care for the patch myself, but it was submitted well before > feature freeze and deserves a review. It looks good to me at first glance. This patch was originally submitted before we realized that pg_stats failed to distinguish the effects of committed vs rolled-back transactions (which was fixed about three months ago); and we also recently fixed several other bugs such as losing stats data for shared catalogs. So there's a significant probability that the errors it was trying to compensate for are already fixed. Also, I'm still quite unhappy that the patch converts the tracking of n_dead_tuples into a dead-reckoning system in which incremental changes are continually applied without any feedback that'd prevent the value from diverging arbitrarily far from reality. Murphy's law says that the value *will* diverge, if you don't have any negative feedback in the loop to force it to track reality. There may be something to be done here, but there's not any evidence at hand that CVS HEAD actually suffers from a problem in tracking n_dead_tuples, and even if it does I do not think that this particular patch is a good fix. regards, tom lane
On Tue, 21 Aug 2007, ITAGAKI Takahiro wrote: > Does anyone have a way to measure the performance difference by > bgwriter_lru_xxx ? I have no performance results not only of the patch > but also of those parameters. I'd like to use those test cases to compare > manual and automatic tunings of lru parameters for 8.4. The version of this patch I submitted at http://archives.postgresql.org/pgsql-patches/2007-05/msg00142.php puts statistics into the pg_stat_bgwriter structure so you can compare how much work is being done by the background writer vs. the backends. Even if there is no explicit change in the BW behavior, I would very much like to see that part get committed so people can actually tune more easily by hand using the stock PG in 8.3. Heikki didn't really like the way I passed that data around internally, but I never got a suggestion for doing it a better way I thought was an improvement. Heikki's "Bgwriter strategies" thread used that to compare the various approaches, which I did quite a bit of as well but didn't bother publishing the results as they weren't any more interesting than his: http://archives.postgresql.org/pgsql-hackers/2007-07/msg00144.php I have a set of scripts to automate pgbench to collect this data automatically and compare various settings. Now that I see all the big patches are settling down, I'll collect all that up and pass it along. I've been mulling over options here for a month now, and I'm not done with this patch yet; I'll take Bruce's message as a call to urgent action to finish and submit my final results ASAP. -- * Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> o Error correction for n_dead_tuples > Also, I'm still quite unhappy that the patch converts the tracking of > n_dead_tuples into a dead-reckoning system in which incremental changes > are continually applied without any feedback that'd prevent the value > from diverging arbitrarily far from reality. Murphy's law says that > the value *will* diverge, if you don't have any negative feedback > in the loop to force it to track reality. There is *no feedback loop* in the patch. It will clear the stats at the beginning of vacuum, and leave n_dead_tuples collected during the vacuum. Even if some errors are left after the vacuum, they will be cleared at the next vacuum. Errors should not be accumulated through repeated vacuums. > There may be something to be done here, but there's not any evidence > at hand that CVS HEAD actually suffers from a problem in tracking > n_dead_tuples, and even if it does I do not think that this particular > patch is a good fix. The problem is in the cost-based delayed vacuum. We turned cost-delay on as default and will encourage to use autovacuum at 8.3. Dead tuple ratio is not predictable from autovacuum_vacuum_scale_factor in the current behavior; It might make DBA feel unhappy. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
Tom Lane wrote: > This patch was originally submitted before we realized that pg_stats > failed to distinguish the effects of committed vs rolled-back > transactions (which was fixed about three months ago); and we also > recently fixed several other bugs such as losing stats data for shared > catalogs. So there's a significant probability that the errors it was > trying to compensate for are already fixed. Quite possible. I don't recall a real world example or a test case preceding the patch. I guess the problem scenario would be a table with a lot of update/delete activity, and very unaggressive cost_delay. > Also, I'm still quite unhappy that the patch converts the tracking of > n_dead_tuples into a dead-reckoning system in which incremental changes > are continually applied without any feedback that'd prevent the value > from diverging arbitrarily far from reality. Murphy's law says that > the value *will* diverge, if you don't have any negative feedback > in the loop to force it to track reality. I believe the latest version doesn't have that problem. At the beginning of vacuum, n_dead_tuples is saved, and at the end of vacuum n_dead_tuples is decremented by the value it had at the beginning. At the end n_dead_tuples will be equal to the number of new dead tuples generated during the vacuum, no matter how out of whack it was in the beginning. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
jd@commandprompt.com ("Joshua D. Drake") writes: > Alvaro Herrera wrote: >> Joshua D. Drake wrote: >>> -----BEGIN PGP SIGNED MESSAGE----- >>> Hash: SHA1 >>> >>> Heikki Linnakangas wrote: >>>> Joshua D. Drake wrote: > >>> I guess my point is, if the patch looks good and does not appear >>> to hurt anything, why not apply it? At least that way we can start >>> to review the progress of the feature itself as it starts to see >>> use. >> >> Yeah, you mean like commit_delay. It really worked great, that >> reviewing of a feature, you know. It only took 3 years until >> someone realized that it didn't work as advertised. > > You can not compare the relevant smallness of use from three years > ago to the explosion of use at present. It is certainly unfortunate > that commit_delay didn't work as advertised, but then again had we > not applied it, we would have never known in the first place and now > we have the opportunity to fix or remove it. > > You can compare other such features that many people don't touch > that are starting to show promise over time such as cpu_tuple_cost. I'd like to see a bit more available for the TOAST threshold, even if only a little bit of documentation to make it an unscary thing to modify the threshold so that I could recommend that our DBAs modify it, and *not* have them get all hinky at the fact that I'm telling them to touch the sources. "Yes, I'm telling you to modify the sources. It's a safe modification - see? The documentation says so!" Without this, I'll have to fight some ghastly uphill battle in order to get this set in our builds, and I *don't* want to wait until 8.4 to see our ~900 byte long XML values TOASTed. I'd be happy to help make the patch. -- (reverse (concatenate 'string "ofni.secnanifxunil" "@" "enworbbc")) http://cbbrowne.com/info/linux.html :FATAL ERROR -- VECTOR OUT OF HILBERT SPACE