Re: [HACKERS] Mini improvement: statement_cost_limit
Josh Berkus wrote: Tom, Wasn't this exact proposal discussed and rejected awhile back? We rejected Greenplum's much more invasive resource manager, because it created a large performance penalty on small queries whether or not it was turned on. However, I don't remember any rejection of an idea as simple as a cost limit rejection. This would, IMHO, be very useful for production instances of PostgreSQL. The penalty for mis-rejection of a poorly costed query is much lower than the penalty for having a bad query eat all your CPU. Greenplum's introduced a way to creating a cost "threshold" a bit like the way Simon was going to do "shared" work_mem. It did 2 things: 1/ Counted the cost of an about-to-be run query against the threshold, and made the query wait if it would exhaust it 2/ Aborted the query if its cost was greater than the threshold Initially there was quite a noticeable performance penalty with it enabled - but as the guy working on it (me) redid bits and pieces then penalty decreased massively. Note that in all cases, disabling the feature meant there was no penalty. The latest variant of the code is around in the Bizgres repository (src/backend/utils/resscheduler I think) - some bits might be worth looking at! Best wishes Mark P.s : I'm not working for Greenplum now. -- 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] small bug in hlCover
On Mon, 2008-08-04 at 00:36 -0300, Euler Taveira de Oliveira wrote: > Sushant Sinha escreveu: > > I think there is a slight bug in hlCover function in wparser_def.c > > > The bug is not in the hlCover. In prsd_headline, if we didn't find a > suitable bestlen (i.e. >= 0), than it includes up to document length or > *maxWords* (here is the bug). I'm attaching a small patch that fixes it > and some comment typos. Please apply it to 8_3_STABLE too. Well hlCover purpose is to find a cover and for the document '1 2 3 4 5 6 7 8 9 10' and the query '1'::tsquery, a cover exists. So it should point it out. On my source I see that prsd_headline marks only min_words which seems like the right thing to do. -Sushant. > > euler=# select ts_headline('1 2 3 4 5 6 7 8 9 10','1'::tsquery, > 'MinWords=5'); > ts_headline > - > 1 2 3 4 5 6 7 8 9 10 > (1 registro) > > euler=# select ts_headline('1 2 3 4 5 6 7 8 9 10','1'::tsquery); > ts_headline > - > 1 2 3 4 5 6 7 8 9 10 > (1 registro) > > -- 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] Instructions for adding new catalog
"Gustavo Tonini" <[EMAIL PROTECTED]> writes: > Well, just for documenting the process... > Adding new postgres catalog in 2 little steps: > 1)Write catalog header file and save it into "src/include/catalog" > directory. Hint: copy format from other catalog headers. > 2)Add your header file name to variable POSTGRES_BKI_SRCS in file > "src/backend/catalog/Makefile" As long as we're documenting ... * Documenting a new catalog in catalogs.sgml is not considered optional. * Most likely, your catalog needs some indexes, which should be declared in catalog/indexing.h. * Of course, the reason you are defining a new system catalog is that the C code is going to know about it. So there is a whole lot of other stuff you probably need to write: object insertion code, object deletion code, lookup code (maybe a new syscache or two). Traditionally the lowest-level insertion/deletion code goes into catalog/pg_yourcatalog.c, while slightly higher-level code such as the implementation of new utility commands to manage this new kind of object will go elsewhere. That's before you get to any actual new functionality... 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] Mini improvement: statement_cost_limit
On Sunday 03 August 2008 15:12:22 Simon Riggs wrote: > On Sun, 2008-08-03 at 00:44 -0700, daveg wrote: > > On Sat, Aug 02, 2008 at 09:30:08PM +0200, Hans-Jürgen Schönig wrote: > > > On Aug 2, 2008, at 8:38 PM, Tom Lane wrote: > > > >Andrew Dunstan <[EMAIL PROTECTED]> writes: > > > >>Hans-Jürgen Schönig wrote: > > > >>>i introduced a GUC called statement_cost_limit which can be used to > > > >>>error out if a statement is expected to be too expensive. > > > >> > > > >>You clearly have far more faith in the cost estimates than I do. > > > > > > > >Wasn't this exact proposal discussed and rejected awhile back? > > > > > > i don't remember precisely. > > > i have seen it on simon's wiki page and it is something which would > > > have been useful in some cases in the past. > > I still support it. Regrettably, many SQL developers introduce product > joins and other unintentional errors. Why let problem queries through? > Security-wise they're great Denial of Service attacks, bringing the > server to its knees better than most ways I know, in conjunction with a > nice hefty work_mem setting. 27 table product joins: memory, CPU, I/O > and diskspace resources used all in a simple killer query. > ISTR that what ended up killing the enthusiasm for this was that most people realized that this GUC was just a poor tool to take a stab at solving other problems (ie. rate limiting cpu for queries). > If anybody thinks costs are inaccurate, don't use it. Or better still > improve the cost models. It isn't any harder or easier to find a useful > value than it is to use statement_timeout. What's the difference between > picking an arbitrary time and an arbitrary cost? You need to alter the > value according to people's complaints in both cases. > I think the original argument for statement_timeout was that long running queries were known to cause have wrt vacuum strategies (remember, that one has been in the back end a long time). ISTR some recent threds on -hackers questioning whether statement_timeout should be eliminated itself. > > I think a variation on this could be very useful in development and test > > environments. Suppose it raised a warning or notice if the cost was over > > the limit. Then one could set a limit of a few million on the development > > and test servers and developers would at least have a clue that they > > needed to look at explain for that query. As it is now, one can exhort > > them to run explain, but it has no effect. Instead we later see queries > > killed by a 24 hour timeout with estimated costs ranging from "until they > > unplug the machine and dump it" to "until the sun turns into a red > > giant". > > Great argument. So that's 4 in favour at least. > Not such a great argument. Cost models on development servers can and often are quite different from those on production, so you might be putting an artifical limit on top of your developers. > A compromise would be to have log_min_statement_cost (or > warn_min_statement_cost) which will at least help find these problems in > testing before we put things live, but that still won't help with > production issues. > > Another alternative would be to have a plugin that can examine the plan > immediately after planner executes, so you can implement this yourself, > plus some other possibilities. > I still think it is worth revisiting what problems people are trying to solve, and see if there are better tools they can be given to solve them. Barring that, I suppose a crude solution is better than nothing, though I fear people might point at the crude solution as a good enough solution to justify not working on better solutions. -- Robert Treat Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL -- 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] Instructions for adding new catalog
Thanks for reply, Tom. Well, just for documenting the process... Adding new postgres catalog in 2 little steps: 1)Write catalog header file and save it into "src/include/catalog" directory. Hint: copy format from other catalog headers. 2)Add your header file name to variable POSTGRES_BKI_SRCS in file "src/backend/catalog/Makefile" "Make", "make install", then "initdb" will create new system catalogs for you. Gustavo. On Fri, Aug 1, 2008 at 11:28 AM, Tom Lane <[EMAIL PROTECTED]> wrote: > "Gustavo Tonini" <[EMAIL PROTECTED]> writes: >> I read the archives, but I can't find the instructions for adding new >> catalogs to the system. > > That's probably because there aren't any. Look at recent patches > that added a new catalog to get an idea of what you need to do. > The enum patch might be a good choice. > >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] Mini improvement: statement_cost_limit
Tom, > Wasn't this exact proposal discussed and rejected awhile back? We rejected Greenplum's much more invasive resource manager, because it created a large performance penalty on small queries whether or not it was turned on. However, I don't remember any rejection of an idea as simple as a cost limit rejection. This would, IMHO, be very useful for production instances of PostgreSQL. The penalty for mis-rejection of a poorly costed query is much lower than the penalty for having a bad query eat all your CPU. -- --Josh Josh Berkus PostgreSQL San Francisco -- 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] unnecessary code in_bt_split
Zdenek Kotala <[EMAIL PROTECTED]> writes: > I found that _bt_split function calls PageGetTempPage, but next call is > _bt_page_init which clear all contents anyway. Is there any reason to call > PageGetTempPage instead of palloc? Not violating a perfectly good abstraction? I agree that PageGetTempPage isn't amazingly efficient, but internal refactoring would halve its cost; and if you have some evidence that there's a real performance issue then we could think about adjusting the temp-page API to allow _bt_pageinit to be combined with it. But I have a real problem with hacking up _bt_split so that it will call PageRestoreTempPage on something it didn't get from PageGetTempPage. Considering the WAL and regular I/O that will be induced by a split, I kinda doubt this is even worth worrying about anyway... 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
[HACKERS] unnecessary code in_bt_split
I found that _bt_split function calls PageGetTempPage, but next call is _bt_page_init which clear all contents anyway. Is there any reason to call PageGetTempPage instead of palloc? Original code: 00796 leftpage = PageGetTempPage(origpage, sizeof(BTPageOpaqueData)); 00797 rightpage = BufferGetPage(rbuf); 00798 00799 _bt_pageinit(leftpage, BufferGetPageSize(buf)); Suggested code: 00796 leftpage = palloc(PageGetSize(origpage)); 00797 rightpage = BufferGetPage(rbuf); 00798 00799 _bt_pageinit(leftpage, BufferGetPageSize(buf)); Any idea? thanks Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- 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] Mini improvement: statement_cost_limit
On Sun, 2008-08-03 at 22:09 +0200, Hans-Jürgen Schönig wrote: > > Another alternative would be to have a plugin that can examine the > > plan > > immediately after planner executes, so you can implement this > > yourself, > > plus some other possibilities. > > > this would be really fancy. > how could a plugin like that look like? Hmm...thinks: exactly like the existing planner_hook(). So, rewrite this as a planner hook and submit as a contrib module. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] patch: Add a separate TRUNCATE permission
On Tue, 2008-07-29 at 09:03 -0400, Stephen Frost wrote: > * Peter Eisentraut ([EMAIL PROTECTED]) wrote: > > Am Tuesday, 29. July 2008 schrieb Stephen Frost: > > > I'd certainly like to see a truncate permission, I wrote a patch for it > > > myself back in January of 2006. That thread starts here: > > > > > > http://archives.postgresql.org/pgsql-patches/2006-01/msg00028.php > > > > > > I think someone else submitted a patch for it last year too, actually. It was raised in January and rejected again then. Glad to see it raised again here. I believe Tom's previous concerns about allowing truncate permissions were related to the potentially increased number of truncate commands this would cause and the need to tune invalidation messages. That's done now. > > Well, that certainly indicates some demand. > > > > I think we should worry about adding more bits when we need them. It's > > certainly possible to add more bits, so it is not like we need to save the > > ones we have forever. > > I would agree with this. Others? I've no problem with running out of bits. At this rate, we have enough some for some years yet. But I don't see too many additional permissions coming along anyhow. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Mini improvement: statement_cost_limit
hello ... I still support it. Regrettably, many SQL developers introduce product joins and other unintentional errors. Why let problem queries through? i think the killer is that we don't have to wait until the query dies with a statement_timeout. it is ways more elegant to kill things before they have already eaten too many cycles. one thing which is important as well: statement_cost_limit does not kill queries which have just been waiting for a lock. this makes things slightly more predictable. Security-wise they're great Denial of Service attacks, bringing the server to its knees better than most ways I know, in conjunction with a nice hefty work_mem setting. 27 table product joins: memory, CPU, I/O and diskspace resources used all in a simple killer query. i am not too concerned about DNS, i have to admit. i would rather see it as a way to make developers do better things. If anybody thinks costs are inaccurate, don't use it. Or better still improve the cost models. It isn't any harder or easier to find a useful value than it is to use statement_timeout. What's the difference between picking an arbitrary time and an arbitrary cost? You need to alter the value according to people's complaints in both cases. the cost model is good enough to see if something is good or bad. this is basically all we want to do here --- killing all evil. *snip* A compromise would be to have log_min_statement_cost (or warn_min_statement_cost) which will at least help find these problems in testing before we put things live, but that still won't help with production issues. definitely. a good idea as well - but people will hardly read it, i guess :(. Another alternative would be to have a plugin that can examine the plan immediately after planner executes, so you can implement this yourself, plus some other possibilities. this would be really fancy. how could a plugin like that look like? hans -- Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt Web: www.postgresql-support.de -- 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] small bug in hlCover
Has any one noticed this? -Sushant. On Wed, 2008-07-16 at 23:01 -0400, Sushant Sinha wrote: > I think there is a slight bug in hlCover function in wparser_def.c > > If there is only one query item and that is the first word in the text, > then hlCover does not returns any cover. This is evident in this example > when ts_headline only generates the min_words: > > testdb=# select ts_headline('1 2 3 4 5 6 7 8 9 10','1'::tsquery, > 'MinWords=5'); >ts_headline > -- > 1 2 3 4 5 > (1 row) > > The problem is that *q is initialized to 0 which is a legitimate value > for a cover. So I have attached a patch that fixes it and after applying > the patch here is the result. > > testdb=# select ts_headline('1 2 3 4 5 6 7 8 9 10','1'::tsquery, > 'MinWords=5'); > ts_headline > - > 1 2 3 4 5 6 7 8 9 10 > (1 row) > > -Sushant. -- 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] Mini improvement: statement_cost_limit
On Sun, 2008-08-03 at 00:44 -0700, daveg wrote: > On Sat, Aug 02, 2008 at 09:30:08PM +0200, Hans-Jürgen Schönig wrote: > > On Aug 2, 2008, at 8:38 PM, Tom Lane wrote: > > > > >Andrew Dunstan <[EMAIL PROTECTED]> writes: > > >>Hans-Jürgen Schönig wrote: > > >>>i introduced a GUC called statement_cost_limit which can be used to > > >>>error out if a statement is expected to be too expensive. > > > > > >>You clearly have far more faith in the cost estimates than I do. > > > > > >Wasn't this exact proposal discussed and rejected awhile back? > > > > > i don't remember precisely. > > i have seen it on simon's wiki page and it is something which would > > have been useful in some cases in the past. I still support it. Regrettably, many SQL developers introduce product joins and other unintentional errors. Why let problem queries through? Security-wise they're great Denial of Service attacks, bringing the server to its knees better than most ways I know, in conjunction with a nice hefty work_mem setting. 27 table product joins: memory, CPU, I/O and diskspace resources used all in a simple killer query. If anybody thinks costs are inaccurate, don't use it. Or better still improve the cost models. It isn't any harder or easier to find a useful value than it is to use statement_timeout. What's the difference between picking an arbitrary time and an arbitrary cost? You need to alter the value according to people's complaints in both cases. > I think a variation on this could be very useful in development and test > environments. Suppose it raised a warning or notice if the cost was over > the limit. Then one could set a limit of a few million on the development > and test servers and developers would at least have a clue that they needed > to look at explain for that query. As it is now, one can exhort them to > run explain, but it has no effect. Instead we later see queries killed > by a 24 hour timeout with estimated costs ranging from "until they unplug > the machine and dump it" to "until the sun turns into a red giant". Great argument. So that's 4 in favour at least. A compromise would be to have log_min_statement_cost (or warn_min_statement_cost) which will at least help find these problems in testing before we put things live, but that still won't help with production issues. Another alternative would be to have a plugin that can examine the plan immediately after planner executes, so you can implement this yourself, plus some other possibilities. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Parsing of pg_hba.conf and authentication inconsistencies
* Magnus Hagander ([EMAIL PROTECTED]) wrote: > Stephen Frost wrote: > > A little extra code in the backend is well worth fixing this foot-gun. > > The concerns raised so far have been "who will write it?" and "what if > > it has bugs?". Neither of these are particularly compelling arguments > > when you've already offered to write and bug-test it (right, Magnus? :). > > Toms main argument has been that it would move the code *from* the > backend and into the *postmaster*, which is much more sensitive. erm, yes, sorry I wasn't being clear. Brain moving faster than fingers sometimes. :) > And yes, I've offered to write the code. I take this as an offer from > you to bug-test it :-) Indeed, as long as I'm bug-testing the Kerberos mappings at the same time... ;) Seriously though, I'd be happy to review and test the code. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies
* Magnus Hagander ([EMAIL PROTECTED]) wrote: > I think it'd be reasonable to refuse starting if the config is *known > broken* (such as containing lines that are unparseable, or that contain > completely invalid tokens), whereas you'd start if they just contain > things that are "probably wrong". But picking from your previous > examples of "more advanced checks", there are lots of cases where > things like overlapping CIDR address ranges are perfectly valid, so I > don't think we could even throw a warning for that - unless there's a > separate flag to enable/disable warnings for such a thing. Agreed. Making sure the config can parse is different from parsable but non-sensible. It's ridiculously easy to mistakenly add a line w/ a single character on it or something equally bad when saving a file that's being modified by hand. That's a simple check that should be done on re-hup and the broken config shouldn't be put in place. I certainly agree that we should *also* have a way to just check the config, so that can be built into init scripts and whatnot. I don't think having one precludes having the other, and I'm pretty confident we could find a way to not duplicate the code and have things be clean. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies
Stephen Frost wrote: > * Magnus Hagander ([EMAIL PROTECTED]) wrote: >> For pg_hba.conf, I don't see that as a very big problem, really. It >> doesn't (and shouldn't) modify any "external" variables, so it should be >> as simple as parsing the new file into a completely separate >> list-of-structs and only if it's all correct switch the main pointer >> (and free the old struct). > > I'm in agreement with this approach. Allowing a config which won't > parse properly to completely break access to a running database is > terrible. It just doesn't come across to me as being all that difficult > or complex code for pg_hba.conf. That's my thoughts as well, which may be off of course ;-) >> Yes, I still think we should do the "simple parsing" step at HUP time. >> That doesn't mean that it wouldn't be a good idea to have one of these >> check-config options that can look for conflicting options *as well*, of >> course. But I'm getting the feeling I'm on the losing side of the debate >> here... > > A little extra code in the backend is well worth fixing this foot-gun. > The concerns raised so far have been "who will write it?" and "what if > it has bugs?". Neither of these are particularly compelling arguments > when you've already offered to write and bug-test it (right, Magnus? :). Toms main argument has been that it would move the code *from* the backend and into the *postmaster*, which is much more sensitive. And yes, I've offered to write the code. I take this as an offer from you to bug-test it :-) //Magnus -- 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] Parsing of pg_hba.conf and authentication inconsistencies
* Magnus Hagander ([EMAIL PROTECTED]) wrote: > For pg_hba.conf, I don't see that as a very big problem, really. It > doesn't (and shouldn't) modify any "external" variables, so it should be > as simple as parsing the new file into a completely separate > list-of-structs and only if it's all correct switch the main pointer > (and free the old struct). I'm in agreement with this approach. Allowing a config which won't parse properly to completely break access to a running database is terrible. It just doesn't come across to me as being all that difficult or complex code for pg_hba.conf. > Yes, I still think we should do the "simple parsing" step at HUP time. > That doesn't mean that it wouldn't be a good idea to have one of these > check-config options that can look for conflicting options *as well*, of > course. But I'm getting the feeling I'm on the losing side of the debate > here... A little extra code in the backend is well worth fixing this foot-gun. The concerns raised so far have been "who will write it?" and "what if it has bugs?". Neither of these are particularly compelling arguments when you've already offered to write and bug-test it (right, Magnus? :). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies
"Tom Lane" <[EMAIL PROTECTED]> writes: > Robert Treat <[EMAIL PROTECTED]> writes: >> Certainly there isn't any reason to allow a reload of a file that is just >> going to break things when the first connection happens. For that matter, >> why would we ever not want to parse it at HUP time rather than connect time? > > Two or three reasons why not were already mentioned upthread, but for > the stubborn, here's another one: are you volunteering to write the code > that backs out the config-file reload after the checks have determined > it was bad? Given the amount of pain we suffered trying to make GUC do > something similar, any sane person would run screaming from the > prospect. Wouldn't that be *easier* if we do more parsing in the postmaster instead of in the backends as Magnus suggested? Then it could build a new set of structures and if there are any errors just throw them out before replacing the old ones. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support! -- 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] Auto-explain patch
Thanks for the feedback, and sorry for my delay in replying (I was on holiday). > Tom Lane wrote: > > Comments: > > I do not think that you should invent a new elog level for this, and > especially not one that is designed to send unexpected messages to the > client. Having to kluge tab completion like that is just a signal that > you're going to break a lot of other clients too. It seems to me that > the right behavior for auto-explain messages is to go only to the log by > default, which means that LOG is already a perfectly good elog level for > auto-explain messages. The more I thought about this, the more I thought that it was OTT to add a new elog level just for this, so I agree it should probably just go to the LOG elog level. I don't agree with your reasoning on tab-completion though. I actually think that this is a signal of a broken client. If the user sets client_min_messages to LOG or lower, and has any of the other logging or debugging parameters enabled, the output tramples all over the suggested completions. I don't think the average user is interested in log/debug output from the queries psql does internally during tab-completion. So IMHO the tab-completion 'kludge', is still worthwhile, regardless of the rest of the patch. > Drop the query_string addition to PlannedStmt --- there are other ways > you can get that string in CVS HEAD. OK. What is the best way to get this string now? Are you referring to debug_query_string, or is there another way? > I don't think that planner_time > belongs there either. It would be impossible to define a principled way > to compare two PlannedStmts for equality with that in there. Nor do I > see the point of doing it the way you're doing it. Why don't you just > log the slow planning cycle immediately upon detecting it in planner()? > I don't see that a slow planning cycle has anything necessarily to do > with a slow execution cycle, so IMHO they ought to just get logged > independently. Makes sense. > Please do not export ExplainState --- that's an internal matter for > explain.c. Export some wrapper function with a cleaner API than > explain_outNode, instead. > > regards, tom lane OK, that's much neater. I'll try to rework this ASAP but I understand if it's too late for this commitfest. Cheers, Dean. _ Win a voice over part with Kung Fu Panda & Live Search and 100’s of Kung Fu Panda prizes to win with Live Search http://clk.atdmt.com/UKM/go/107571439/direct/01/ -- 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] Parsing of pg_hba.conf and authentication inconsistencies
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: >>> The good way to solve this would be to have independant command line >>> utilities which check pg_hba.conf, pg_ident.conf and postgresql.conf for >>> errors. Then DBAs could run a check *before* restarting the server. > >> While clearly useful, it'd still leave the fairly large foot-gun that is >> editing the hba file and HUPing things which can leave you with a >> completely un-connectable database because of a small typo. > > That will *always* be possible, just because software is finite and > human foolishness is not ;-). Certainly - been bitten by that more than once. But we can make it harder or easier to make the mistakes.. > Now, we could ameliorate it a bit given a "postgres --check-config" > mode by having pg_ctl automatically run that mode before any start, > restart, or reload command, and then refusing to proceed if the check > detects any indubitable errors. On the other hand, that would leave > us with the scenario where the checking code warns about stuff that it > can't be sure is wrong, but then we go ahead and install the borked > config anyway. (Nobody is going to put up with code that refuses > to install config settings that aren't 100% clean, unless the checks > are so weak that they miss a lot of possibly-useful warnings.) > > Seems a lot better to me to just train people to run the check-config > code by hand before pulling the trigger to load the settings for real. It's certainly easier for us, but that means a whole lot of people are never going to do it. And initscripts might end up using it anyway, forcing the issue. I think it'd be reasonable to refuse starting if the config is *known broken* (such as containing lines that are unparseable, or that contain completely invalid tokens), whereas you'd start if they just contain things that are "probably wrong". But picking from your previous examples of "more advanced checks", there are lots of cases where things like overlapping CIDR address ranges are perfectly valid, so I don't think we could even throw a warning for that - unless there's a separate flag to enable/disable warnings for such a thing. //Magnus -- 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] Parsing of pg_hba.conf and authentication inconsistencies
Tom Lane wrote: > Robert Treat <[EMAIL PROTECTED]> writes: >> Certainly there isn't any reason to allow a reload of a file that is just >> going to break things when the first connection happens. For that matter, >> why would we ever not want to parse it at HUP time rather than connect time? > > Two or three reasons why not were already mentioned upthread, but for > the stubborn, here's another one: are you volunteering to write the code > that backs out the config-file reload after the checks have determined > it was bad? Given the amount of pain we suffered trying to make GUC do > something similar, any sane person would run screaming from the > prospect. For pg_hba.conf, I don't see that as a very big problem, really. It doesn't (and shouldn't) modify any "external" variables, so it should be as simple as parsing the new file into a completely separate list-of-structs and only if it's all correct switch the main pointer (and free the old struct). Yes, I still think we should do the "simple parsing" step at HUP time. That doesn't mean that it wouldn't be a good idea to have one of these check-config options that can look for conflicting options *as well*, of course. But I'm getting the feeling I'm on the losing side of the debate here... //Magnus -- 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] Mini improvement: statement_cost_limit
On Sat, Aug 02, 2008 at 09:30:08PM +0200, Hans-Jürgen Schönig wrote: > On Aug 2, 2008, at 8:38 PM, Tom Lane wrote: > > >Andrew Dunstan <[EMAIL PROTECTED]> writes: > >>Hans-Jürgen Schönig wrote: > >>>i introduced a GUC called statement_cost_limit which can be used to > >>>error out if a statement is expected to be too expensive. > > > >>You clearly have far more faith in the cost estimates than I do. > > > >Wasn't this exact proposal discussed and rejected awhile back? > > > > regards, tom lane > > > > > i don't remember precisely. > i have seen it on simon's wiki page and it is something which would > have been useful in some cases in the past. I think a variation on this could be very useful in development and test environments. Suppose it raised a warning or notice if the cost was over the limit. Then one could set a limit of a few million on the development and test servers and developers would at least have a clue that they needed to look at explain for that query. As it is now, one can exhort them to run explain, but it has no effect. Instead we later see queries killed by a 24 hour timeout with estimated costs ranging from "until they unplug the machine and dump it" to "until the sun turns into a red giant". -dg -- David Gould [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers