Re: [HACKERS] Interrupting long external library calls
On Fri, May 25, 2012 at 12:34:54PM -0400, Tom Lane wrote: > Sandro Santilli writes: > > I ended up providing an explicit mechanism to request interruption of > > whatever the library is doing, and experimented (successfully so far) > > requesting the interruption from a SIGINT handler. > > > Do you see any major drawback in doing so ? > > This seems a bit fragile. It might work all right in Postgres, where > we tend to set up signal handlers just once at process start, but ISTM > other systems might assume they can change their signal handlers at > any time. The handler itself looks less than portable anyway --- > what about the SIGINFO case? Indeed setting the handler from within the library itself was a temporary implementation to see how effective it would have been. The idea is to move the registration of the hanlder outside the library and into the user (PostGIS in this case). The library itself would only expose GEOS_requestInterrupt/GEOS_cancelInterrupt API calls. I'm guessing PostGIS should use the more portable pqsignal functions ? What should I know about SIGINFO ? > I assume that the geos::util::Interrupt::request() call sets a flag > somewhere that's going to be periodically checked in long-running > loops. Yes, this is what will happen. > Would it be possible for the periodic checks to include a > provision for a callback into Postgres-specific glue code, wherein > you could test the same flags CHECK_FOR_INTERRUPTS does? A similar > approach might then be usable in other contexts, and it seems safer > to me than messing with a host environment's signal handling. Would it be enough for the signal handler (installed by PostGIS) to check those flags and call the GEOS_requestInterrupt function when appropriate ? --strk; ,--o-. | __/ |Delivering high quality PostGIS 2.0 ! | / 2.0 |http://strk.keybit.net - http://vizzuality.com `-o--' -- 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 wrote: > Harshitha S 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 >
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] 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] --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] Synchronized scans versus relcache reinitialization
Noah Misch 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] 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] 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] 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] 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] [RFC] Interface of Row Level Security
Excerpts from Kohei KaiGai 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] 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] pg_upgrade libraries check
On 05/27/2012 02:32 PM, Tom Lane wrote: Andrew Dunstan 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] 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
Bruce Momjian 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] pg_upgrade libraries check
Andrew Dunstan 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
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
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 http://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: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 http://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 11:31:12AM -0400, Tom Lane wrote: > Bruce Momjian 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 http://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 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 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 http://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] No, pg_size_pretty(numeric) was not such a hot idea
On Sun, May 27, 2012 at 1:33 AM, Tom Lane wrote: > Fujii Masao writes: >> On Sat, May 26, 2012 at 9:30 AM, Tom Lane 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] VIP: new format for psql - shell - simple using psql in shell
Hello 2012/5/27 hubert depesz lubaczewski : > 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] 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
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 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 http://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] pg_receivexlog stops upon server restart
On Thursday, May 24, 2012, Thom Brown wrote: > On 24 May 2012 13:37, Magnus Hagander > > wrote: > > On Thu, May 24, 2012 at 2:34 PM, Thom Brown > > wrote: > >> On 24 May 2012 13:05, Magnus Hagander > > wrote: > >>> On Thu, Apr 19, 2012 at 1:00 PM, Thom Brown > > wrote: > On 10 April 2012 21:07, Magnus Hagander > > > 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_stat_statements temporary file
On Friday, May 25, 2012, Tom Lane wrote: > Andres Freund > writes: > > On Friday, May 25, 2012 04:03:49 PM Peter Geoghegan wrote: > >> Where do you suggest the file be written to? > > > One could argue stats_temp_directory would be the correct place. > > No, that would be exactly the *wrong* place, because that directory can > be on a RAM disk. We need to put this somewhere where it'll survive > a shutdown. > > One could imagine creating a PGDATA subdirectory just for permanent (not > temp) stats files, but right at the moment that seems like overkill. > If we accumulate a few more similar files, I'd start to think it was > worth doing. > That's pretty much what I was thinking. But yeah, at the time it's probably overkill - the main use today would be for better isolation of non-core extensions, but I'm not sure there are enough of those that want to write files in the data directory to care about.. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] pg_stat_statements temporary file
On Friday, May 25, 2012, Peter Geoghegan wrote: > On 25 May 2012 14:13, Magnus Hagander > > wrote: > > Here's a patch that does the two easy fixes: > > 1) writes the file to a temp file and rename()s it over the main file > > as it writes down. This removes the (small) risk of corruption because > > of a crash during write > > > > 2) unlinks the file after reading it. this makes sure it's not > > included in online backups. > > Seems reasonable. It might be better to consistently concatenate the > string literals PGSS_DUMP_FILE and ".tmp" statically. Also, I'd have > updated the string in the errmsg callsite after the "error" tag too, > to refer to the tmp file rather than the file proper. Forgive the > Agreed on the first one, and oops-forgot on the second one. > pedantry, but I should mention that I believe that it is project > policy to not use squiggly parenthesis following an if expression when > that is unnecessary due to there only being a single statement. > Good point too - I had some other code there as well during testing, and didn't clean it up all the way. Thanks for pointing it out! Will apply with those fixes. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
