Re: [PATCHES] Preliminary GSSAPI Patches
The server uses the keytab to decrypt the token provided by the client. By using the GSS_C_NO_CREDENTIAL arg on the server anything put in the keytab is OK. (The server doesn't need to authenticate itself to Kerberos, it just accepts authentication. Mutual authentication is done using the same keys.) The documentation needs to reflect that. I agree there's some disconnect there between the documentation and the apparent implementation but I'm not sure I'm in favor of changing the documentation on this one. Personally, I'd rather it return an error if someone tries to use GSS_C_NO_CREDENTIAL when accepting a context than to just be happy using anything in the keytab. How about doing both, then? Set the principal name if it's specified in the config file. If it's explicitly set to an empty string, use GSS_C_NO_CREDENTIAL. Seems straightforward enough to me, and shouldn't be hard to implement. I don't have a problem with that, but you'll want multiple service names as soon as you want to support the SSPI. Also don't get too bent out of shape about some client using the wrong service name. The client *still* needs to prove who it represents; there's no hole there. The only real security issue I can think of is that someone who subverts the PostgreSQL server could steal the host service keys and then (with a whole bunch of other work) masquerade as the SSH daemon. Ok. that's certainly a lot more narrow than I thought. you can see from my updated patch that it's not particularly lots of code. but if the gain is so little and we end up recommending people not to use that part anyway for compatibility I'm more than happy to take it out again. You certainly know more about these aspect of gss than me ;) Don't read too much into the mod_auth_kerb situation. The main reason it still takes a specific, configured service name is that neither Russ Allbery nor I has gotten around to submitting a proper patch to fix that. The only reason it was written the way it is in the first place is that the ability to use GSS_C_NO_CREDENTIAL that way is obscure and most people don't know it. I can say that with some confidence because Russ and I had a long discussion with Sam Hartman about how it ought to be done. Ok. That makes a lot of sense then. I'm told that the way Apple's equivalent to mod_auth_kerb works is it uses GSS_C_NO_CREDENTIAL and then does a case-insensitive compare of the resulting match to HTTP. We could do the same thing, if you think it's worth it. Do you know if this is documented somewhere? It's always nice with references. /Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Preliminary GSSAPI Patches
On Jun 23, 2007, at 1:44 AM, Magnus Hagander wrote: Stephen Frost wrote: * Henry B. Hotz ([EMAIL PROTECTED]) wrote: On Jun 22, 2007, at 9:56 AM, Magnus Hagander wrote: Most likely it's just checking the keytab to find a principal with the same name as the one presented from the client. Since one is present, it loads it up automatically, and verifies against it. Bingo! The server uses the keytab to decrypt the token provided by the client. By using the GSS_C_NO_CREDENTIAL arg on the server anything put in the keytab is OK. (The server doesn't need to authenticate itself to Kerberos, it just accepts authentication. Mutual authentication is done using the same keys.) The documentation needs to reflect that. I agree there's some disconnect there between the documentation and the apparent implementation but I'm not sure I'm in favor of changing the documentation on this one. Personally, I'd rather it return an error if someone tries to use GSS_C_NO_CREDENTIAL when accepting a context than to just be happy using anything in the keytab. How about doing both, then? Set the principal name if it's specified in the config file. If it's explicitly set to an empty string, use GSS_C_NO_CREDENTIAL. Seems straightforward enough to me, and shouldn't be hard to implement. I don't have a problem with that, but you'll want multiple service names as soon as you want to support the SSPI. Also don't get too bent out of shape about some client using the wrong service name. The client *still* needs to prove who it represents; there's no hole there. The only real security issue I can think of is that someone who subverts the PostgreSQL server could steal the host service keys and then (with a whole bunch of other work) masquerade as the SSH daemon. If we do do that, then we need to allow for the ways Microsoft mucks with the case of the name. (Kerberos is supposed to be case sensitive, but Microsoft work that way.) In particular I think we may need both postgres/server and POSTGRES/server in the keytab in order to support the to-be-written native Windows SSPI client at the same time as the current Kerberos 5 and GSSAPI Unix clients. Supporting multiple, specific, keys might be an interesting challenge, but I'm not too keen on worrying about it right now regardless. I'd also much rather err on the side of overly paranoid than if it works, just let it in. If someone ends up having to support both windows SSPI clients and unix Kerberos/GSSAPI clients it's entirely possible to suggest they just make it POSTGRES and configure the clients accordingly. Yeah, that's how we do it today with Kerberos. But it *would* be handy if this was easier ;-) Don't read too much into the mod_auth_kerb situation. The main reason it still takes a specific, configured service name is that neither Russ Allbery nor I has gotten around to submitting a proper patch to fix that. The only reason it was written the way it is in the first place is that the ability to use GSS_C_NO_CREDENTIAL that way is obscure and most people don't know it. I can say that with some confidence because Russ and I had a long discussion with Sam Hartman about how it ought to be done. I'm told that the way Apple's equivalent to mod_auth_kerb works is it uses GSS_C_NO_CREDENTIAL and then does a case-insensitive compare of the resulting match to HTTP. We could do the same thing, if you think it's worth it. The opinions expressed in this message are mine, not those of Caltech, JPL, NASA, or the US Government. [EMAIL PROTECTED], or [EMAIL PROTECTED] ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Preliminary GSSAPI Patches
On Jun 24, 2007, at 11:03 PM, Magnus Hagander wrote: I'm told that the way Apple's equivalent to mod_auth_kerb works is it uses GSS_C_NO_CREDENTIAL and then does a case-insensitive compare of the resulting match to HTTP. We could do the same thing, if you think it's worth it. Do you know if this is documented somewhere? It's always nice with references. Not as far as I know, publicly. I heard most of it from an Apple developer at the 2005 WWDC (and I inferred the rest from things Sam Hartman has said). I guess that technically puts it under NDA, except I think the code in question is open source. I don't know which project it's in so I haven't been able to locate it to verify what I said for sure. What I can say for certain concerns the client side. Apple's Safari browser went through at least two iterations before they got it right: 1) in OSX 10.3 Safari would ask for a server/ server.example.com service ticket. 2) in early 10.4 Safari would ask for a http/server.example.com service ticket (this actually works fine if have Active Directory as your Kerberos server, and IIS, or Apple as your web server). 3) in later 10.4 Safari asks for a HTTP/server.example.com service ticket. This is the correct thing to do. Due to the numbers of people talking to Apple about the situation (state 2) during that WWDC, they publicly acknowledged the problem and promised to fix it during the same WWDC. If you have access to the video recordings you can probably find the relevant session in the latter half of the week. The key technical point is that Kerberos is case sensitive, but Windows Kerberos isn't. We can deal with that how we choose, but I kind of like Apple's solution. It's annoying to have to put two service principals in the keytab, but I personally prefer that to going upper-case only just 'cause that's the only way Windows SSPI clients can work with non-Windows servers. The opinions expressed in this message are mine, not those of Caltech, JPL, NASA, or the US Government. [EMAIL PROTECTED], or [EMAIL PROTECTED] ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Load Distributed Checkpoints, take 3
Greg Smith wrote: LDC certainly makes things better in almost every case. My allegiance comes from having seen a class of transactions where LDC made things worse on a fast/overloaded system, in that it made some types of service guarantees harder to meet, and I just don't know who else might run into problems in that area. Please describe the class of transactions and the service guarantees so that we can reproduce that, and figure out what's the best solution. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Load Distributed Checkpoints, take 3
On Mon, 2007-06-25 at 01:33 -0400, Greg Smith wrote: On Sun, 24 Jun 2007, Simon Riggs wrote: Greg can't choose to use checkpoint_segments as the limit and then complain about unbounded recovery time, because that was clearly a conscious choice. I'm complaining I apologise for using that emotive phrase. only because everyone seems content to wander in a direction where the multiplier on checkpoint_segments for how many segments are actually active at once will go up considerably, which can make a known problem (recovery time) worse. +50% more. Recovery time is a consideration that can be adjusted for. We have done nothing to make recovery rate worse; the additional WAL leads to an increased recovery time *only* if you keep the same parameter settings. There is no reason to keep them the same, nor do we promise that parameters will keep the exact meaning they had previous releases. As you say, we can put comments in the release notes to advise people of 50% increase in recovery time if the parameters stay the same. That would be balanced by the comment that checkpoints are now considerably smoother than before and more frequent checkpoints are unlikely to be a problem, so it is OK to reduce the parameters from the settings you used in previous releases. I don't see any reason why we would want unsmooth checkpoints; similarly we don't want unsmooth bg writing, unsmooth archiving etc.. Performance will increase, response times will drop and people will be happy. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] New Zealand - TZ change
I would like to inform, that New Zealand changed DST rules and new timezone files are available. See http://www.dia.govt.nz/diawebsite.nsf/wpg_URL/Services-Daylight-Saving-Daylight-saving-to-be-extended Patch for head attached. I kept zic.c untouched, but I think it would be nice to update it as well. Are there any updated release scheduled 8.0-8.2? Zdenek ? tz.diff Index: src/timezone/data/africa === RCS file: /projects/cvsroot/pgsql/src/timezone/data/africa,v retrieving revision 1.5 diff -c -r1.5 africa *** src/timezone/data/africa 19 Apr 2007 22:44:32 - 1.5 --- src/timezone/data/africa 25 Jun 2007 10:26:50 - *** *** 1,4 ! # @(#)africa 8.7 # pre # This data is by no means authoritative; if you think you know better, --- 1,4 ! # @(#)africa 8.8 # pre # This data is by no means authoritative; if you think you know better, *** *** 416,421 --- 416,435 # Namibia # The 1994-04-03 transition is from Shanks Pottenger. # Shanks Pottenger report no DST after 1998-04; go with IATA. + + # From Petronella Sibeene (2007-03-30) in + # http://allafrica.com/stories/200703300178.html: + # While the entire country changes its time, Katima Mulilo and other + # settlements in Caprivi unofficially will not because the sun there + # rises and sets earlier compared to other regions. Chief of + # Forecasting Riaan van Zyl explained that the far eastern parts of + # the country are close to 40 minutes earlier in sunrise than the rest + # of the country. + # + # From Paul Eggert (2007-03-31): + # Apparently the Caprivi Strip informally observes Botswana time, but + # we have no details. In the meantime people there can use Africa/Gaborone. + # RULE NAME FROM TO TYPE IN ON AT SAVE LETTER/S Rule Namibia 1994 max - Sep Sun=1 2:00 1:00 S Rule Namibia 1995 max - Apr Sun=1 2:00 0 - Index: src/timezone/data/australasia === RCS file: /projects/cvsroot/pgsql/src/timezone/data/australasia,v retrieving revision 1.5 diff -c -r1.5 australasia *** src/timezone/data/australasia 19 Apr 2007 22:44:32 - 1.5 --- src/timezone/data/australasia 25 Jun 2007 10:26:51 - *** *** 1,4 ! # @(#)australasia 8.6 # pre # This file also includes Pacific islands. --- 1,4 ! # @(#)australasia 8.7 # pre # This file also includes Pacific islands. *** *** 348,357 Rule Chatham 1976 1989 - Mar Sun=1 2:45s 0 S Rule NZ 1989 only - Oct Sun=8 2:00s 1:00 D Rule Chatham 1989 only - Oct Sun=8 2:45s 1:00 D ! Rule NZ 1990 max - Oct Sun=1 2:00s 1:00 D ! Rule Chatham 1990 max - Oct Sun=1 2:45s 1:00 D ! Rule NZ 1990 max - Mar Sun=15 2:00s 0 S ! Rule Chatham 1990 max - Mar Sun=15 2:45s 0 S # Zone NAME GMTOFF RULES FORMAT [UNTIL] Zone Pacific/Auckland 11:39:04 - LMT 1868 Nov 2 11:30 NZ NZ%sT 1946 Jan 1 --- 348,361 Rule Chatham 1976 1989 - Mar Sun=1 2:45s 0 S Rule NZ 1989 only - Oct Sun=8 2:00s 1:00 D Rule Chatham 1989 only - Oct Sun=8 2:45s 1:00 D ! Rule NZ 1990 2006 - Oct Sun=1 2:00s 1:00 D ! Rule Chatham 1990 2006 - Oct Sun=1 2:45s 1:00 D ! Rule NZ 1990 2007 - Mar Sun=15 2:00s 0 S ! Rule Chatham 1990 2007 - Mar Sun=15 2:45s 0 S ! Rule NZ 2007 max - Sep lastSun 2:00s 1:00 D ! Rule Chatham 2007 max - Sep lastSun 2:45s 1:00 D ! Rule NZ 2008 max - Apr Sun=1 2:00s 0 S ! Rule Chatham 2008 max - Apr Sun=1 2:45s 0 S # Zone NAME GMTOFF RULES FORMAT [UNTIL] Zone Pacific/Auckland 11:39:04 - LMT 1868 Nov 2 11:30 NZ NZ%sT 1946 Jan 1 *** *** 1146,1151 --- 1150,1161 # transitions at 2:45 local standard time; this confirms that Chatham # is always exactly 45 minutes ahead of Auckland. + # From Colin Sharples (2007-04-30): + # DST will now start on the last Sunday in September, and end on the + # first Sunday in April. The changes take effect this year, meaning + # that DST will begin on 2007-09-30 2008-04-06. + # http://www.dia.govt.nz/diawebsite.nsf/wpg_URL/Services-Daylight-Saving-Daylight-saving-to-be-extended + ### Index: src/timezone/data/northamerica === RCS file: /projects/cvsroot/pgsql/src/timezone/data/northamerica,v retrieving revision 1.7 diff -c -r1.7 northamerica *** src/timezone/data/northamerica 19 Apr 2007 22:44:32 - 1.7 --- src/timezone/data/northamerica 25 Jun 2007 10:26:54 - *** *** 1,4 ! # @(#)northamerica 8.16 # pre # also includes Central America and the Caribbean --- 1,4 ! # @(#)northamerica 8.17 # pre # also includes Central America and the Caribbean *** *** 2325,2330 --- 2325,2333 # # The reason seems to be an energy crisis. + # From Stephen Colebourne (2007-02-22): + # Some IATA info: Haiti
Re: [PATCHES] Load Distributed Checkpoints, take 3
On Mon, Jun 25, 2007 at 10:15:07AM +0100, Simon Riggs wrote: On Mon, 2007-06-25 at 01:33 -0400, Greg Smith wrote: On Sun, 24 Jun 2007, Simon Riggs wrote: Greg can't choose to use checkpoint_segments as the limit and then complain about unbounded recovery time, because that was clearly a conscious choice. I'm complaining I apologise for using that emotive phrase. only because everyone seems content to wander in a direction where the multiplier on checkpoint_segments for how many segments are actually active at once will go up considerably, which can make a known problem (recovery time) worse. +50% more. Recovery time is a consideration that can be adjusted for. We have done nothing to make recovery rate worse; the additional WAL leads to an increased recovery time *only* if you keep the same parameter settings. There is no reason to keep them the same, nor do we promise that parameters will keep the exact meaning they had previous releases. As you say, we can put comments in the release notes to advise people of 50% increase in recovery time if the parameters stay the same. That would be balanced by the comment that checkpoints are now considerably smoother than before and more frequent checkpoints are unlikely to be a problem, so it is OK to reduce the parameters from the settings you used in previous releases. Didn't we already add other featuers that makes recovery much *faster* than before? In that case, are they faster enugh to neutralise this increased time (a guestimate, of course) Or did I mess that up with stuff we added for 8.2? :-) //Magnus ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Load Distributed Checkpoints, take 3
Magnus Hagander wrote: On Mon, Jun 25, 2007 at 10:15:07AM +0100, Simon Riggs wrote: As you say, we can put comments in the release notes to advise people of 50% increase in recovery time if the parameters stay the same. That would be balanced by the comment that checkpoints are now considerably smoother than before and more frequent checkpoints are unlikely to be a problem, so it is OK to reduce the parameters from the settings you used in previous releases. Didn't we already add other featuers that makes recovery much *faster* than before? Yes, we no longer read in a page just to overwrite it with a full page image. In that case, are they faster enugh to neutralise this increased time (a guestimate, of course) It depends a lot on your workload. Under the right circumstances, it will more than offset any increase caused by LDC, but sometimes it won't make any difference at all (for example with full_page_writes=off). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Load Distributed Checkpoints, take 3
On Mon, 2007-06-25 at 12:56 +0200, Magnus Hagander wrote: Didn't we already add other featuers that makes recovery much *faster* than before? In that case, are they faster enugh to neutralise this increased time (a guestimate, of course) Or did I mess that up with stuff we added for 8.2? :-) Only with full_page_writes on, but yes you're right, the predicted recovery times are changing for 8.3 anyway. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] New Zealand - TZ change
Zdenek Kotala [EMAIL PROTECTED] writes: I would like to inform, that New Zealand changed DST rules and new timezone files are available. See http://www.dia.govt.nz/diawebsite.nsf/wpg_URL/Services-Daylight-Saving-Daylight-saving-to-be-extended Patch for head attached. We do not patch those files; the update technique is download, untar, commit, and there's really no point in doing it except when a release is imminent. See src/timezone/README and src/tools/RELEASE_CHANGES. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Preliminary GSSAPI Patches
On Mon, Jun 25, 2007 at 12:31:44AM -0700, Henry B. Hotz wrote: On Jun 24, 2007, at 11:03 PM, Magnus Hagander wrote: I'm told that the way Apple's equivalent to mod_auth_kerb works is it uses GSS_C_NO_CREDENTIAL and then does a case-insensitive compare of the resulting match to HTTP. We could do the same thing, if you think it's worth it. Do you know if this is documented somewhere? It's always nice with references. Not as far as I know, publicly. I heard most of it from an Apple developer at the 2005 WWDC (and I inferred the rest from things Sam Hartman has said). I guess that technically puts it under NDA, except I think the code in question is open source. I don't know which project it's in so I haven't been able to locate it to verify what I said for sure. Ok. no problem. What I can say for certain concerns the client side. Apple's Safari browser went through at least two iterations before they got it right: 1) in OSX 10.3 Safari would ask for a server/ server.example.com service ticket. 2) in early 10.4 Safari would ask for a http/server.example.com service ticket (this actually works fine if have Active Directory as your Kerberos server, and IIS, or Apple as your web server). 3) in later 10.4 Safari asks for a HTTP/server.example.com service ticket. This is the correct thing to do. Due to the numbers of people talking to Apple about the situation (state 2) during that WWDC, they publicly acknowledged the problem and promised to fix it during the same WWDC. If you have access to the video recordings you can probably find the relevant session in the latter half of the week. The key technical point is that Kerberos is case sensitive, but Windows Kerberos isn't. We can deal with that how we choose, but I kind of like Apple's solution. It's annoying to have to put two service principals in the keytab, but I personally prefer that to going upper-case only just 'cause that's the only way Windows SSPI clients can work with non-Windows servers. Interesting, indeed. I think gonig down the same approach they were using is the best way to do, so I've changed my working copy back to that version, and will update the documentation with that information. //Magnus ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Load Distributed Checkpoints, take 3
Heikki Linnakangas [EMAIL PROTECTED] writes: On further thought, there is one workload where removing the non-LRU part would be counterproductive: If you have a system with a very bursty transaction rate, it's possible that when it's time for a checkpoint, there hasn't been any WAL logged activity since last checkpoint, so we skip it. When that happens, the buffer cache might still be full of dirty pages, because of hint bit updates. That still isn't a problem on it's own, but if you then do a huge batch update, you have to flush those dirty pages at that point. It would be better to trickle flush those dirty pages during the idle period. But wouldn't the LRU-based scan accomplish that? regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[PATCHES] remove unused caller arg from stringToQualifiedNameList
This function seems to have an argument that is no longer used (probably
leftover from when it used to report an error message?). This rather
trivial patch removes it and fixes associated fallout.
--
Alvaro Herrerahttp://www.advogato.org/person/alvherre
Before you were born your parents weren't as boring as they are now. They
got that way paying your bills, cleaning up your room and listening to you
tell them how idealistic you are. -- Charles J. Sykes' advice to teenagers
Index: src/backend/utils/adt/regproc.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/adt/regproc.c,v
retrieving revision 1.101
diff -c -p -r1.101 regproc.c
*** src/backend/utils/adt/regproc.c 5 Jun 2007 21:31:06 - 1.101
--- src/backend/utils/adt/regproc.c 25 Jun 2007 17:09:54 -
***
*** 35,42
#include utils/lsyscache.h
#include utils/syscache.h
! static void parseNameAndArgTypes(const char *string, const char *caller,
! bool allowNone,
List **names, int *nargs, Oid *argtypes);
--- 35,41
#include utils/lsyscache.h
#include utils/syscache.h
! static void parseNameAndArgTypes(const char *string, bool allowNone,
List **names, int *nargs, Oid *argtypes);
*** regprocin(PG_FUNCTION_ARGS)
*** 127,133
* Normal case: parse the name into components and see if it matches any
* pg_proc entries in the current search path.
*/
! names = stringToQualifiedNameList(pro_name_or_oid, regprocin);
clist = FuncnameGetCandidates(names, -1);
if (clist == NULL)
--- 126,132
* Normal case: parse the name into components and see if it matches any
* pg_proc entries in the current search path.
*/
! names = stringToQualifiedNameList(pro_name_or_oid);
clist = FuncnameGetCandidates(names, -1);
if (clist == NULL)
*** regprocedurein(PG_FUNCTION_ARGS)
*** 271,278
* datatype cannot be used for any system column that needs to receive
* data during bootstrap.
*/
! parseNameAndArgTypes(pro_name_or_oid, regprocedurein, false,
! names, nargs, argtypes);
clist = FuncnameGetCandidates(names, nargs);
--- 270,276
* datatype cannot be used for any system column that needs to receive
* data during bootstrap.
*/
! parseNameAndArgTypes(pro_name_or_oid, false, names, nargs, argtypes);
clist = FuncnameGetCandidates(names, nargs);
*** regoperin(PG_FUNCTION_ARGS)
*** 476,482
* Normal case: parse the name into components and see if it matches any
* pg_operator entries in the current search path.
*/
! names = stringToQualifiedNameList(opr_name_or_oid, regoperin);
clist = OpernameGetCandidates(names, '\0');
if (clist == NULL)
--- 474,480
* Normal case: parse the name into components and see if it matches any
* pg_operator entries in the current search path.
*/
! names = stringToQualifiedNameList(opr_name_or_oid);
clist = OpernameGetCandidates(names, '\0');
if (clist == NULL)
*** regoperatorin(PG_FUNCTION_ARGS)
*** 626,633
* datatype cannot be used for any system column that needs to receive
* data during bootstrap.
*/
! parseNameAndArgTypes(opr_name_or_oid, regoperatorin, true,
! names, nargs, argtypes);
if (nargs == 1)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_PARAMETER),
--- 624,630
* datatype cannot be used for any system column that needs to receive
* data during bootstrap.
*/
! parseNameAndArgTypes(opr_name_or_oid, true, names, nargs, argtypes);
if (nargs == 1)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_PARAMETER),
*** regclassin(PG_FUNCTION_ARGS)
*** 827,833
* Normal case: parse the name into components and see if it matches any
* pg_class entries in the current search path.
*/
! names = stringToQualifiedNameList(class_name_or_oid, regclassin);
result = RangeVarGetRelid(makeRangeVarFromNameList(names), false);
--- 824,830
* Normal case: parse the name into components and see if it matches any
* pg_class entries in the current search path.
*/
! names = stringToQualifiedNameList(class_name_or_oid);
result = RangeVarGetRelid(makeRangeVarFromNameList(names), false);
*** text_regclass(PG_FUNCTION_ARGS)
*** 1093,1099
* Given a C string, parse it into a qualified-name list.
*/
List *
! stringToQualifiedNameList(const char *string, const char *caller)
{
char *rawname;
List *result = NIL;
--- 1090,1096
* Given a C string, parse it into a qualified-name list.
*/
List *
! stringToQualifiedNameList(const char *string)
{
char *rawname;
List *result = NIL;
*** stringToQualifiedNameList(const char *st
*** 1141,1149
* for unary operators).
*/
static void
! parseNameAndArgTypes(const char
Re: [PATCHES] remove unused caller arg from stringToQualifiedNameList
Alvaro Herrera [EMAIL PROTECTED] writes: This function seems to have an argument that is no longer used (probably leftover from when it used to report an error message?). Yeah, I recall having left the argument in place because it seemed like we might want it again someday. But that was for 7.4 which was awhile ago. There's probably not a strong argument to keep it, but on the other hand is there a strong argument to remove it? regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] remove unused caller arg from stringToQualifiedNameList
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: This function seems to have an argument that is no longer used (probably leftover from when it used to report an error message?). Yeah, I recall having left the argument in place because it seemed like we might want it again someday. But that was for 7.4 which was awhile ago. There's probably not a strong argument to keep it, but on the other hand is there a strong argument to remove it? Other than removing cruft, nope, I don't see any ... In any case, this is an exported symbol so maybe it's not a good idea to mess with it. OTOH I checked PL/R and PL/php and neither uses it, so this may not be a problem at all. -- Alvaro Herrera http://www.flickr.com/photos/alvherre/ The ability to monopolize a planet is insignificant next to the power of the source ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] remove SIBackendInit return value
SIBackendInit returns a flag indicating whether it worked or not. Since
there is only one caller and it dies with a FATAL error when
SIBackendInit failed, it seems better to move the elog and remove the
return value, per this patch.
--
Alvaro Herrera http://www.amazon.com/gp/registry/5ZYLFMCVHXC
Always assume the user will do much worse than the stupidest thing
you can imagine.(Julien PUYDT)
Index: src/backend/storage/ipc/sinval.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/storage/ipc/sinval.c,v
retrieving revision 1.82
diff -c -p -r1.82 sinval.c
*** src/backend/storage/ipc/sinval.c 5 Jan 2007 22:19:38 - 1.82
--- src/backend/storage/ipc/sinval.c 25 Jun 2007 17:33:06 -
*** CreateSharedInvalidationState(void)
*** 67,83
void
InitBackendSharedInvalidationState(void)
{
- int flag;
-
LWLockAcquire(SInvalLock, LW_EXCLUSIVE);
! flag = SIBackendInit(shmInvalBuffer);
LWLockRelease(SInvalLock);
- if (flag 0)/* unexpected problem */
- elog(FATAL, shared cache invalidation initialization failed);
- if (flag == 0)/* expected problem: MaxBackends exceeded */
- ereport(FATAL,
- (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
- errmsg(sorry, too many clients already)));
}
/*
--- 67,75
void
InitBackendSharedInvalidationState(void)
{
LWLockAcquire(SInvalLock, LW_EXCLUSIVE);
! SIBackendInit(shmInvalBuffer);
LWLockRelease(SInvalLock);
}
/*
Index: src/backend/storage/ipc/sinvaladt.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/storage/ipc/sinvaladt.c,v
retrieving revision 1.63
diff -c -p -r1.63 sinvaladt.c
*** src/backend/storage/ipc/sinvaladt.c 5 Jan 2007 22:19:38 - 1.63
--- src/backend/storage/ipc/sinvaladt.c 25 Jun 2007 17:33:51 -
*** SIBufferInit(void)
*** 79,96
/*
* SIBackendInit
! * Initialize a new backend to operate on the sinval buffer
! *
! * Returns:
! * 0 A-OK
! * 0 Failed to find a free procState slot (ie, MaxBackends exceeded)
! * 0 Some other failure (not currently used)
*
* NB: this routine, and all following ones, must be executed with the
* SInvalLock lock held, since there may be multiple backends trying
* to access the buffer.
*/
! int
SIBackendInit(SISeg *segP)
{
int index;
--- 79,92
/*
* SIBackendInit
! * Initialize a new backend to operate on the sinval buffer. It fails
! * with a FATAL error if there are no free procState slots.
*
* NB: this routine, and all following ones, must be executed with the
* SInvalLock lock held, since there may be multiple backends trying
* to access the buffer.
*/
! void
SIBackendInit(SISeg *segP)
{
int index;
*** SIBackendInit(SISeg *segP)
*** 117,124
else
{
/* out of procState slots */
! MyBackendId = InvalidBackendId;
! return 0;
}
}
--- 113,121
else
{
/* out of procState slots */
! ereport(FATAL,
! (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
! errmsg(sorry, too many clients already)));
}
}
*** SIBackendInit(SISeg *segP)
*** 137,144
/* register exit routine to mark my entry inactive at exit */
on_shmem_exit(CleanupInvalidationState, PointerGetDatum(segP));
-
- return 1;
}
/*
--- 134,139
Index: src/include/storage/sinvaladt.h
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/storage/sinvaladt.h,v
retrieving revision 1.42
diff -c -p -r1.42 sinvaladt.h
*** src/include/storage/sinvaladt.h 5 Jan 2007 22:19:58 - 1.42
--- src/include/storage/sinvaladt.h 25 Jun 2007 17:32:49 -
*** extern SISeg *shmInvalBuffer; /* pointer
*** 107,113
* prototypes for functions in sinvaladt.c
*/
extern void SIBufferInit(void);
! extern int SIBackendInit(SISeg *segP);
extern bool SIInsertDataEntry(SISeg *segP, SharedInvalidationMessage *data);
extern int SIGetDataEntry(SISeg *segP, int backendId,
--- 107,113
* prototypes for functions in sinvaladt.c
*/
extern void SIBufferInit(void);
! extern void SIBackendInit(SISeg *segP);
extern bool SIInsertDataEntry(SISeg *segP, SharedInvalidationMessage *data);
extern int SIGetDataEntry(SISeg *segP, int backendId,
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Load Distributed Checkpoints, take 3
Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: On further thought, there is one workload where removing the non-LRU part would be counterproductive: If you have a system with a very bursty transaction rate, it's possible that when it's time for a checkpoint, there hasn't been any WAL logged activity since last checkpoint, so we skip it. When that happens, the buffer cache might still be full of dirty pages, because of hint bit updates. That still isn't a problem on it's own, but if you then do a huge batch update, you have to flush those dirty pages at that point. It would be better to trickle flush those dirty pages during the idle period. But wouldn't the LRU-based scan accomplish that? It only scans bgwriter_lru_percent buffers ahead of the clock hand. If the hand isn't moving, it keeps scanning the same buffers over and over again. You can crank it all the way up to 100%, though, in which case it would work, but that starts to get expensive CPU-wise. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] remove SIBackendInit return value
Alvaro Herrera [EMAIL PROTECTED] writes: SIBackendInit returns a flag indicating whether it worked or not. Since there is only one caller and it dies with a FATAL error when SIBackendInit failed, it seems better to move the elog and remove the return value, per this patch. The reason for the existing coding is to release the SInvalLock before throwing the error. Now proc_exit cleanup should release the lock anyway, but this proposed change will mean that a failing backend holds the lock a bit longer before releasing, which might be a bad thing. A more severe consequence would be if the proc_exit sequence tried to grab SInvalLock again before getting to LWLockReleaseAll. I think that's not possible but I'm not 100% certain. In particular it's a bit scary that CleanupInvalidationState is registered with on_proc_exit while still holding the lock; maybe we should change that? If an error were to be thrown at just that instant, it *would* fail because ProcKill will be further down the on_proc_exit list than CleanupInvalidationState. On the whole it doesn't seem worth messing with, or at least not to just this extent. If you want to refactor this, I'd suggest that management of the SInvalLock should be moved over to the sinvaladt.c side of the fence altogether. It seems a bit bogus that CleanupInvalidationState knows about the lock while the other routines in that file don't. (Actually, the division of responsibility between sinval.c and sinvaladt.c has never been real clean AFAICS ... so I think if you want to touch this you should try to make that division better-specified.) regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] msvc and vista fun
Andrew Dunstan wrote: I wrote: Would making a change like this in those 12 places be so ugly? Specifically, I propose the following patch, which should fix the issues buildfarm apparently has with the XP command shell (or some incarnations of it). Strictly speaking, we don't need the changes in builddoc.bat and install.bat, but I did them for the sake of completeness. Since it should not affect anyone not setting XP_EXIT_FIX it seems fairly low risk. Yeah. I still would like to have it confirmed that this is actually an XP fault, and not something else in your environment. To that end, I've bugged Dave to run some builds on his XP VM (since it apparently works fine on my XP x64 machine). //magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] msvc and vista fun
Magnus Hagander wrote: Andrew Dunstan wrote: I wrote: Would making a change like this in those 12 places be so ugly? Specifically, I propose the following patch, which should fix the issues buildfarm apparently has with the XP command shell (or some incarnations of it). Strictly speaking, we don't need the changes in builddoc.bat and install.bat, but I did them for the sake of completeness. Since it should not affect anyone not setting XP_EXIT_FIX it seems fairly low risk. Yeah. I still would like to have it confirmed that this is actually an XP fault, and not something else in your environment. To that end, I've bugged Dave to run some builds on his XP VM (since it apparently works fine on my XP x64 machine). Ok. FWIW, this box is extremely clean and up to date. And I had the identical issue on my previous XP/x386 box. It would be very strange if somehow two environments got polluted like this. cheers andrew ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Load Distributed Checkpoints, take 3
Heikki Linnakangas [EMAIL PROTECTED] writes: Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: If you have a system with a very bursty transaction rate, it's possible that when it's time for a checkpoint, there hasn't been any WAL logged activity since last checkpoint, so we skip it. When that happens, the buffer cache might still be full of dirty pages, because of hint bit updates. That still isn't a problem on it's own, but if you then do a huge batch update, you have to flush those dirty pages at that point. It would be better to trickle flush those dirty pages during the idle period. But wouldn't the LRU-based scan accomplish that? It only scans bgwriter_lru_percent buffers ahead of the clock hand. If the hand isn't moving, it keeps scanning the same buffers over and over again. You can crank it all the way up to 100%, though, in which case it would work, but that starts to get expensive CPU-wise. Hmm. But if we're going to do that, we might as well have a checkpoint for our troubles, no? The reason for the current design is the assumption that a bgwriter_all scan is less burdensome than a checkpoint, but that is no longer true given this rewrite. AFAICS all the bgwriter_all scan will accomplish is induce extra I/O in most scenarios. And it's certainly extra code in an area that's already been rendered pretty darn baroque by this patch. Also, the design assumption here is that the bgwriter_lru scan is supposed to keep enough buffers clean to satisfy the system's need for new buffers. If it's not able to do so, it's not apparent to me that the bgwriter_all scan helps --- the latter is most likely flushing the wrong buffers, ie, those not close in front of the clock sweep. You'd be well advised to increase lru_percent anyway to keep more buffers clean, if this scenario is worth optimizing rather than others for your usage. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] remove SIBackendInit return value
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: SIBackendInit returns a flag indicating whether it worked or not. Since there is only one caller and it dies with a FATAL error when SIBackendInit failed, it seems better to move the elog and remove the return value, per this patch. The reason for the existing coding is to release the SInvalLock before throwing the error. Now proc_exit cleanup should release the lock anyway, but this proposed change will mean that a failing backend holds the lock a bit longer before releasing, which might be a bad thing. I'll leave this alone until early 8.4. Thanks for the insight. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Load Distributed Checkpoints, take 3
On Mon, 25 Jun 2007, Heikki Linnakangas wrote: Greg, is this the kind of workload you're having, or is there some other scenario you're worried about? The way transitions between completely idle and all-out bursts happen were one problematic area I struggled with. Since the LRU point doesn't move during the idle parts, and the lingering buffers have a usage_count0, the LRU scan won't touch them; the only way to clear out a bunch of dirty buffers leftover from the last burst is with the all-scan. Ideally, you want those to write during idle periods so you're completely clean when the next burst comes. My plan for the code I wanted to put into 8.4 one day was to have something like the current all-scan that defers to the LRU and checkpoint, such that if neither of them are doing anything it would go searching for buffers it might blow out. Because the all-scan mainly gets in the way under heavy load right now I've only found mild settings helpful, but if it had a bit more information about what else was going on it could run much harder during slow spots. That's sort of the next stage to the auto-tuning LRU writer code in the grand design floating through my head. As a general comment on this subject, a lot of the work in LDC presumes you have an accurate notion of how close the next checkpoint is. On systems that can dirty buffers and write WAL really fast, I've found hyper bursty workloads are a challenge for it to cope with. You can go from thinking you have all sorts of time to stream the data out to discovering the next checkpoint is coming up fast in only seconds. In that situation, you'd have been better off had you been writing faster during the period preceeding the burst when the code thought it should be smooth[1]. That falls into the category of things I haven't found a good way for other people to test (I happened to have an internal bursty app that aggrevated this area to use). [1] This is actually a reference to Yacht Rock, one of my favorite web sites: http://www.channel101.com/shows/show.php?show_id=152 -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Load Distributed Checkpoints, take 3
On Mon, 25 Jun 2007, Heikki Linnakangas wrote: It only scans bgwriter_lru_percent buffers ahead of the clock hand. If the hand isn't moving, it keeps scanning the same buffers over and over again. You can crank it all the way up to 100%, though, in which case it would work, but that starts to get expensive CPU-wise. In addition to being a CPU pig, that still won't necessarily work because the way the LRU writer ignores things with a non-zero usage_count. If it's dirty, it's probably been used recently as well. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Load Distributed Checkpoints, take 3
On Mon, 25 Jun 2007, Heikki Linnakangas wrote: Please describe the class of transactions and the service guarantees so that we can reproduce that, and figure out what's the best solution. I'm confident you're already moving in that direction by noticing how the 90th percentile numbers were kind of weird with your 150 warehouse DBT2 tests, and I already mentioned how that could be usefully fleshed out by more tests during beta. That number is the kind of service guarantee I'm talking about--if before 90% of transactions were 4.5ms, but now that number is closer to 6ms, that could be considered worse performance by some service metrics even if the average and worst-case performance were improved. The only thing I can think of if you wanted to make the problem more like what I was seeing would be switching the transaction mix on that around to do more UPDATEs relative to the other types of transactions; having more of those seemed to aggrevate my LDC-related issues because they leave a different pattern of dirty+used buffers around than other operations. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Load Distributed Checkpoints, take 3
Greg Smith [EMAIL PROTECTED] writes: The way transitions between completely idle and all-out bursts happen were one problematic area I struggled with. Since the LRU point doesn't move during the idle parts, and the lingering buffers have a usage_count0, the LRU scan won't touch them; the only way to clear out a bunch of dirty buffers leftover from the last burst is with the all-scan. One thing that might be worth changing is that right now, BgBufferSync starts over from the current clock-sweep point on each call --- that is, each bgwriter cycle. So it can't really be made to write very many buffers without excessive CPU work. Maybe we should redefine it to have some static state carried across bgwriter cycles, such that it would write at most N dirty buffers per call, but scan through X percent of the buffers, possibly across several calls, before returning to the (by now probably advanced) clock-sweep point. This would allow a larger value of X to be used than is currently practical. You might wish to recheck the clock sweep point on each iteration just to make sure the scan hasn't fallen behind it, but otherwise I don't see any downside. The scenario where somebody re-dirties a buffer that was cleaned by the bgwriter scan isn't a problem, because that buffer will also have had its usage_count increased and thereby not be a candidate for replacement. As a general comment on this subject, a lot of the work in LDC presumes you have an accurate notion of how close the next checkpoint is. Yeah; this is one reason I was interested in carrying some write-speed state across checkpoints instead of having the calculation start from scratch each time. That wouldn't help systems that sit idle a long time and suddenly go nuts, but it seems to me that smoothing the write rate across more than one checkpoint could help if the fluctuations occur over a timescale of a few checkpoints. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Maintaining cluster order on insert
[ back to this thread... ] Heikki Linnakangas [EMAIL PROTECTED] writes: Tom Lane wrote: The other question is why is execMain involved in this? That makes the design nonfunctional for tuples inserted in any other way than through the main executor --- COPY for instance. Also, if this is successful, I could see using it on system catalogs eventually. I'm inclined to think that the right design is for heap_insert to call amsuggestblock for itself, or maybe even push that down to RelationGetBufferForTuple. Hmm. My first reaction on that is that having heap_insert reach into an index is a modularity violation. It wouldn't be too hard to make similar changes to COPY that I did to the executor. Well, this entire patch is a modularity violation by definition. Bringing the executor into it just makes the damage worse, by involving relatively high-level code in the communication between two relatively low-level bits of code, and ensuring that we'll have to involve still other bits of high-level code in future if we want to make the behavior work in more scenarios. There is a problem with my suggestion of relying on the relcache: that would only give you the index relation's OID, not a handle to an open Relation for it. I'm not sure this is a killer, however, as the cost of an index_open/index_close cycle isn't really much more than a dynahash search if the index is already open and locked by some upper code level. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] remove unused caller arg from stringToQualifiedNameList
On 6/25/07, Alvaro Herrera [EMAIL PROTECTED] wrote: Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: This function seems to have an argument that is no longer used (probably leftover from when it used to report an error message?). Yeah, I recall having left the argument in place because it seemed like we might want it again someday. But that was for 7.4 which was awhile ago. There's probably not a strong argument to keep it, but on the other hand is there a strong argument to remove it? Other than removing cruft, nope, I don't see any ... In any case, this is an exported symbol so maybe it's not a good idea to mess with it. OTOH I checked PL/R and PL/php and neither uses it, so this may not be a problem at all. FWIW, we remove the second argument in textToQualifiedNameList() two years ago so i think this is just finishing what we already started, IMHO anyway http://archives.postgresql.org/pgsql-committers/2005-05/msg00318.php -- regards, Jaime Casanova Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning. Richard Cook ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] remove unused caller arg from stringToQualifiedNameList
Jaime Casanova [EMAIL PROTECTED] writes: On 6/25/07, Alvaro Herrera [EMAIL PROTECTED] wrote: In any case, this is an exported symbol so maybe it's not a good idea to mess with it. OTOH I checked PL/R and PL/php and neither uses it, so this may not be a problem at all. FWIW, we remove the second argument in textToQualifiedNameList() two years ago so i think this is just finishing what we already started, True, we whack around APIs like this one in every release anyway ... I don't have an objection to changing this in HEAD. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
