Re: [HACKERS] pg_receivexlog stops upon server restart
On Thursday, May 24, 2012, Thom Brown wrote: On 24 May 2012 13:37, Magnus Hagander mag...@hagander.net javascript:; wrote: On Thu, May 24, 2012 at 2:34 PM, Thom Brown t...@linux.comjavascript:; wrote: On 24 May 2012 13:05, Magnus Hagander mag...@hagander.netjavascript:; wrote: On Thu, Apr 19, 2012 at 1:00 PM, Thom Brown t...@linux.comjavascript:; wrote: On 10 April 2012 21:07, Magnus Hagander mag...@hagander.netjavascript:; wrote: On Friday, April 6, 2012, Thom Brown wrote: Hi, I've tried out pg_receivexlog and have noticed that when restarting the cluster, pg_receivexlog gets cut off... it doesn't keep waiting. This is surprising as the DBA would have to remember to start pg_receivexlog up again. This is intentional as far as that's how the code was written, there's not a malfunctioning piece of code somewhere. It would probably make sense to have an auto-reconnect feature, and to have an option to turn it on/off. If you haven't already (my wifi here is currently quite useless, which is why I'm working on my email backlog, so I can't check), please add it to the open items list. I think it would also be useful to add a paragraph to the documentation stating use-cases for this feature, and its advantages. Attached is a patch that implements this. Seems reasonable? s/non fatal/non-fatal/ Yes, this solves the problem for me, except you forgot to translate noloop in long_options[] . :) Fixed :-) Did you test it, or just assumed it worked? ;) How very dare you. Of course I tested it. It successfully reconnects on multiple restarts, checks intermittently when I've stopped the server, showing the connection error message, successfully continues when I eventually bring the server back up, and doesn't attempt a reconnect when using -n. So looks good to me. Thanks - applied! -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] pg_upgrade libraries check
On Fri, May 25, 2012 at 11:08:10PM -0400, Bruce Momjian wrote: On Fri, May 25, 2012 at 10:20:29AM -0400, Andrew Dunstan wrote: pg_upgrade is a little over-keen about checking for shared libraries that back functions. In particular, it checks for libraries that support functions created in pg_catalog, even if pg_dump doesn't export the function. The attached patch mimics the filter that pg_dump uses for functions so that only the relevant libraries are checked. This would remove the need for a particularly ugly hack in making the 9.1 backport of JSON binary upgradeable. Andrew is right that pg_upgrade is overly restrictive in checking _any_ shared object file referenced in pg_proc. I never expected that pg_catalog would have such references, but in Andrew's case it does, and pg_dump doesn't dump them, so I guess pg_upgrade shouldn't check them either. In some sense this is a hack for the JSON type, but it also gives users a way to create shared object references in old clusters that are _not_ checked by pg_upgrade, and not migrated to the new server, so I suppose it is fine. OK, now I know it is _not_ fine. :-( I just realized the problem as part of debugging the report of a problem with plpython_call_handler(): http://archives.postgresql.org/pgsql-hackers/2012-03/msg01101.php http://archives.postgresql.org/pgsql-bugs/2012-05/msg00205.php The problem is that functions defined in the pg_catalog schema, while no explicitly dumped by pg_dump, are implicitly dumped by things like CREATE LANGUAGE plperl. I have added a pg_upgrade C comment documenting this issue in case we revisit it later. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/function.c b/contrib/pg_upgrade/function.c new file mode 100644 index e38071e..afa7543 *** a/contrib/pg_upgrade/function.c --- b/contrib/pg_upgrade/function.c *** get_loadable_libraries(void) *** 142,148 DbInfo *active_db = old_cluster.dbarr.dbs[dbnum]; PGconn *conn = connectToServer(old_cluster, active_db-db_name); ! /* Fetch all libraries referenced in this DB */ ress[dbnum] = executeQueryOrDie(conn, SELECT DISTINCT probin FROM pg_catalog.pg_proc --- 142,153 DbInfo *active_db = old_cluster.dbarr.dbs[dbnum]; PGconn *conn = connectToServer(old_cluster, active_db-db_name); ! /* ! * Fetch all libraries referenced in this DB. We can't exclude ! * the pg_catalog schema because, while such functions are not ! * explicitly dumped by pg_dump, they do reference implicit objects ! * that pg_dump does dump, e.g. creation of the plperl language. ! */ ress[dbnum] = executeQueryOrDie(conn, SELECT DISTINCT probin FROM pg_catalog.pg_proc -- 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] VIP: new format for psql - shell - simple using psql in shell
On Sat, May 26, 2012 at 05:39:23PM +0200, Pavel Stehule wrote: I proposed new psql's format shell. This format is optimized for processing returned result in shell: While I generally like the idea, please note that safe reading output from queries is possible, with COPY, and proper IFS, like: =$ psql -c select * from t a | b | c +-+--- a1 | b 2 | c|3 a +| b +| c:| 6 4 | 5 +| | | (2 rows) =$ psql -qAtX -c copy (select * from t) to stdout | while IFS=$'\t' read -r a b c; do echo -e a=[$a] b=[$b] c=[$c]; done a=[a1] b=[b 2] c=[c|3] a=[a 4] b=[b 5 ] c=[c:|6] that being said - I would love to get more functional psql. Best regards, depesz -- The best thing about modern society is how easy it is to avoid contact with it. http://depesz.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade libraries check
On 05/27/2012 06:40 AM, Bruce Momjian wrote: On Fri, May 25, 2012 at 11:08:10PM -0400, Bruce Momjian wrote: On Fri, May 25, 2012 at 10:20:29AM -0400, Andrew Dunstan wrote: pg_upgrade is a little over-keen about checking for shared libraries that back functions. In particular, it checks for libraries that support functions created in pg_catalog, even if pg_dump doesn't export the function. The attached patch mimics the filter that pg_dump uses for functions so that only the relevant libraries are checked. This would remove the need for a particularly ugly hack in making the 9.1 backport of JSON binary upgradeable. Andrew is right that pg_upgrade is overly restrictive in checking _any_ shared object file referenced in pg_proc. I never expected that pg_catalog would have such references, but in Andrew's case it does, and pg_dump doesn't dump them, so I guess pg_upgrade shouldn't check them either. In some sense this is a hack for the JSON type, but it also gives users a way to create shared object references in old clusters that are _not_ checked by pg_upgrade, and not migrated to the new server, so I suppose it is fine. OK, now I know it is _not_ fine. :-( I just realized the problem as part of debugging the report of a problem with plpython_call_handler(): http://archives.postgresql.org/pgsql-hackers/2012-03/msg01101.php http://archives.postgresql.org/pgsql-bugs/2012-05/msg00205.php The problem is that functions defined in the pg_catalog schema, while no explicitly dumped by pg_dump, are implicitly dumped by things like CREATE LANGUAGE plperl. I have added a pg_upgrade C comment documenting this issue in case we revisit it later. things like CREATE LANGUAGE plperl is a rather vague phrase. The PL case could be easily handled by adding this to the query: OR EXISTS (SELECT 1 FROM pg_catalog.pg_language WHERE lanplcallfoid = p.oid) Do you know of any other cases that this would miss? The fact is that unless we do something like this there is a potential for unnecessary pg_upgrade failures. The workaround I am currently using for the JSON backport of having to supply a dummy shared library is almost unspeakably ugly. If you won't consider changing the query, how about an option to explicitly instruct pg_upgrade to ignore a certain library in its checks? cheers andrew -- 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] VIP: new format for psql - shell - simple using psql in shell
Hello 2012/5/27 hubert depesz lubaczewski dep...@depesz.com: On Sat, May 26, 2012 at 05:39:23PM +0200, Pavel Stehule wrote: I proposed new psql's format shell. This format is optimized for processing returned result in shell: While I generally like the idea, please note that safe reading output from queries is possible, with COPY, and proper IFS, like: I newer say so it is impossible =$ psql -c select * from t a | b | c +-+--- a1 | b 2 | c|3 a +| b +| c:| 6 4 | 5 +| | | (2 rows) =$ psql -qAtX -c copy (select * from t) to stdout | while IFS=$'\t' read -r a b c; do echo -e a=[$a] b=[$b] c=[$c]; done a=[a1] b=[b 2] c=[c|3] a=[a 4] b=[b 5 ] c=[c:| 6] I know about this feature http://archives.postgresql.org/pgsql-hackers/2012-05/msg01169.php but may shell format patch is very simple and can really simplify usage in shell. that being said - I would love to get more functional psql. This patch doesn't break anything - and it is only 30 lines of non invasive simple code. Implementation of statements to psql is probably long task - I wrote prototype - but I have not time finish it and push to core. Regards Pavel Best regards, depesz -- The best thing about modern society is how easy it is to avoid contact with it. http://depesz.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] No, pg_size_pretty(numeric) was not such a hot idea
On Sun, May 27, 2012 at 1:33 AM, Tom Lane t...@sss.pgh.pa.us wrote: Fujii Masao masao.fu...@gmail.com writes: On Sat, May 26, 2012 at 9:30 AM, Tom Lane t...@sss.pgh.pa.us wrote: The argument for adding pg_size_pretty(numeric) was pretty darn thin in the first place, IMHO; it does not seem to me that it justified this loss of usability. Ouch! But removing pg_size_pretty(numeric) causes another usability issue, e.g., pg_size_pretty(pg_xlog_location_diff(...)) fails. So how about removing pg_size_pretty(bigint) to resolve those two issues? I guess pg_size_pretty(numeric) is a bit slower than bigint version, but I don't think that such a bit slowdown of pg_size_pretty() becomes a matter practically. No? AFAICS that argument is based on wishful thinking, not evidence. I did some simple measurements and determined that at least on my development machine, pg_size_pretty(numeric) is about a factor of four slower than pg_size_pretty(bigint) --- and that's just counting the function itself, not any added coercion-to-numeric processing. Now maybe you could argue that it's never going to be used in a context where anyone cares about its performance at all, but I've got doubts about that. OK, let me propose another approach: add pg_size_pretty(int). If we do this, all usability and performance problems will be solved. Thought? Attached patch adds pg_size_pretty(int). Regards, -- Fujii Masao size_pretty_int4_v1.patch Description: Binary data -- 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] pg_upgrade libraries check
On Sun, May 27, 2012 at 08:48:54AM -0400, Andrew Dunstan wrote: I just realized the problem as part of debugging the report of a problem with plpython_call_handler(): http://archives.postgresql.org/pgsql-hackers/2012-03/msg01101.php http://archives.postgresql.org/pgsql-bugs/2012-05/msg00205.php The problem is that functions defined in the pg_catalog schema, while no explicitly dumped by pg_dump, are implicitly dumped by things like CREATE LANGUAGE plperl. I have added a pg_upgrade C comment documenting this issue in case we revisit it later. things like CREATE LANGUAGE plperl is a rather vague phrase. The PL case could be easily handled by adding this to the query: OR EXISTS (SELECT 1 FROM pg_catalog.pg_language WHERE lanplcallfoid = p.oid) Do you know of any other cases that this would miss? The problem is I don't know. I don't know in what places we reference shared object files implicit but not explicitly, and I can't know what future places we might do this. The fact is that unless we do something like this there is a potential for unnecessary pg_upgrade failures. The workaround I am currently using for the JSON backport of having to supply a dummy shared library is almost unspeakably ugly. If you won't consider changing the query, how about an option to explicitly instruct pg_upgrade to ignore a certain library in its checks? The plpython_call_handler case I mentioned is a good example of a place where a pg_upgrade hack to map plpython to plpython2 has caused pg_upgrade check to succeed, but the actual pg_upgrade to fail --- certainly a bad thing, and something we would like to avoid. This kind of tinkering can easily cause such problems. We are not writing a one-off pg_upgrade for JSON-backpatchers here. If you want to create a new pg_upgrade binary with that hack, feel free to do so. Unless someone can explain a second use case for this, I am not ready to risk making pg_upgrade more unstable, and I don't think the community is either. I am not the person who decides if this gets added to pg_upgrade, but I am guessing what the community would want here. FYI, your fix would not address the plpython_call_handler problem because in that case we are actually dumping that function that references the shared object, and the pg_dumpall restore will fail. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] pg_upgrade libraries check
Bruce Momjian br...@momjian.us writes: On Sun, May 27, 2012 at 08:48:54AM -0400, Andrew Dunstan wrote: things like CREATE LANGUAGE plperl is a rather vague phrase. The PL case could be easily handled by adding this to the query: OR EXISTS (SELECT 1 FROM pg_catalog.pg_language WHERE lanplcallfoid = p.oid) Do you know of any other cases that this would miss? Well, laninline and lanvalidator for two ;-) The problem is I don't know. I don't know in what places we reference shared object files implicit but not explicitly, and I can't know what future places we might do this. The future changes argument seems like a straw man to me. We're already in the business of adjusting pg_upgrade when we make significant catalog changes. We are not writing a one-off pg_upgrade for JSON-backpatchers here. I tend to agree with that position, and particularly think that we should not allow the not-community-approved design of the existing JSON backport to drive changes to pg_upgrade. It would be better to ask first if there were a different way to construct that backport that would fit better with pg_upgrade. Having said that, I've got to also say that I think we've fundamentally blown it with the current approach to upgrading extensions. Because we dump all the extension member objects, the extension contents have got to be restorable into a new database version as-is, and that throws away most of the flexibility that we were trying to buy with the extension mechanism. IMO we have *got* to get to a place where both pg_dump and pg_upgrade dump extensions just as CREATE EXTENSION, and the sooner the better. Once we have that, this type of issue could be addressed by having different contents of the extension creation script for different major server versions --- or maybe even the same server version but different python library versions, to take something on-point for this discussion. For instance, Andrew's problem could be dealt with if the backport were distributed as an extension json-backport, and then all that's needed in a new installation is an empty extension script of that name. More generally, this would mean that cross-version compatibility problems for extensions could generally be solved in the extension scripts, and not with kluges in pg_upgrade. As things stand, you can be sure that kluging pg_upgrade is going to be the only possible fix for a very wide variety of issues. I don't recall exactly what problems drove us to make pg_upgrade do what it does with extensions, but we need a different fix for them. 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] pg_upgrade libraries check
On Sun, May 27, 2012 at 11:31:12AM -0400, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On Sun, May 27, 2012 at 08:48:54AM -0400, Andrew Dunstan wrote: things like CREATE LANGUAGE plperl is a rather vague phrase. The PL case could be easily handled by adding this to the query: OR EXISTS (SELECT 1 FROM pg_catalog.pg_language WHERE lanplcallfoid = p.oid) Do you know of any other cases that this would miss? Well, laninline and lanvalidator for two ;-) The problem is I don't know. I don't know in what places we reference shared object files implicit but not explicitly, and I can't know what future places we might do this. The future changes argument seems like a straw man to me. We're already in the business of adjusting pg_upgrade when we make significant catalog changes. The bottom line is I just don't understand the rules of when a function in the pg_catalog schema implicitly creates something that references a shared object, and unless someone tells me, I am inclined to just have pg_upgrade check everything and throw an error during 'check', rather than throw an error during the upgrade. If someone did tell me, I would be happy with modifying the pg_upgrade query to match. Also, pg_upgrade rarely requires adjustments for major version changes, and we want to keep it that way. We are not writing a one-off pg_upgrade for JSON-backpatchers here. I tend to agree with that position, and particularly think that we should not allow the not-community-approved design of the existing JSON backport to drive changes to pg_upgrade. It would be better to ask first if there were a different way to construct that backport that would fit better with pg_upgrade. Yep. A command-line flag just seems too user-visible for this use-case, and too error-pone. I barely understand what is going on, particularly with plpython in public (which we don't fully even understand yet), so adding a command-line flag seems like the wrong direction. Having said that, I've got to also say that I think we've fundamentally blown it with the current approach to upgrading extensions. Because we dump all the extension member objects, the extension contents have got to be restorable into a new database version as-is, and that throws away most of the flexibility that we were trying to buy with the extension mechanism. IMO we have *got* to get to a place where both pg_dump and pg_upgrade dump extensions just as CREATE EXTENSION, and the sooner the better. Once we have that, this type of issue could be addressed by having different contents of the extension creation script for different major server versions --- or maybe even the same server version but different python library versions, to take something on-point for this discussion. For instance, Andrew's problem could be dealt with if the backport were distributed as an extension json-backport, and then all that's needed in a new installation is an empty extension script of that name. More generally, this would mean that cross-version compatibility problems for extensions could generally be solved in the extension scripts, and not with kludges in pg_upgrade. As things stand, you can be sure that kludging pg_upgrade is going to be the only possible fix for a very wide variety of issues. I don't recall exactly what problems drove us to make pg_upgrade do what it does with extensions, but we need a different fix for them. Uh, pg_upgrade doesn't do anything special with extensions, so it must have been something people did in pg_dump. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] pg_upgrade libraries check
On Sun, May 27, 2012 at 12:08:14PM -0400, Bruce Momjian wrote: We are not writing a one-off pg_upgrade for JSON-backpatchers here. I tend to agree with that position, and particularly think that we should not allow the not-community-approved design of the existing JSON backport to drive changes to pg_upgrade. It would be better to ask first if there were a different way to construct that backport that would fit better with pg_upgrade. Yep. A command-line flag just seems too user-visible for this use-case, and too error-pone. I barely understand what is going on, particularly with plpython in public (which we don't fully even understand yet), so adding a command-line flag seems like the wrong direction. FYI, I recommend to Andrew that he just set probin to NULL for the JSON type in the old cluster before performing the upgrade. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] pg_upgrade libraries check
On 05/27/2012 11:31 AM, Tom Lane wrote: Having said that, I've got to also say that I think we've fundamentally blown it with the current approach to upgrading extensions. Because we dump all the extension member objects, the extension contents have got to be restorable into a new database version as-is, and that throws away most of the flexibility that we were trying to buy with the extension mechanism. IMO we have *got* to get to a place where both pg_dump and pg_upgrade dump extensions just as CREATE EXTENSION, and the sooner the better. Once we have that, this type of issue could be addressed by having different contents of the extension creation script for different major server versions --- or maybe even the same server version but different python library versions, to take something on-point for this discussion. For instance, Andrew's problem could be dealt with if the backport were distributed as an extension json-backport, and then all that's needed in a new installation is an empty extension script of that name. It sounds nice, but we'd have to make pg_upgrade drop its current assumption that libraries wanted in the old version will be named the same (one for one) as the libraries wanted in the new version. Currently it looks for every shared library named in probin (other than plpgsql.so) in the old cluster and tries to LOAD it in the new cluster, and errors out if it can't. My current unspeakably ugly workaround for this behaviour is to supply a dummy library for the new cluster. The only other suggestion I have heard (from Bruce) to handle this is to null out the relevant probin entries before doing the upgrade. I'm not sure if that's better or worse. It is certainly just about as ugly. So pg_upgrade definitely needs to get a lot smarter IMNSHO. cheers andrew -- 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] pg_upgrade libraries check
On Sun, May 27, 2012 at 12:31:09PM -0400, Andrew Dunstan wrote: On 05/27/2012 11:31 AM, Tom Lane wrote: Having said that, I've got to also say that I think we've fundamentally blown it with the current approach to upgrading extensions. Because we dump all the extension member objects, the extension contents have got to be restorable into a new database version as-is, and that throws away most of the flexibility that we were trying to buy with the extension mechanism. IMO we have *got* to get to a place where both pg_dump and pg_upgrade dump extensions just as CREATE EXTENSION, and the sooner the better. Once we have that, this type of issue could be addressed by having different contents of the extension creation script for different major server versions --- or maybe even the same server version but different python library versions, to take something on-point for this discussion. For instance, Andrew's problem could be dealt with if the backport were distributed as an extension json-backport, and then all that's needed in a new installation is an empty extension script of that name. It sounds nice, but we'd have to make pg_upgrade drop its current assumption that libraries wanted in the old version will be named the same (one for one) as the libraries wanted in the new version. Currently it looks for every shared library named in probin (other than plpgsql.so) in the old cluster and tries to LOAD it in the new cluster, and errors out if it can't. I didn't fully understand this. Are you saying pg_upgrade will check some extension config file for the library name? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] pg_upgrade libraries check
On 05/27/2012 12:50 PM, Bruce Momjian wrote: On Sun, May 27, 2012 at 12:31:09PM -0400, Andrew Dunstan wrote: On 05/27/2012 11:31 AM, Tom Lane wrote: Having said that, I've got to also say that I think we've fundamentally blown it with the current approach to upgrading extensions. Because we dump all the extension member objects, the extension contents have got to be restorable into a new database version as-is, and that throws away most of the flexibility that we were trying to buy with the extension mechanism. IMO we have *got* to get to a place where both pg_dump and pg_upgrade dump extensions just as CREATE EXTENSION, and the sooner the better. Once we have that, this type of issue could be addressed by having different contents of the extension creation script for different major server versions --- or maybe even the same server version but different python library versions, to take something on-point for this discussion. For instance, Andrew's problem could be dealt with if the backport were distributed as an extension json-backport, and then all that's needed in a new installation is an empty extension script of that name. It sounds nice, but we'd have to make pg_upgrade drop its current assumption that libraries wanted in the old version will be named the same (one for one) as the libraries wanted in the new version. Currently it looks for every shared library named in probin (other than plpgsql.so) in the old cluster and tries to LOAD it in the new cluster, and errors out if it can't. I didn't fully understand this. Are you saying pg_upgrade will check some extension config file for the library name? AIUI, for Tom's scheme to work pg_upgrade would need not to check libraries that belonged to extension functions. Currently it simply assumes that what is present in the old cluster is exactly what will be needed in the new cluster. Tom's proposed scheme would completely invalidate that assumption (which I would argue is already of doubtful validity). Instead of explicitly trying to LOAD the libraries it would have to rely on the CREATE EXTENSION foo failing, presumably at some time before it would be too late to abort. cheers andrew -- 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] pg_upgrade libraries check
Andrew Dunstan and...@dunslane.net writes: AIUI, for Tom's scheme to work pg_upgrade would need not to check libraries that belonged to extension functions. Currently it simply assumes that what is present in the old cluster is exactly what will be needed in the new cluster. Tom's proposed scheme would completely invalidate that assumption (which I would argue is already of doubtful validity). Instead of explicitly trying to LOAD the libraries it would have to rely on the CREATE EXTENSION foo failing, presumably at some time before it would be too late to abort. Well, the scheme I had in mind would require pg_upgrade to verify that the new cluster contains an extension control file for each extension in the old cluster (which is something it really oughta do anyway, if it doesn't already). After that, though, it ought not be looking at the individual functions defined by an extension --- it is the extension's business which libraries those are in. The only reason for pg_upgrade to still look at probin at all would be to cover C functions that weren't within extensions. In the long run it might be possible to consider those unsupported, or at least not common enough to justify a safety check in pg_upgrade. 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] pg_upgrade libraries check
Bruce Momjian br...@momjian.us writes: On Sun, May 27, 2012 at 11:31:12AM -0400, Tom Lane wrote: I don't recall exactly what problems drove us to make pg_upgrade do what it does with extensions, but we need a different fix for them. Uh, pg_upgrade doesn't do anything special with extensions, so it must have been something people did in pg_dump. Most of the dirty work is in pg_dump --binary_upgrade, but it's pretty disingenuous of you to disavow responsibility for those kluges. They are part and parcel of pg_upgrade IMO. 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] 9.2beta1, parallel queries, ReleasePredicateLocks, CheckForSerializableConflictIn in the oprofile
Hi, I did another test using the same data and the same code, which I've provided before and the performance of the single thread seems to be degrading quadratically with the number of threads. Here are the results: Nthreads Time_to_execute_one_thread 1 8.1 2 7.8 3 8.1 4 9.0 5 10.2 6 11.4 7 13.3 8 16.1 9 19.0 10 21.4 11 23.8 12 27.3 13 30.2 14 32.0 15 34.1 16 37.5 Regards, Sergey On Sat, 26 May 2012, Stephen Frost wrote: * Sergey Koposov (kopo...@ast.cam.ac.uk) wrote: Turning off synch seq scans doesn't help either. 18 sec multithreaded run vs 7 sec single threaded. Alright, can you just time 'cat' when they're started a few seconds or whatever apart from each other? I can't imagine it being affected in the same way as these, but seems like it wouldn't hurt to check. Thanks, Stephen * Sergey E. Koposov, PhD, Research Associate Institute of Astronomy, University of Cambridge Madingley road, CB3 0HA, Cambridge, UK Tel: +44-1223-337-551 Web: http://www.ast.cam.ac.uk/~koposov/ -- 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] pg_upgrade libraries check
On 05/27/2012 02:32 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: AIUI, for Tom's scheme to work pg_upgrade would need not to check libraries that belonged to extension functions. Currently it simply assumes that what is present in the old cluster is exactly what will be needed in the new cluster. Tom's proposed scheme would completely invalidate that assumption (which I would argue is already of doubtful validity). Instead of explicitly trying to LOAD the libraries it would have to rely on the CREATE EXTENSION foo failing, presumably at some time before it would be too late to abort. Well, the scheme I had in mind would require pg_upgrade to verify that the new cluster contains an extension control file for each extension in the old cluster (which is something it really oughta do anyway, if it doesn't already). After that, though, it ought not be looking at the individual functions defined by an extension --- it is the extension's business which libraries those are in. Yeah, that makes sense. cheers andrew -- 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] Per-Database Roles
On tis, 2012-05-22 at 10:19 -0400, Robert Haas wrote: I think we should have made roles and tablespaces database objects rather than shared objects, User names are global objects in the SQL standard, which is part of the reason that the current setup was never seriously challenged. -- 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] [RFC] Interface of Row Level Security
Excerpts from Kohei KaiGai kai...@kaigai.gr.jp wrote on Fri, May 25, 2012 at 11:08 PM: If we assume RLS is applied when user has no privileges on tables, the current ExecCheckRTEPerms() always raises an error towards unprivileged users, prior to execution of queries. Isn't it preferable behavior to allow unprivileged users to reference a table (or columns) when it has RLS policy? Rather than assuming the the RLS checks will be applied when there are no privileges it would make sense to regard RLS as a limitation on the scope of a particular privilege. This makes RLS a property of a particular grant of a privilege rather than of the table. Viewed this way it is possible to control which users are subject to restrictions imposed by the RLS function, which will avoid RLS overhead for operations which don't require it while allowing checks for those that do, provide a mechanism exempting object owners from RLS checks and make it possible to avoid pg_dump calling user defined code. This suggests an alternative declaration syntax, to use the salesman example: GRANT SELECT ON TABLE client_detail TO super_sales_group; GRANT SELECT TABLE ON client_detail TO limited_sales_group WITH (QUALIFICATION FUNCTION sales_view_check); and since more easily usable security features will be used more effectively, a possible future inlining of the condition: GRANT SELECT ON TABLE client_detail TO limited_sales_group WHERE sales_rep = my_sales_rep(); Regards, Alastair. -- 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] Backends stalled in 'startup' state: index corruption
I've been continuing to poke at this business of relcache-related startup stalls, and have come to some new conclusions. One is that it no longer seems at all likely that the pg_attribute rows for system catalogs aren't at the front of pg_attribute, because the commands that might be used to update them are disallowed even to superusers: foo=# alter table pg_index alter column indnatts set statistics -1; ERROR: permission denied: pg_index is a system catalog foo=# alter table pg_index alter column indnatts set (n_distinct = -1); ERROR: permission denied: pg_index is a system catalog Now a sufficiently stubborn DBA could override that protection (via direct manual UPDATE on pg_attribute, or by doing ALTER TABLE in a standalone backend with allowSystemTableMods set). But it doesn't seem like something anybody would try in a production database. So while I'd still like to see the results of that ctid query in Jeff's broken database, it seems pretty unlikely that we will see high block numbers. But, if all those rows are at the front of pg_attribute, how do we explain the high block numbers seen in Jeff's stack traces? He posted three traces that show RelationBuildTupleDesc doing heapscans: http://archives.postgresql.org/pgsql-bugs/2012-04/msg00186.php http://archives.postgresql.org/pgsql-bugs/2012-04/msg00196.php http://archives.postgresql.org/pgsql-bugs/2012-04/msg00197.php and in each one of these, the block number being passed to ReadBufferExtended is well past where those rows ought to be. What's more, the three traces are performing the pg_attribute heapscan for three different catalogs; so to blame this observation on some rows having gotten relocated to high block numbers, you'd have to believe that had happened for all three of these catalogs (one of which is an index, so there'd be no reason to fool with its stats attributes anyway). AFAICS, if there are not rows at high block numbers, the only other possibility is that syncscan mode was active. (A third explanation is that some rows are just plain missing, but then the system would have been erroring out altogether, which it was not.) That theory requires us to invent an explanation for the fact that Jeff now observes pg_attribute to be slightly smaller than 1/4th shared_buffers, but perhaps autovacuum managed to shrink it at some point after the trouble happened. Another thing that can be deduced from those stack traces is that sinval resets were happening. For example, in the third message linked above, the heapscan is being done to load up a relcache entry for relation 2601 (pg_am). This would be unsurprising, except that stack frames 17 and 18 show this is happening during the fifth call of load_critical_index, and we should have had both pg_am's reldesc and the syscache entry that's being fetched at relcache.c:1010 loaded up in the first call. So those cache entries have to have gotten blown away by sinval reset. This is not terribly surprising if there were a steady flux of DDL happening in the database, for instance temp table creations/drops. (Jeff, can you comment on that?) If heapscans over the whole of pg_attribute were occurring, they'd take long enough to expose the process to sinval overrun even with not-very-high DDL rates. Now the interesting thing about that is that if an sinval reset occurs at any time while relcache.c is trying to load up initial relcache entries, it will not try to write a new relcache init file. This means that a steady but not very large flow of DDL activity from existing sessions in the database would be sufficient to prevent incoming sessions from ever writing pg_internal.init, and thus the stall behavior could persist for a long time, which is something I hadn't been able to explain before. (If this were not happening, the stalls would disappear as soon as any incoming session successfully got through its stall.) So it appears to me that Jeff's problem is adequately explained if we assume (1) pg_attribute a bit larger than 1/4th shared_buffers, and (2) steady flow of DDL activity from existing sessions; and that if you want to dispute (1) you have to invent some other explanation for the observed attempts to read high block numbers in pg_attribute. Also, if there were lots of temp table creation/dropping going on, this could result in lots of dead rows at the end of pg_attribute, which might explain how autovacuum could've shortened the relation later. Now this theory does not explain Greg's problem, because his pg_attribute was apparently many times too small to make syncscans credible, or to make them take very long even if they did happen. But in view of the fact that that was 8.3.5 :-(, and that the problem was observed in conjunction with corruption of system indexes, I don't feel a compulsion to assume that Greg's problem was the same as Jeff's. Trawling through the commit logs I find several post-8.3.5 fixes for race conditions and other bugs in relcache initialization,
Re: [HACKERS] No, pg_size_pretty(numeric) was not such a hot idea
On 27-05-2012 10:45, Fujii Masao wrote: OK, let me propose another approach: add pg_size_pretty(int). If we do this, all usability and performance problems will be solved. I wouldn't like to add another function but if it solves both problems... +1. -- Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- 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] Bug in new buffering GiST build code
On Sat, May 26, 2012 at 12:33 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I think we should rewrite the way we track the parents completely. Rather than keep a path stack attached to every node buffer, let's just maintain a second hash table that contains the parent of every internal node. Whenever a downlink is moved to another page, update the hash table with the new information. That way we always have up-to-date information about the parent of every internal node. That's much easier to understand than the path stack structures we have now. I think the overall memory consumption will be about the same too. Although we need the extra hash table with one entry for every internal node, we get rid of the path stack structs, which are also one per every internal node at the moment. I believe it is faster too. I added some instrumentation to the existing gist code (with the additional getNodeBuffer() call added to fix this bug), to measure the time spent moving right, when refinding the parent of a page. I added gettimeofday() calls before and after moving right, and summed the total. In my test case, the final index size was about 19GB, and the index build took 3545 seconds (59 minutes). Of that time, 580 seconds (~ 10 minutes) was spent moving right to refind parents. That's a lot. I also printed a line whenever a refind operation had to move right 20 pages or more. That happened 2482 times during the build, in the worst case we moved right over 4 pages. Attached is a patch to replace the path stacks with a hash table. With this patch, the index build time in my test case dropped from 59 minutes to about 55 minutes. I don'ẗ know how representative or repeatable this test case is, but this definitely seems very worthwhile, not only because it fixes the bug and makes the code simpler, but also on performance grounds. Cool, seems that we've both simplier and faster implementation of finding parent. Thanks! Alexander, do you still have the test environments and data lying around that you used for GiST buffering testing last summer? Could you rerun some of those tests with this patch? I think I can restore test environment and data. Will rerun tests soon. -- With best regards, Alexander Korotkov.
Re: [HACKERS] Synchronized scans versus relcache reinitialization
On Sat, May 26, 2012 at 03:14:18PM -0400, Tom Lane wrote: It seems clear to me that we should just disable syncscans for the relcache reload heapscans. There is lots of downside due to breaking the early-exit optimization in RelationBuildTupleDesc, and basically no upside. I'm inclined to just modify systable_beginscan to prevent use of syncscan whenever indexOK is false. If we wanted to change its API we could make this happen only for RelationBuildTupleDesc's calls, but I don't see any upside for allowing syncscans for other forced-heapscan callers either. Looks harmless enough, though it's only targeting a symptom. No matter how you cut it, the system is in a bad state when many backends simultaneously heapscan a large system catalog. 2. The larger problem here is that when we have N incoming connections we let all N of them try to rebuild the init file independently. This doesn't make things faster for any one of them, and once N gets large enough it makes things slower for all of them. We would be better off letting the first arrival do the rebuild work while the others just sleep waiting for it. I believe that this fix would probably have ameliorated Jeff and Greg's cases, even though those do not seem to have triggered the syncscan logic. This strikes me as the clearer improvement; it fixes the root cause. Thanks, nm -- 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] Synchronized scans versus relcache reinitialization
Noah Misch n...@leadboat.com writes: On Sat, May 26, 2012 at 03:14:18PM -0400, Tom Lane wrote: It seems clear to me that we should just disable syncscans for the relcache reload heapscans. There is lots of downside due to breaking the early-exit optimization in RelationBuildTupleDesc, and basically no upside. I'm inclined to just modify systable_beginscan to prevent use of syncscan whenever indexOK is false. If we wanted to change its API we could make this happen only for RelationBuildTupleDesc's calls, but I don't see any upside for allowing syncscans for other forced-heapscan callers either. Looks harmless enough, though it's only targeting a symptom. No matter how you cut it, the system is in a bad state when many backends simultaneously heapscan a large system catalog. Agreed, but actually this isn't just a symptom: the syncscan code is *causing* full-table heapscans that would not occur otherwise. 2. The larger problem here is that when we have N incoming connections we let all N of them try to rebuild the init file independently. This doesn't make things faster for any one of them, and once N gets large enough it makes things slower for all of them. We would be better off letting the first arrival do the rebuild work while the others just sleep waiting for it. I believe that this fix would probably have ameliorated Jeff and Greg's cases, even though those do not seem to have triggered the syncscan logic. This strikes me as the clearer improvement; it fixes the root cause. As I noted in the other thread, I've had second thoughts about this proposal: it would serialize incoming sessions even in cases where no benefit would be gained. Given the lack of previous reports I'm inclined to think that fixing the misapplication of syncscan logic should be enough to cure the problem, and hence we shouldn't take a risk of de-optimizing behavior that has generally worked fine for the last fifteen years. 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] --disable-shared is entirely broken these days
On lör, 2012-05-26 at 15:53 -0400, Tom Lane wrote: 2. Seeing that this is the first complaint since 9.0, should we decide that --disable-shared is no longer worth supporting? Seems like we should either make this case work or remove this switch. I think the last remaining use was the QNX port, which didn't support shared libraries. We should just remove it 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] Backends stalled in 'startup' state: index corruption
On May 27, 2012, at 12:53 PM, Tom Lane wrote: Another thing that can be deduced from those stack traces is that sinval resets were happening. For example, in the third message linked above, the heapscan is being done to load up a relcache entry for relation 2601 (pg_am). This would be unsurprising, except that stack frames 17 and 18 show this is happening during the fifth call of load_critical_index, and we should have had both pg_am's reldesc and the syscache entry that's being fetched at relcache.c:1010 loaded up in the first call. So those cache entries have to have gotten blown away by sinval reset. This is not terribly surprising if there were a steady flux of DDL happening in the database, for instance temp table creations/drops. (Jeff, can you comment on that?) If heapscans over the whole of pg_attribute were occurring, they'd take long enough to expose the process to sinval overrun even with not-very-high DDL rates. As it turns out, there are quite a few temporary tables created. During the busiest periods of the day, this system averages 1.75 temp tables per second.
Re: [HACKERS] [RFC] Interface of Row Level Security
On Thu, May 24, 2012 at 07:31:37PM +0200, Florian Pflug wrote: On May24, 2012, at 18:42 , Kohei KaiGai wrote: As we discussed, it causes a problem with approach to append additional qualifiers to where clause implicitly, because it does not solve the matter corresponding to the order to execute qualifiers. So, I'm inclined to the approach to replace reference to tables with security policy by sub-queries with security barrier flag. Since the security barrier flag carries a potentially hefty performance penalty, I think it should be optional. Application which don't allow SQL-level access to the database might still benefit from row-level security, because it saves them from having to manually add the WHERE clause to every statement, or having to wrap all their tables with views. Yet without direct SQL-level access, the security barrier thing isn't really necessary, so it'd be nice if they wouldn't have to pay for it. How about ALTER TABLE ? SET ROW POLICY ? WITH (security_barrier) The conventional case for a RLS facility is to wholly implement a security policy, so security_barrier should be the default. Using the same facility to implement a security policy in cooperation with a trusted query generator is the variant case. Backward compatibility concerns limited our options when retrofitting the security_barrier treatment for views, but I'd rather not add a knob completely disabling it in the context of a brand new feature. A better avenue is to enhance our facilities for identifying safe query fragments. For example, ALTER FUNCTION ... LEAKPROOF is superuser-only. Adding a way for a table owner to similarly trust functions for the purpose of his own tables would help close the gap that motivates such an all-or-nothing knob. Thanks, nm -- 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] proclock table corrupted
Sorry, the OS is WindRiver Linux. Yes , I am taking of the fast path locking patch discussed in the link below. http://postgresql.1045698.n5.nabble.com/bug-in-fast-path-locking-td5626629.html Regards, Harshitha On Fri, May 25, 2012 at 7:09 PM, Tom Lane t...@sss.pgh.pa.us wrote: Harshitha S hershe...@gmail.com writes: We are encoutering the following error during normal operation of postgres. postgres[10982]: [2-1] PANIC: proclock table corrupted Ugh. Can you provide a reproducible test case? Version of Postgres : 9.0.3 Architecture : mips OS: RedHat Linux [ raised eyebrow... ] I've been working at Red Hat for ten years, and I'm pretty sure they have never shipped a MIPS-based distro in that time. So what is that OS really? Can you please let me know if 'fix-strong-lock-cleanup.patch' and this error are related? This is not an adequate identification of what patch you are talking about; but if you are speaking of something related to Robert Haas' fast-path locking code, that's not in 9.0.x. regards, tom lane
