Re: [HACKERS] ALTER EXTENSION UPGRADE, v3
Josh Berkus j...@agliodbs.com writes: How *are* we detecting which version is installed, anyway? Is that in the pg_extenstions table? The installed version is in the pg_extenstion catalog, the version we're upgrading to is in the control file and can be seen in the system view pg_available_extensions or from the system SRF named the same: ~:5490=# \dx List of extensions Schema | Name| Version | Description +---+--+- pg_catalog | adminpack | 9.1devel | Administrative functions for PostgreSQL public | lo| 9.1devel | managing Large Objects (2 rows) ~:5490=# select oid, * from pg_extension ; oid | extname | extnamespace | relocatable | extversion ---+---+--+-+ 16385 | lo| 2200 | t | 9.1devel 16406 | adminpack | 11 | f | 9.1devel (2 rows) ~:5490=# select schema, name, installed, version from pg_available_extensions limit 10; schema |name| installed | version ++---+-- public | lo | 9.1devel | 9.1devel pg_catalog | adminpack | 9.1devel | 9.1devel | citext | | 9.1devel | chkpass| | 9.1devel | cube | | 9.1devel | pg_stat_statements | | 9.1devel | pg_buffercache | | 9.1devel | dict_xsyn | | 9.1devel | earthdistance | | 9.1devel | xml2 | | 9.1devel (10 rows) So every package would include a script called upgrade.sql ( or upgrade.c? ) which is supposed to handle the upgrade, and it's up to the module author to power that, at least until 9.2? Seem like the most reasonable course for February ... Yeah. Of course you want to support shipping upgrade files for more than one upgrade situation, that's in my proposal and patch too. The extension author just have to fill in the control file with an upgrade setup: regexp against installed version string = upgrade script to use. And that's about it. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compilation failed
On Fri, Feb 4, 2011 at 06:19, Robert Haas robertmh...@gmail.com wrote: On Fri, Feb 4, 2011 at 12:11 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Feb 3, 2011 at 11:42 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Feb 3, 2011 at 11:02 PM, Fujii Masao masao.fu...@gmail.com wrote: Hi, When I run git pull from the git master and make, I encountered the following compilation error. gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wformat-security -fno-strict-aliasing -fwrapv -g -I../../src/port -I../../src/include -D_GNU_SOURCE -c chklocale.c -o chklocale_srv.o In file included from ../../src/include/postgres.h:48, from chklocale.c:17: ../../src/include/utils/elog.h:69:28: error: utils/errcodes.h: No such file or directory make[2]: *** [chklocale_srv.o] Error 1 make[2]: Leaving directory `/home/postgres/head/src/port' make[1]: *** [install-port-recurse] Error 2 make[1]: Leaving directory `/home/postgres/head/src' make: *** [install-src-recurse] Error 2 I guess the following commit causes a problem. - Avoid maintaining three separate copies of the error codes list. src/pl/plpgsql/src/plerrcodes.h, src/include/utils/errcodes.h, and a big chunk of errcodes.sgml are now automatically generated from a single file, src/backend/utils/errcodes.txt. - The build farm doesn't look too happy with it either, but of course it worked for me here. I guess there's a missing dependency somewhere. *goes off to look* I just pushed some fixes to unbreak the VPATH build (I hope) but I don't see exactly what's going wrong to cause you the problem shown above. Can you try with a completely fresh tree and send the whole build log if it's still failing? MSVC isn't happy with this either: Writing fmgroids.h Writing fmgrtab.c Generating probes.h... Generating errcodes.h... The filename, directory name, or volume label syntax is incorrect. Could not open src\backend\utils\errcodes.h at src/tools/msvc/Mkvcbuild.pm line 463 I can't immediately grok what I need to do to fix that. I think it's because it's using double quotes and then backslashes in the filenames, which will apply escaping. I've changed it to single quotes and hope that will fix it - the firewall at the airport here is blocking my outbound RDP so I can't verify it now - if it doesn't work, I'll take another look later. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] exposing COPY API
Here is a demonstration to support jagged input files. It's a patch on the latest patch. The new added API is: bool NextLineCopyFrom( [IN] CopyState cstate, [OUT] char ***fields, [OUT] int *nfields, [OUT] Oid *tupleOid) It just returns separated fields in the next line. Fortunately, I need no extra code for it because it is just extracted from NextCopyFrom(). I'm willing to include the change into copy APIs, but we still have a few issues. See below. On Fri, Feb 4, 2011 at 16:53, Andrew Dunstan and...@dunslane.net wrote: The problem with COPY FROM is that nobody's come up with a good syntax for allowing it as a FROM target. Doing what I want via FDW neatly gets us around that problem. But I'm quite OK with doing the hard work inside the COPY code - that's what my working prototype does in fact. I think it is not only syntax issue. I found an issue that we hard to support FORCE_NOT_NULL option for extra fields. See FIXME in the patch. It is a fundamental problem to support jagged fields. One thing I'd like is to to have file_fdw do something we can't do another way. currently it doesn't, so it's nice but uninteresting. BTW, how do you determine which field is shifted in your broken CSV file? For example, the case you find AB,CD,EF for 2 columns tables. I could provide a raw CSV reader for jagged files, but you still have to cook the returned fields into a proper tuple... -- Itagaki Takahiro jagged_csv_api-20110204.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] SSI patch version 14
On 02.02.2011 19:36, Kevin Grittner wrote: I really appreciate you putting this testing framework together. This is clearly the right way to do it, although we also clearly don't want this test in the check target at the root directory, because of the run time. It would be nice to get some buildfarm members to run them though. I'm reading through the patch again now, hoping to commit this weekend. There's still this one TODO item that you commented on earlier: Then there's one still lurking outside the new predicate* files, in heapam.c. It's about 475 lines into the heap_update function (25 lines from the bottom). In reviewing where we needed to acquire predicate locks, this struck me as a place where we might need to duplicate the predicate lock from one tuple to another, but I wasn't entirely sure. I tried for a few hours one day to construct a scenario which would show a serialization anomaly if I didn't do this, and failed create one. If someone who understands both the patch and heapam.c wants to look at this and offer an opinion, I'd be most grateful. I'll take another look and see if I can get my head around it better, but failing that, I'm disinclined to either add locking I'm not sure we need or to remove the comment that says we *might* need it there. Have you convinced yourself that there's nothing to do? If so, we should replace the todo comment with a comment explaining why. PageIsPredicateLocked() is currently only called by one assertion in RecordFreeIndexPage(). The comment in PageIsPredicateLocked() says something about gist vacuuming; I presume this is something that will be needed to implement more fine-grained predicate locking in GiST. But we don't have that at the moment. The assertion in RecordFreeIndexPage() is a reasonable sanity check, but I'm inclined to move it to the caller instead: I don't think the FSM should need to access predicate lock manager, even for an assertion. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] We need to log aborted autovacuums
On Fri, Feb 4, 2011 at 12:59 AM, Josh Berkus j...@agliodbs.com wrote: Robert, Seeing as how there seem to be neither objections nor endorsements, I'm inclined to commit what I proposed more or less as-is. There remains the issue of what do about the log spam. Josh Berkus suggested logging it when log_autovacuum_min_duration != -1, which seems like a bit of an abuse of that setting, but it's certainly not worth adding another setting for, and the alternative of logging it at, say, DEBUG2 seems unappealing because you'll then have to turn on logging for a lot of unrelated crap to get this information. So on balance I think that proposal is perhaps the least of evils. Wait, which proposal is? I meant to say that I think the least evil proposal is to log at LOG when log_autovacuum_min_duration != -1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Compilation failed
On Fri, Feb 4, 2011 at 6:39 AM, Fujii Masao masao.fu...@gmail.com wrote: When I ran make before make install, both successfully finished. So I guess that the make install rule in Makefile might be corrupted. Ah - that's the key. Not corrupted, but missing a dependency. Try it now... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] [GENERAL] Issues with generate_series using integer boundaries
On 3 February 2011 13:58, Thom Brown t...@linux.com wrote: On 3 February 2011 13:32, Thom Brown t...@linux.com wrote: Actually, further testing indicates this causes other problems: postgres=# SELECT x FROM generate_series(1, 9,-1) AS a(x); x --- 1 (1 row) Should return no rows. postgres=# SELECT x FROM generate_series(1, 9,3) AS a(x); x 1 4 7 10 (4 rows) Should return 3 rows. Still messy code, but the attached patch does the job now: postgres=# SELECT x FROM generate_series(2147483643::int4,2147483647::int4) AS a(x); x 2147483643 2147483644 2147483645 2147483646 2147483647 (5 rows) postgres=# SELECT x FROM generate_series(2147483642::int4,2147483647::int4, 2) AS a(x); x 2147483642 2147483644 2147483646 (3 rows) postgres=# SELECT x FROM generate_series(2147483643::int4,2147483647::int4, 6) AS a(x); x 2147483643 (1 row) postgres=# SELECT x FROM generate_series((-2147483643)::int4, (-2147483648)::int4, -1) AS a(x); x - -2147483643 -2147483644 -2147483645 -2147483646 -2147483647 -2147483648 (6 rows) postgres=# SELECT x FROM generate_series(1, 9,-1) AS a(x); x --- (0 rows) postgres=# SELECT x FROM generate_series(1, 9,3) AS a(x); x --- 1 4 7 (3 rows) Copying to -hackers. The issue is that generate_series will not return if the series hits either the upper or lower boundary during increment, or goes beyond it. The attached patch fixes this behaviour, but should probably be done a better way. The first 3 examples above will not return. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 generate_series_fix.v3.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] Compilation failed
On Fri, Feb 4, 2011 at 9:07 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Feb 4, 2011 at 6:39 AM, Fujii Masao masao.fu...@gmail.com wrote: When I ran make before make install, both successfully finished. So I guess that the make install rule in Makefile might be corrupted. Ah - that's the key. Not corrupted, but missing a dependency. Try it now... Thanks! The problem disappeared from my machine. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] Compilation failed
On Fri, Feb 4, 2011 at 8:07 AM, Fujii Masao masao.fu...@gmail.com wrote: On Fri, Feb 4, 2011 at 9:07 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Feb 4, 2011 at 6:39 AM, Fujii Masao masao.fu...@gmail.com wrote: When I ran make before make install, both successfully finished. So I guess that the make install rule in Makefile might be corrupted. Ah - that's the key. Not corrupted, but missing a dependency. Try it now... Thanks! The problem disappeared from my machine. Glad to hear it. But the more I like at this, the more screwed up it seems. make distprep does the wrong thing, and there are some other problems too. Working on it... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] SSI patch version 14
Heikki Linnakangas wrote: On 02.02.2011 19:36, Kevin Grittner wrote: I really appreciate you putting this testing framework together. This is clearly the right way to do it, although we also clearly don't want this test in the check target at the root directory, because of the run time. It would be nice to get some buildfarm members to run them though. Maybe it could be included in the installcheck or installcheck-world target? There's still this one TODO item that you commented on earlier: Then there's one still lurking outside the new predicate* files, in heapam.c. It's about 475 lines into the heap_update function (25 lines from the bottom). In reviewing where we needed to acquire predicate locks, this struck me as a place where we might need to duplicate the predicate lock from one tuple to another, but I wasn't entirely sure. I tried for a few hours one day to construct a scenario which would show a serialization anomaly if I didn't do this, and failed create one. If someone who understands both the patch and heapam.c wants to look at this and offer an opinion, I'd be most grateful. I'll take another look and see if I can get my head around it better, but failing that, I'm disinclined to either add locking I'm not sure we need or to remove the comment that says we *might* need it there. Have you convinced yourself that there's nothing to do? If so, we should replace the todo comment with a comment explaining why. I'll look at that today and tomorrow and try to resolve the issue one way or the other. PageIsPredicateLocked() is currently only called by one assertion in RecordFreeIndexPage(). The comment in PageIsPredicateLocked() says something about gist vacuuming; I presume this is something that will be needed to implement more fine-grained predicate locking in GiST. But we don't have that at the moment. Yeah. We had something close at one point which we had to back out because it didn't cover page splits correctly in all cases. It's a near-certainty that we can have fine-grained coverage for the GiST AM covered with a small patch in 9.2, and I'm pretty sure we'll need that function for it. The assertion in RecordFreeIndexPage() is a reasonable sanity check, but I'm inclined to move it to the caller instead: I don't think the FSM should need to access predicate lock manager, even for an assertion. OK. So it looks like right now it will move to btvacuumpage(), right before the call to RecordFreeIndexPage(), and we will need to add it to one spot each in the GiST and GIN AMs, when we get to those. Would you like me to do that? -Kevin -- 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] exposing COPY API
On 02/04/2011 05:49 AM, Itagaki Takahiro wrote: Here is a demonstration to support jagged input files. It's a patch on the latest patch. The new added API is: bool NextLineCopyFrom( [IN] CopyState cstate, [OUT] char ***fields, [OUT] int *nfields, [OUT] Oid *tupleOid) It just returns separated fields in the next line. Fortunately, I need no extra code for it because it is just extracted from NextCopyFrom(). Thanks, I'll have a look at it, after an emergency job I need to attend to. But the API looks weird. Why are fields and nfields OUT params. The issue isn't decomposing the line into raw fields. The code for doing that works fine as is, including on jagged files. See commit af1a614ec6d074fdea46de2e1c462f23fc7ddc6f which was done for exactly this purpose. The issue is taking those and composing them into the expected tuple. I'm willing to include the change into copy APIs, but we still have a few issues. See below. On Fri, Feb 4, 2011 at 16:53, Andrew Dunstanand...@dunslane.net wrote: The problem with COPY FROM is that nobody's come up with a good syntax for allowing it as a FROM target. Doing what I want via FDW neatly gets us around that problem. But I'm quite OK with doing the hard work inside the COPY code - that's what my working prototype does in fact. I think it is not only syntax issue. I found an issue that we hard to support FORCE_NOT_NULL option for extra fields. See FIXME in the patch. It is a fundamental problem to support jagged fields. It's not a problem at all if you turn the line into a text array. That's exactly why we've been proposing it for this. The array has however many elements are on the line. One thing I'd like is to to have file_fdw do something we can't do another way. currently it doesn't, so it's nice but uninteresting. BTW, how do you determine which field is shifted in your broken CSV file? For example, the case you find AB,CD,EF for 2 columns tables. I could provide a raw CSV reader for jagged files, but you still have to cook the returned fields into a proper tuple... See above. My client who deals with this situation and has been doing so for years treats underflowing fields as null and ignores overflowing fields. They would do he same if the data were delivered with a text array. It works very well for them. See https://github.com/adunstan/postgresql-dev/tree/sqlmed2 for my dev branch on this. 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
[HACKERS] SSI performance
We know that the predicate locking introduced by the serializable snapshot isolation patch adds a significant amount of overhead, when it's used. It was fixed for sequential scans by acquiring a relation level lock upfront and skipping the locking after that, but the general problem for index scans and bitmap index scans remains. I ran a little benchmark of that: postgres=# begin isolation level repeatable read; BEGIN Time: 0,262 ms postgres=# SELECT COUNT(*) FROM foo WHERE id 40; count 39 (1 row) Time: 204,571 ms postgres=# begin isolation level serializable; BEGIN Time: 0,387 ms postgres=# SELECT COUNT(*) FROM foo WHERE id 40; count 39 (1 row) Time: 352,293 ms These numbers are fairly repeatable. I ran oprofile to see where the time is spent, and fed the output to kcachegrind to get a better breakdown. Attached is a screenshot of that (I don't know how to get this information in a nice text format, sorry). As you might expect, about 1/3 of the CPU time is spent in PredicateLockTuple(), which matches with the 50% increase in execution time compared to repeatable read. IOW, all the overhead comes from PredicateLockTuple. The interesting thing is that CoarserLockCovers() accounts for 20% of the overall CPU time, or 2/3 of the overhead. The logic of PredicateLockAcquire is: 1. Check if we already have a lock on the tuple. 2. Check if we already have a lock on the page. 3. Check if we already have a lock on the relation. So if you're accessing a lot of rows, so that your lock is promoted to a relation lock, you perform three hash table lookups on every PredicateLockAcquire() call to notice that you already have the lock. I was going to put a note at the beginning of this mail saying upfront that this is 9.2 materila, but it occurs to me that we could easily just reverse the order of those tests. That would cut the overhead of the case where you already have a relation lock by 2/3, but make the case where you already have a tuple lock slower. Would that be a good tradeoff? For 9.2, perhaps a tree would be better than a hash table for this.. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com attachment: ssi-kcachegrind-screenshot.png -- 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] SSI performance
On Fri, Feb 4, 2011 at 9:29 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: The interesting thing is that CoarserLockCovers() accounts for 20% of the overall CPU time, or 2/3 of the overhead. The logic of PredicateLockAcquire is: 1. Check if we already have a lock on the tuple. 2. Check if we already have a lock on the page. 3. Check if we already have a lock on the relation. So if you're accessing a lot of rows, so that your lock is promoted to a relation lock, you perform three hash table lookups on every PredicateLockAcquire() call to notice that you already have the lock. I was going to put a note at the beginning of this mail saying upfront that this is 9.2 materila, but it occurs to me that we could easily just reverse the order of those tests. That would cut the overhead of the case where you already have a relation lock by 2/3, but make the case where you already have a tuple lock slower. Would that be a good tradeoff? Not sure. How much benefit do we get from upgrading tuple locks to page locks? Should we just upgrade from tuple locks to full-relation locks? It also seems like there might be some benefit to caching the knowledge that we have a full-relation lock somewhere, so that once we get one we needn't keep checking that. Not sure if that actually makes sense... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] ALTER EXTENSION UPGRADE, v3
On Thu, Feb 3, 2011 at 4:24 PM, Josh Berkus j...@agliodbs.com wrote: So every package would include a script called upgrade.sql ( or upgrade.c? ) which is supposed to handle the upgrade, and it's up to the module author to power that, at least until 9.2? Seem like the most reasonable course for February ... I don't think we should commit something that for 9.1 that we may need to change incompatibly for 9.2. If we're not completely happy with it, it gets booted. Whatever we put in place here is going to be with us for a long, long time. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Add ENCODING option to COPY
2011/2/4 Itagaki Takahiro itagaki.takah...@gmail.com: On Tue, Feb 1, 2011 at 13:08, Hitoshi Harada umi.tan...@gmail.com wrote: The third patch is attached, modifying mb routines so that they can receive conversion procedures as FmgrInof * and save the function pointer in CopyState. I tested it with encoding option and could not see performance slowdown. Hmm, sorry, the patch was wrong. Correct version is attached. Here is a brief review for the patch. Thanks for the review. * Syntax: ENCODING encoding vs. ENCODING 'encoding' We don't have to quote encoding names in the patch, but we always need quotes for CREATE DATABASE WITH ENCODING. I think we should modify CREATE DATABASE to accept unquoted encoding names aside from the patch. I followed the syntax in SET client_encoding TO xxx, where you don't need quote xxx. I didn't care CREATE DATABASE. Changes in pg_wchar.h are the most debatable parts of the patch. The patch adds pg_cached_encoding_conversion(). We normally use pg_do_encoding_conversion(), but it is slower than the proposed function because it lookups conversion proc from catalog every call. * Can we use #ifndef FRONTEND in the header? Usage of fmgr.h members will broke client applications without the #ifdef, but I guess client apps don't always have definitions of FRONTEND. If we don't want to change pg_wchar.h, pg_conversion_fn.h might be a better header for the new API because FindDefaultConversion() is in it. It doesn't look to me like clear solution. FindDefaultConversion() is implemented in pg_conversion.c, whereas pg_cached_encoding_conversion() in mbutils.c. Maybe adding another header, namely pg_wchar_backend.h? * CopyState can have conv_proc entity as a member instead of the pointer. * need_transcoding checks could be replaced with conv_proc IS NULL check. No, need_transcoding means you need verification even if the target and server encoding is the same. See comments in CopyTo(). If pg_wchar_backend .h is acceptable, I'll post the revised patch. Regards, -- Hitoshi Harada -- 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] SQL/MED - file_fdw
On Fri, Jan 21, 2011 at 8:12 AM, Shigeru HANADA han...@metrosystems.co.jp wrote: * Try strVal() instead of DefElem-val.str * FdwEPrivate seems too abbreviated for me. How about FileFdwPrivate? Thanks, fixed. Was there supposed to be a patch attached here? Or where is it? We are past out of time to get this committed, and there hasn't been a new version in more than two weeks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] ALTER EXTENSION UPGRADE, v3
Robert Haas robertmh...@gmail.com writes: I don't think we should commit something that for 9.1 that we may need to change incompatibly for 9.2. If we're not completely happy with it, it gets booted. Whatever we put in place here is going to be with us for a long, long time. So, what is it specifically here that you're unhappy with? a. ALTER EXTENSION ... UPGRADE; b. CREATE WRAPPER EXTENSION ...; (version is then NULL) c. upgrade rules in the control file d. ALTER OBJECT ... SET EXTENSION ...; e. having upgrade scripts for upgrading contribs from null f. having those scripts named $contrib.upgrade.sql What I think is that the end-user syntax (the SQL DDLs) that we add are going to fall exactly into the category you're talking about: long, long term support. But that could well be less true of the control file, should we choose so. I think there's enough value in being able to get extension from what you had installed in pre-9.1; that changing some non-DLL bits in 9.2 is something we can set ourselves to consider. But anyway, we've been doing quite a round of expectations, explaining, detailing, and bikeshedding on the features already, so I'd like to see a break down, because it appears clearly that some readers changed their mind in the process. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] REVIEW: PL/Python table functions
2011/1/28 Jan Urbański wulc...@wulczer.org: On 27/01/11 00:41, Jan Urbański wrote: I'm also attaching an updated version that should apply on top of my github refactor branch (or incrementally over the new set of refactor patches that I will post shortly to the refactor thread). Attached is a patch for master, as the refactorings have already been merged. Sorry, but could you update your patch? Patching it against HEAD today makes rej. patching file `src/pl/plpython/Makefile' patching file `src/pl/plpython/expected/plpython_composite.out' patching file `src/pl/plpython/expected/plpython_record.out' Hunk #1 succeeded at 42 with fuzz 2. Hunk #2 FAILED at 296. 1 out of 2 hunks FAILED -- saving rejects to src/pl/plpython/expected/plpython_record.out.rej patching file `src/pl/plpython/expected/plpython_trigger.out' patching file `src/pl/plpython/plpython.c' Hunk #1 succeeded at 100 with fuzz 2. Hunk #2 succeeded at 132 (offset 1 line). Hunk #4 succeeded at 346 (offset 4 lines). Hunk #6 succeeded at 1166 (offset 40 lines). Hunk #8 succeeded at 1485 (offset 40 lines). Hunk #10 succeeded at 1892 (offset 40 lines). Hunk #12 succeeded at 1938 (offset 40 lines). Hunk #14 succeeded at 2011 (offset 40 lines). Hunk #16 succeeded at 2300 (offset 40 lines). Hunk #18 succeeded at 2456 (offset 40 lines). Hunk #20 succeeded at 2531 (offset 40 lines). patching file `src/pl/plpython/sql/plpython_composite.sql' patching file `src/pl/plpython/sql/plpython_record.sql' patching file `src/pl/plpython/sql/plpython_trigger.sql' Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CommitFest progress - or lack thereof
With ten days left in the current CommitFest, being the last CommitFest for 9.1 development, there are presently 40 patches that are marked either Needs Review or Waiting on Author. The 11 patches that are Waiting on Author are the following: Synchronous Replication, transaction-controlled Recovery Control SQL/MED - postgresql_fdw FDW API Skip validation of Foreign Keys Self-tuning checkpoint sync spread PL/Python explicit subtransactions keeping timestamp of the lasts stats reset Named restore points pg_stat_activity.client_hostname field log_csv_fields ; add current_role log option If you are the author of one of these patches, you need to post an updated patch ASAP, or wait for 9.2. A number of these patches have been sitting for WEEKS without an update. When you post your updated patch (or if by chance you already did), please add a link to the CommitFest application and change the status to Needs Review. Many of these patches likely still need a few more rounds of review before they are committed. If you wait until February 14th at 11:59pm to update them, they're not going to make it in. https://commitfest.postgresql.org/action/commitfest_view/inprogress As for the patches that are marked Needs Review, some people have put their names into the CommitFest application, indicating their intent and commitment to review particular patches, and then have not done so. If you are one of those people, please post your review to the list and update the CommitFest application. If you are a reviewer who has completed all of the reviewing you've previously signed up for, and still want to do more, please consider jumping in on one of the patches that still needs review, and help move the discussion along. Even if you cannot do a full review, please review as much as you can and post your feedback to the list. We need to determine which of these patches are viable candidates for 9.2 and which are not. If you are a patch author and your patch is marked as needing review, please double-check that it still applies and has not bitrotted, and verify that you have responded to all feedback previously given, so that if someone has time to review your patch they can do so productively. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] SSI patch version 14
On 04.02.2011 14:30, Kevin Grittner wrote: Heikki Linnakangas wrote: The assertion in RecordFreeIndexPage() is a reasonable sanity check, but I'm inclined to move it to the caller instead: I don't think the FSM should need to access predicate lock manager, even for an assertion. OK. So it looks like right now it will move to btvacuumpage(), right before the call to RecordFreeIndexPage(), and we will need to add it to one spot each in the GiST and GIN AMs, when we get to those. Would you like me to do that? Yeah, please do. Thanks! -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] CommitFest progress - or lack thereof
Robert, I don't see btree_gist with knn-support. I'm afraid it'll be forgotten. Oleg On Fri, 4 Feb 2011, Robert Haas wrote: With ten days left in the current CommitFest, being the last CommitFest for 9.1 development, there are presently 40 patches that are marked either Needs Review or Waiting on Author. The 11 patches that are Waiting on Author are the following: Synchronous Replication, transaction-controlled Recovery Control SQL/MED - postgresql_fdw FDW API Skip validation of Foreign Keys Self-tuning checkpoint sync spread PL/Python explicit subtransactions keeping timestamp of the lasts stats reset Named restore points pg_stat_activity.client_hostname field log_csv_fields ; add current_role log option If you are the author of one of these patches, you need to post an updated patch ASAP, or wait for 9.2. A number of these patches have been sitting for WEEKS without an update. When you post your updated patch (or if by chance you already did), please add a link to the CommitFest application and change the status to Needs Review. Many of these patches likely still need a few more rounds of review before they are committed. If you wait until February 14th at 11:59pm to update them, they're not going to make it in. https://commitfest.postgresql.org/action/commitfest_view/inprogress As for the patches that are marked Needs Review, some people have put their names into the CommitFest application, indicating their intent and commitment to review particular patches, and then have not done so. If you are one of those people, please post your review to the list and update the CommitFest application. If you are a reviewer who has completed all of the reviewing you've previously signed up for, and still want to do more, please consider jumping in on one of the patches that still needs review, and help move the discussion along. Even if you cannot do a full review, please review as much as you can and post your feedback to the list. We need to determine which of these patches are viable candidates for 9.2 and which are not. If you are a patch author and your patch is marked as needing review, please double-check that it still applies and has not bitrotted, and verify that you have responded to all feedback previously given, so that if someone has time to review your patch they can do so productively. Thanks, Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- 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] Add ENCODING option to COPY
Itagaki Takahiro itagaki.takah...@gmail.com writes: * Syntax: ENCODING encoding vs. ENCODING 'encoding' We don't have to quote encoding names in the patch, but we always need quotes for CREATE DATABASE WITH ENCODING. I think we should modify CREATE DATABASE to accept unquoted encoding names aside from the patch. The reason that we use quotes in CREATE DATABASE is that encoding names aren't assumed to be valid SQL identifiers. If this patch isn't following the CREATE DATABASE precedent, it's the patch that's wrong, not CREATE DATABASE. Changes in pg_wchar.h are the most debatable parts of the patch. The patch adds pg_cached_encoding_conversion(). We normally use pg_do_encoding_conversion(), but it is slower than the proposed function because it lookups conversion proc from catalog every call. Why should callers need to be changed for that? Just make the existing function use caching. * Can we use #ifndef FRONTEND in the header? Usage of fmgr.h members will broke client applications without the #ifdef, but I guess client apps don't always have definitions of FRONTEND. If we don't want to change pg_wchar.h, pg_conversion_fn.h might be a better header for the new API because FindDefaultConversion() is in it. Yeah, putting backend-only stuff into that header is a nonstarter. 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] ALTER EXTENSION UPGRADE, v3
On Fri, Feb 4, 2011 at 10:13 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: I don't think we should commit something that for 9.1 that we may need to change incompatibly for 9.2. If we're not completely happy with it, it gets booted. Whatever we put in place here is going to be with us for a long, long time. So, what is it specifically here that you're unhappy with? I'd like to answer this question, but I have not had enough time to read through this patch in detail, because there are 97 patches in this CommitFest. The point I'm trying to make, however, is procedural. We shouldn't commit anything at the very end of a development cycle that we're not reasonably comfortable we can live with, because there is not a lot of time to change our minds later. I completely believe that an extension upgrade mechanism is a good thing to have and I'm sympathetic to your desire to get this into 9.1 - but the fact is that we are very short on time, the prerequisite patch is not committed yet, and this is a big piece of functionality in a tricky area which was submitted for the last CommitFest of the cycle and about which there is not a clear design consensus. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] CommitFest progress - or lack thereof
On Fri, Feb 4, 2011 at 10:46 AM, Oleg Bartunov o...@sai.msu.su wrote: I don't see btree_gist with knn-support. I'm afraid it'll be forgotten. If you don't see it there, it's because you didn't add it. The deadline for getting your patch into the CommitFest application was January 15th, and several reminders were sent out in advance of that date. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] SSI performance
On 04.02.2011 15:37, Robert Haas wrote: Not sure. How much benefit do we get from upgrading tuple locks to page locks? Should we just upgrade from tuple locks to full-relation locks? Hmm, good question. Page-locks are the coarsest level for the b-tree locks, but maybe that would make sense for the heap. It also seems like there might be some benefit to caching the knowledge that we have a full-relation lock somewhere, so that once we get one we needn't keep checking that. Not sure if that actually makes sense... Well, if you reverse the order of the hash table lookups, that's effectively what you get. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] Re: patch: fix performance problems with repated decomprimation of varlena values in plpgsql
On Fri, Jan 28, 2011 at 2:19 PM, Pavel Stehule pavel.steh...@gmail.com wrote: 2011/1/28 Robert Haas robertmh...@gmail.com: On Tue, Jan 25, 2011 at 11:29 AM, Pavel Stehule pavel.steh...@gmail.com wrote: variant a) my first patch - detoast on first usage with avoiding to useless detoast checking variant b) my first patch - detoast on first usage without avoiding to useless detoast checking time for 1 - about 300 ms, a is bout 1.5% faster than b time for 2 - about 3 ms, a is about 3% faster than b This makes your approach sound pretty good, but it sounds like we might need to find a better way to structure the code. do you have a any idea? At first blush, the should_be_detoasted flag looks completely unnecessary to me. I don't see why we have to set a flag in one part of the code to tell some other part of the code whether or not it should detoast. Why is that necessary or a good idea? Why can't we just make the decision at the point where we're detoasting? Also, I believe I'm agreeing with Tom when I say that exec_eval_datum() doesn't look like the right place to do this. Right now that function is a very simple switch, and I've found that it's usually a bad idea to stick complex code into the middle of such things. It looks like function only has three callers, so maybe we should look at each caller individually and see whether it needs this logic or not. For example, I'm guessing make_tuple_from_row() doesn't, because it's only calling exec_eval_datum() so that it can pass the results to heap_form_tuple(), which already contains some logic to detoast in certain cases. It's not clear why we'd want different logic here than we do anywhere else. The other callers are exec_assign_value(), where this looks like it could be important to avoid repeatedly detoasting the array, and plpgsql_param_fetch(), which I'm not sure about, but perhaps you have an idea or can benchmark it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] pl/python custom datatype parsers
2011/1/28 Jan Urbański wulc...@wulczer.org:
On 23/12/10 15:15, Jan Urbański wrote:
Here's a patch implementing custom parsers for data types mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.
Updated to master.
I reviewed this for some time today.
The patch applies with hunks, compiles and tests are passed, though it
looks like not having additional test along with it.
- in hstore_plpython.c,
PLyParsers parsers = {
.in = hstore_to_dict,
.out = dict_to_hstore
};
I'm not sure if this coding style is used anywhere in the core.
Isn't this the C99 style?
- You need define custom variable class to use this feature.
plpython.hstore = 'public.hstore'. I wonder why it's called
plpython[u].hstore = 'public.hstore' (with 'u') because the language
is called plpythonu.
- typo in plpython.h,
Types for parsres functions that ...
- I tried the sample you mention upthread,
regression=# select pick_one('a=3, b=4', 'b');
ERROR: TypeError: string indices must be integers
CONTEXT: PL/Python function pick_one
My python is 2.4.3 again.
That's it for now. It is an exciting feature and plpython will be the
first language to think of when you're building object database if
this feature is in. The design here will affect following pl/perl and
other so it is important enough to discuss.
Regards,
--
Hitoshi Harada
--
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] Add ENCODING option to COPY
2011/2/5 Tom Lane t...@sss.pgh.pa.us: Itagaki Takahiro itagaki.takah...@gmail.com writes: * Syntax: ENCODING encoding vs. ENCODING 'encoding' We don't have to quote encoding names in the patch, but we always need quotes for CREATE DATABASE WITH ENCODING. I think we should modify CREATE DATABASE to accept unquoted encoding names aside from the patch. The reason that we use quotes in CREATE DATABASE is that encoding names aren't assumed to be valid SQL identifiers. If this patch isn't following the CREATE DATABASE precedent, it's the patch that's wrong, not CREATE DATABASE. What about SET client_encoding TO encoding? Changes in pg_wchar.h are the most debatable parts of the patch. The patch adds pg_cached_encoding_conversion(). We normally use pg_do_encoding_conversion(), but it is slower than the proposed function because it lookups conversion proc from catalog every call. Why should callers need to be changed for that? Just make the existing function use caching. Because the demanded behavior is similar to pg_client_to_server/server_to_client, which caches functions as specialized client encoding and database encoding as the file local variables. We didn't have such spaces to cache functions for other encoding conversions, so decided to cache them in CopyState and pass them to the mb routine. Regards, -- Hitoshi Harada -- 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] Add ENCODING option to COPY
Hitoshi Harada umi.tan...@gmail.com writes: 2011/2/5 Tom Lane t...@sss.pgh.pa.us: The reason that we use quotes in CREATE DATABASE is that encoding names aren't assumed to be valid SQL identifiers. If this patch isn't following the CREATE DATABASE precedent, it's the patch that's wrong, not CREATE DATABASE. What about SET client_encoding TO encoding? SET is in its own little world --- it will interchangeably take names with or without quotes. It is not a precedent to follow elsewhere. 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] Add ENCODING option to COPY
2011/2/5 Tom Lane t...@sss.pgh.pa.us: Hitoshi Harada umi.tan...@gmail.com writes: 2011/2/5 Tom Lane t...@sss.pgh.pa.us: The reason that we use quotes in CREATE DATABASE is that encoding names aren't assumed to be valid SQL identifiers. If this patch isn't following the CREATE DATABASE precedent, it's the patch that's wrong, not CREATE DATABASE. What about SET client_encoding TO encoding? SET is in its own little world --- it will interchangeably take names with or without quotes. It is not a precedent to follow elsewhere. I see. I'll update my patch, after the mb change discussion gets done. * Can we use #ifndef FRONTEND in the header? Usage of fmgr.h members will broke client applications without the #ifdef, but I guess client apps don't always have definitions of FRONTEND. If we don't want to change pg_wchar.h, pg_conversion_fn.h might be a better header for the new API because FindDefaultConversion() is in it. Yeah, putting backend-only stuff into that header is a nonstarter. Do you mean you think it' all right to define pg_cached_encoding_conversion() in pg_conversion_fn.h? Regards, -- Hitoshi Harada -- 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] SSI patch version 14
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 04.02.2011 14:30, Kevin Grittner wrote: OK. So it looks like right now it will move to btvacuumpage(), right before the call to RecordFreeIndexPage(), and we will need to add it to one spot each in the GiST and GIN AMs, when we get to those. Would you like me to do that? Yeah, please do. Thanks! Done: http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=69b1c072048fbae42dacb098d70f5e29fb8be63c Any objections to me taking out the dcheck target and scripts now? -Kevin -- 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] ALTER EXTENSION UPGRADE, v3
On Feb 4, 2011, at 7:48 AM, Robert Haas wrote: I'd like to answer this question, but I have not had enough time to read through this patch in detail, because there are 97 patches in this CommitFest. The point I'm trying to make, however, is procedural. We shouldn't commit anything at the very end of a development cycle that we're not reasonably comfortable we can live with, because there is not a lot of time to change our minds later. I completely believe that an extension upgrade mechanism is a good thing to have and I'm sympathetic to your desire to get this into 9.1 - but the fact is that we are very short on time, the prerequisite patch is not committed yet, and this is a big piece of functionality in a tricky area which was submitted for the last CommitFest of the cycle and about which there is not a clear design consensus. Robert, I think that the core extension if pretty uncontroversial, modulo some minor issues. It's the upgrade process that's more controversial. I think the case can be made to accept even that part as Dim has written it, because it is pretty much the bare minimum that other solutions could be built on top of and improve upon. But if not, I think that's the only part the one might really look at as something to omit for 9.1. Dim, I haven't followed that closely lately, but is the ALTER EXTENSION UPGRADE bit still a separate patch? Best, David -- 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] Add ENCODING option to COPY
Hitoshi Harada umi.tan...@gmail.com writes: 2011/2/5 Tom Lane t...@sss.pgh.pa.us: Yeah, putting backend-only stuff into that header is a nonstarter. Do you mean you think it' all right to define pg_cached_encoding_conversion() in pg_conversion_fn.h? That seems pretty random too. I still think you've designed this API badly and it'd be better to avoid exposing the FmgrInfo to callers by letting the function manage the cache internally. 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] ALTER EXTENSION UPGRADE, v3
On Fri, Feb 4, 2011 at 11:48 AM, David E. Wheeler da...@kineticode.com wrote: Robert, I think that the core extension if pretty uncontroversial, modulo some minor issues. It's the upgrade process that's more controversial. I think the case can be made to accept even that part as Dim has written it, because it is pretty much the bare minimum that other solutions could be built on top of and improve upon. But if not, I think that's the only part the one might really look at as something to omit for 9.1. Yeah, I understand. I believe Tom said he was going to look at the basic functionality with an eye toward commit, and I hope to look at it as well, either before or after it gets committed. Time permitting, I'd then like to look at this, but I'm not sure I'm going to be able to squeeze that into the time available, nor am I sure that I'd get sufficient consensus to commit something even if I did. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] SSI performance
On Fri, Feb 4, 2011 at 11:07 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 04.02.2011 15:37, Robert Haas wrote: Not sure. How much benefit do we get from upgrading tuple locks to page locks? Should we just upgrade from tuple locks to full-relation locks? Hmm, good question. Page-locks are the coarsest level for the b-tree locks, but maybe that would make sense for the heap. It also seems like there might be some benefit to caching the knowledge that we have a full-relation lock somewhere, so that once we get one we needn't keep checking that. Not sure if that actually makes sense... Well, if you reverse the order of the hash table lookups, that's effectively what you get. I was wondering if it could be cached someplace that was cheaper to examine, though, like (stabs wildly) the executor scan state. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] ALTER EXTENSION UPGRADE, v3
On Feb 4, 2011, at 8:54 AM, Robert Haas wrote: Yeah, I understand. I believe Tom said he was going to look at the basic functionality with an eye toward commit, and I hope to look at it as well, either before or after it gets committed. Time permitting, I'd then like to look at this, but I'm not sure I'm going to be able to squeeze that into the time available, nor am I sure that I'd get sufficient consensus to commit something even if I did. Great, thanks. Best, David -- 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] ALTER EXTENSION UPGRADE, v3
David E. Wheeler da...@kineticode.com writes: Dim, I haven't followed that closely lately, but is the ALTER EXTENSION UPGRADE bit still a separate patch? Yes it is. It's an incremental that apply on top of the extension patch and get its own patch entry on the commit fest application: https://commitfest.postgresql.org/action/patch_view?id=472 As such it will need bitrot fixing as soon as the extension main patch makes it in. Also I have some cleaning to do here, but given the current open discussion about the design I'm still holding this work. Well, it seems the discussion is slowing down to realize I only included the bare minimum just so that we avoid having too long a discussion and that the patch has its chances to 9.1 :) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CommitFest progress - or lack thereof
Aha, Teodor sent it to the list Dec 28, see http://archives.postgresql.org/message-id/4D1A1677.80300%40sigaev.ru After a month I didn't see any activity on this patch, so I I added it to https://commitfest.postgresql.org/action/patch_view?id=350 Jan 21 Now, I realised it was too late. Added to current commitfest. Oleg On Fri, 4 Feb 2011, Robert Haas wrote: On Fri, Feb 4, 2011 at 10:46 AM, Oleg Bartunov o...@sai.msu.su wrote: I don't see btree_gist with knn-support. I'm afraid it'll be forgotten. If you don't see it there, it's because you didn't add it. The deadline for getting your patch into the CommitFest application was January 15th, and several reminders were sent out in advance of that date. Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- 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] pl/python quoting functions
2011/1/11 Jan Urbański wulc...@wulczer.org: Here's a patch that adds a few PL/Python functions for quoting strings. It's an incremental patch on top of the plpython-refactor patch sent in http://archives.postgresql.org/message-id/4d135170.3080...@wulczer.org. Git branch for this patch: https://github.com/wulczer/postgres/tree/functions The new functions are plpy.quote_literal, plpy.quote_nullable and plpy.quote_ident, and work just like their sql or plperl equivalents. I reviewed this. The patch applies and compiles cleanly and all the tests are passed. The patch adds 3 functions which works as the corresponding SQL functions. The test is enough, without any additional docs. No feature/performance issues found. I mark this Reader for Committer. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] more buildfarm breakage
mingw is unhappy with my latest stab at fixing the mess created by the errcodes patch last night. It appears that there are several files in src/port that include postgres.h even when FRONTEND is defined. For example, chklocale.c does this, which looks good: #ifndef FRONTEND #include postgres.h #else #include postgres_fe.h #endif But dirent.c, pipe.c, and win32error.c just do this, which seems ungood: #include postgres.h Can we get away with using the former incantation for these files, or do they really need to include the backend version of that file even when compiled with -DFRONTEND? If so, I can fix it by adding some more dependencies, but I thought I'd ask first. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] SSI performance
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: The logic of PredicateLockAcquire is: 1. Check if we already have a lock on the tuple. 2. Check if we already have a lock on the page. 3. Check if we already have a lock on the relation. So if you're accessing a lot of rows, so that your lock is promoted to a relation lock, you perform three hash table lookups on every PredicateLockAcquire() call to notice that you already have the lock. I was going to put a note at the beginning of this mail saying upfront that this is 9.2 materila, but it occurs to me that we could easily just reverse the order of those tests. That would cut the overhead of the case where you already have a relation lock by 2/3, but make the case where you already have a tuple lock slower. Would that be a good tradeoff? The problem with that is that we'd need to figure out how to handle LW locking. The target at each level may be protected by a different partition lock, so locks must be released and acquired for each level checked. When we promote granularity we add to the higher level first and then remove from the lower level, again protected by potentially different partition locks. To avoid having a search miss during concurrent promotion, we did the searches from the finer to the coarser granularity. In early benchmarks we were held up mostly by LW lock contention, and went to some effort to rearrange locking to minimize that; we'd need to be very careful trying to go the other direction here. I just had a thought -- we already have the LocalPredicateLockHash HTAB to help with granularity promotion issues without LW locking. Offhand, I can't see any reason we couldn't use this for an initial check for a relation level lock, before going through the more rigorous pass under cover of the locks. We won't have a relation lock showing in the local table if it never was taken out, and it won't go away until the end of transaction (unless the whole transaction is deamed safe from SSI conflicts and everything is cleaned up early). Dan, does that look sane to you, or am I having another senior moment here? If this works, it would be a very minor change, which might eliminate a lot of that overhead for many common cases. -Kevin -- 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] multiset patch review
On Mon, Jan 31, 2011 at 1:49 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: I removed collect() aggregate function because the result type is MULTISET in the spec. I keep all of other functions and operators because they won't break anything in the standard. When we will have true MULTISET data type, we can overload those functions and operators for MULTISET and ARRAY. I am still not in favor of adding this syntax. I'd be in favor of adding array_cardinality(), trim_array(), array_sort(), and array_flatten(). [It sure is awkward that trim_array has the word array second and all of our other array functions have it first - is this required by the spec?] array_to_set() and array_is_set() seem possibly useful, but I probably would have called them array_remove_dups() and array_has_dups(). I might be in the minority on that one, though. I think array_subset(), array_union(), array_intersect(), and array_except() are useful, but I think they should just be regular functions, without any special syntax. fusion() and intersection() also seem useful, but maybe it would be more consistent to all them array_fusion() and array_intersection(). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] arrays as pl/perl input arguments [PATCH]
On 02/03/2011 08:29 PM, Alex Hunsaker wrote: On Mon, Jan 31, 2011 at 01:34, Alexey Klyukinal...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. One thing I find odd is we allow for nested arrays, but not nested composites. This is not unique to the patch, it how nested composites are handled in general. That is if you passed in ROW(ROW()), the first ROW would be a hash while the 2nd row would be a scalar. On the other end, the same is true when returning. If you try to return a nested composite or array, the child better be a string as it wont let you pass a hash I think the reason this is so has to do with the history. Composite support came to plperl about the same time (in the 8.0 cycle, IIRC) that we were getting more support for nested composites in Postgres, and they sorta passed like ships in the night. Anyone object to fixing the above as part of this patch? That is making plperl_(build_tuple_result, modify_tuple, return_next, hash_from_tuple) handle array and hash (composite/row) types consistently? And _that_ would be to recurse and turn them from/into perl objects. Thoughts? I have no objection to making the whole thing work recursively, in principle, but will it break legacy code? 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] multiset patch review
On Sat, Feb 5, 2011 at 02:29, Robert Haas robertmh...@gmail.com wrote: I am still not in favor of adding this syntax. I also don't like the syntax, but unfortunately, almost all of them are in the SQL standard :-(. In addition, Oracle already uses the same feature with the special syntax, though true multiset data types are supported in it. [It sure is awkward that trim_array has the word array second and all of our other array functions have it first - is this required by the spec?] Yes, again, but we could have array_trim() for the alias. array_to_set() and array_is_set() seem possibly useful, but I probably would have called them array_remove_dups() and array_has_dups(). I might be in the minority on that one, though. array_to_set is the only function we can rename freely. Another candidates might be array_unique (contrib/intarray uses uniq). array_is_set() is an internal representation of IS A SET operator. So, the name is not so important (and not documented.) I think array_subset(), array_union(), array_intersect(), and array_except() are useful, but I think they should just be regular functions, without any special syntax. All of the special syntax are in the spec, except the argument types should be multisets rather than arrays. fusion() and intersection() also seem useful, but maybe it would be more consistent to all them array_fusion() and array_intersection(). Both of the names are the standard... We could have array_fusion() for array types and fusion() for multiset types, but I prefer overloaded fusion() to have both names. -- Itagaki Takahiro -- 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] more buildfarm breakage
Robert Haas robertmh...@gmail.com writes: mingw is unhappy with my latest stab at fixing the mess created by the errcodes patch last night. It appears that there are several files in src/port that include postgres.h even when FRONTEND is defined. For example, chklocale.c does this, which looks good: #ifndef FRONTEND #include postgres.h #else #include postgres_fe.h #endif But dirent.c, pipe.c, and win32error.c just do this, which seems ungood: #include postgres.h I agree, that is not cool. Can we get away with using the former incantation for these files, or do they really need to include the backend version of that file even when compiled with -DFRONTEND? If so, I can fix it by adding some more dependencies, but I thought I'd ask first. If the #ifndef FRONTEND incantation doesn't work, then either the file isn't meant to be built for frontend at all (src/port does have some like that IIRC), or we need to fix the code. BTW, I noted here that errcodes.h doesn't seem to get built till after src/port/ is built. That seems wrong --- there is definitely code in there that needs to throw errors. 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] multiset patch review
On Fri, Feb 4, 2011 at 1:02 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Sat, Feb 5, 2011 at 02:29, Robert Haas robertmh...@gmail.com wrote: I am still not in favor of adding this syntax. I also don't like the syntax, but unfortunately, almost all of them are in the SQL standard :-(. The standard specifies this syntax for arrays, or for multisets? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] SSI performance
I wrote: I just had a thought -- we already have the LocalPredicateLockHash HTAB to help with granularity promotion issues without LW locking. Offhand, I can't see any reason we couldn't use this for an initial check for a relation level lock, before going through the more rigorous pass under cover of the locks. We won't have a relation lock showing in the local table if it never was taken out, and it won't go away until the end of transaction (unless the whole transaction is deamed safe from SSI conflicts and everything is cleaned up early). Dan, does that look sane to you, or am I having another senior moment here? If this works, it would be a very minor change, which might eliminate a lot of that overhead for many common cases. To make this more concrete: http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=91d93734b8a84cf54e77f8d30b26afde7cc43218 I *think* we should be able to do this with the page lock level, too; but I'd like Dan to weigh in on that before putting something out there for that. He did the local lock table work, and will be in a better position to know how safe this is. Also, while it's pretty clear this should be a win at the relation level, it's not as clear to me whether adding this for the page level will be a net gain. -Kevin -- 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] keeping a timestamp of the last stats reset (for a db, table and function)
Dne 4.2.2011 03:37, Greg Smith napsal(a):
Thinking I should start with why I think this patch is neat...most of
the servers I deal with are up 24x7 minus small amounts of downtime,
presuming everyone does their job right that is. In that environment,
having a starting timestamp for when the last stats reset happened lets
you quickly compute some figures in per-second terms that are pretty
close to actual average activity on the server. Some examples of how I
would use this:
psql -c
SELECT
CAST(buffers_backend * block_size AS numeric) / seconds_uptime /
(1024*1024)
AS backend_mb_per_sec
FROM
(SELECT *,extract(epoch from now()-stats_reset) AS seconds_uptime,
(SELECT cast(current_setting('block_size') AS int8)) AS block_size
FROM pg_stat_bgwriter) AS raw WHERE raw.seconds_uptime 0
backend_mb_per_sec
4.27150807681618
psql -c
SELECT
datname,CAST(xact_commit AS numeric) / seconds_uptime
AS commits_per_sec
FROM
(SELECT *,extract(epoch from now()-stats_reset) AS seconds_uptime
FROM pg_stat_database) AS raw WHERE raw.seconds_uptime 0
datname | commits_per_sec ---+
template1 | 0.0338722604313051
postgres | 0.0363144438470267
gsmith| 0.0820573653236174
pgbench | 0.059147072347085
Now I reset, put some load on the system and check the same stats
afterward; watch how close these match up:
$ psql -d pgbench -c select pg_stat_reset()
$ pgbench -j 4 -c 32 -T 30 pgbench
transaction type: TPC-B (sort of)
scaling factor: 100
query mode: simple
number of clients: 32
number of threads: 4
duration: 30 s
number of transactions actually processed: 6604
tps = 207.185627 (including connections establishing)
tps = 207.315043 (excluding connections establishing)
datname | commits_per_sec ---+
pgbench | 183.906308135572
Both these examples work as I expected, and some playing around with the
patch didn't find any serious problems with the logic it implements.
One issue though, an oversight I think can be improved upon; watch what
happens when I create a new database:
$ createdb blank
$ psql -c select datname,stats_reset from pg_stat_database where
datname='blank'
datname | stats_reset
-+-
blank |
That's not really what I would hope for here. One major sort of
situation I'd like this feature to work against is the one where someone
asks for help but has never touched their database stats before, which
is exactly what I'm simulating here. In this case that person would be
out of luck, the opposite of the experience I'd like a newbie to have at
this point.
Are you sure about it? Because when I create a database, the field is
NULL - that's true. But once I connect to the database, the stats are
updated and the field is set (thanks to the logic in pgstat.c).
In that case 'stat_reset=NULL' would mean 'no-one ever touched this db'
which seems quite reasonable to me.
$ createdb testdb1
$ createdb testdb2
$ psql -d testdb1 -c select stats_reset from pg_stat_database where
datname = 'testdb2'
stats_reset
-
(1 row)
$ psql -d testdb2 -c \q
$ psql -d testdb1 -c select stats_reset from pg_stat_database where
datname = 'testdb2'
stats_reset
---
2011-02-04 19:03:23.938929+01
(1 row)
But maybe I've missed something and it does not work the way I think it
does.
regards
Tomas
--
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] multiset patch review
On Sat, Feb 5, 2011 at 03:04, Robert Haas robertmh...@gmail.com wrote: I am still not in favor of adding this syntax. I also don't like the syntax, but unfortunately, almost all of them are in the SQL standard :-(. The standard specifies this syntax for arrays, or for multisets? Multisets. But I chose the same function name and syntax because arrays *are* multisets by definition. -- Itagaki Takahiro -- 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] more buildfarm breakage
On Fri, Feb 4, 2011 at 1:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: I agree, that is not cool. OK, changed. BTW, I noted here that errcodes.h doesn't seem to get built till after src/port/ is built. That seems wrong --- there is definitely code in there that needs to throw errors. It does, actually, but it's only guaranteed to be built in time for the *backend* versions of the object files. Hence the problem when the backend headers are used for the frontend build. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] multiset patch review
On Fri, Feb 4, 2011 at 1:15 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Sat, Feb 5, 2011 at 03:04, Robert Haas robertmh...@gmail.com wrote: I am still not in favor of adding this syntax. I also don't like the syntax, but unfortunately, almost all of them are in the SQL standard :-(. The standard specifies this syntax for arrays, or for multisets? Multisets. But I chose the same function name and syntax because arrays *are* multisets by definition. In math class, maybe. But in programming, no. Multiset is a datatype. Array is a different datatype. There is no reason why we need to clutter our parser with extra keywords to support a non-standard feature extension. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] SELECT ... WHERE fti_query ORDER BY numeric_col LIMIT x - problem
I have toyed around with KNN a little and I am pretty impressed when it comes to the results we have seen in the GIS world. Given the infrastructure we have at the moment I wonder if KNN can help to speedup queries like that: SELECT ... WHERE fti_query ORDER BY numeric_col LIMIT x The use case is fairly simple: Give me all products matching a certain tsquery and order those products by price or so to show the top 10. KNN is supposed to gives sorted output which works perfectly for points and so on but can there theoretically be a sensible / reliable distance function for a (tsvector / numeric) combination? Some first testing gave me some interesting output . If there is no way of defining a reasonable distance function performance is screwed up if fti_query returns a large list (it requires a complete sort of all prices then). does anybody have some useful input here? many thanks, hans -- Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Linux filesystem performance and checkpoint sorting
Switching to a new thread for this summary since there's some much more generic info here...at this point I've finished exploring the major Linux filesystem and tuning options I wanted to, as part of examining changes to the checkpoint code. You can find all the raw data at http://www.2ndquadrant.us/pgbench-results/index.htm Here are some highlights of what's been demonstrated there recently, with a summary of some of the more subtle and interesting data in the attached CSV file too: -On ext3, tuning the newish kernel tunables dirty_bytes and dirty_background_bytes down to a lower level than was possible using the older dirty_*ratio ones shows a significant reduction in maximum latency on ext3; it drops to about 1/4 of the worst-case behavior. Unfortunately transactions per second takes a 10-15% hit in the process. Not shown in the data there is that the VACUUM cleanup time between tests was really slowed down, too, running at around half the speed of when the system has a full-size write cache. -Switching from ext3 to xfs gave over a 3X speedup on the smaller test set: from the 600-700 TPS range to around 2200 TPS. TPS rate on the larger data set actually slowed down a touch on XFS, around 10%. Still, such a huge win when it's better makes it easy to excuse the occasional cases where it's a bit slower. And the latency situation is just wildly better, the main thing that drove me toward using XFS more in the first place. Anywhere from 1/6 to 1/25 of the worst-case latency seen on ext3. With abusively high client counts for this hardware, you can still see 10 second pauses, but you don't see 40 second ones at moderate client counts like ext3 experiences. -Switching to the lower possible dirty_*bytes parameters on XFS was negative in every way. TPS was cut in half, and maximum latency actually went up. Between this and the nasty VACUUM slowdown, I don't really see that much potential for these new tunables. They do lower latency on ext3 a lot, but even there the penalty you pay for that is quite high. VACUUM in particular seems to really, really benefit from having a giant write cache to dump its work into--possibly due to the way the ring buffer implementation avoids using the database's own cache for that work. -Since earlier tests suggested sorting checkpoints gave little change on ext3, I started testing that with XFS instead. The result is a bit messy. At the lower scale, TPS went up a bit, but so did maximum latency. At the higher scale, TPS dropped in some cases (typically less than 1%), but most latency results were better too. At this point I would say checkpoint sorting remains a wash: you can find workloads it benefits a little, and others it penalizes a little. I would say that it's neutral enough on average that if it makes sense to include for other purposes, that's unlikely to be a really bad change for anyone. But I wouldn't want to see it committed by itself; there needs to be some additional benefit from the sorting before it's really worthwhile. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books Compact fsync,,ext3,,,XFS + Regular Writes,,Sorted Writes scale,clients,tps,max_latency,XFS Speedup,tps,max_latency,tps,max_latency,TPS Delta,%,Latency Delta 500,16,631,17116.31,3.49,2201,1290.73,2210,2070.74,9,0.41%,780.01 500,32,655,24311.54,3.37,2205,1379.14,2357,1971.2,152,6.89%,592.06 500,64,727,38040.39,3.11,2263,1440.48,2332,1763.29,69,3.05%,322.81 500,128,687,48195.77,3.2,2201,1743.11,2221,2742.18,20,0.91%,999.07 500,256,747,46799.48,2.92,2184,2429.74,2171,2356.14,-13,-0.60%,-73.6 1000,16,321,40826.58,1.21,389,1586.17,386,1598.54,-3,-0.77%,12.37 1000,32,345,27910.51,0.91,314,2150.94,331,2078.02,17,5.41%,-72.91 1000,64,358,45138.1,0.94,336,6681.57,320,6469.71,-16,-4.76%,-211.87 1000,128,372,47125.46,0.88,328,8707.42,330,9037.63,2,0.61%,330.21 1000,256,350,83232.14,0.91,317,11973.35,315,11248.18,-2,-0.63%,-725.17 -- 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] Spread checkpoint sync
Michael Banck wrote: On Sat, Jan 15, 2011 at 05:47:24AM -0500, Greg Smith wrote: For example, the pre-release Squeeze numbers we're seeing are awful so far, but it's not really done yet either. Unfortunately, it does not look like Debian squeeze will change any more (or has changed much since your post) at this point, except for maybe further stable kernel updates. Which file system did you see those awful numbers on and could you maybe go into some more detail? Once the release comes out any day now I'll see if I can duplicate them on hardware I can talk about fully, and share the ZCAV graphs if it's still there. The server I've been running all of the extended pgbench tests in this thread on is running Ubuntu simply as a temporary way to get 2.6.32 before Squeeze ships. Last time I tried installing one of the Squeeze betas I didn't get anywhere; hoping the installer bug I ran into has been sorted when I try again. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Fri, Feb 4, 2011 at 10:29, Andrew Dunstan and...@dunslane.net wrote:
Anyone object to fixing the above as part of this patch? That is
making plperl_(build_tuple_result, modify_tuple, return_next,
hash_from_tuple) handle array and hash (composite/row) types
consistently? And _that_ would be to recurse and turn them from/into
perl objects. Thoughts?
I have no objection to making the whole thing work recursively, in
principle, but will it break legacy code?
It will certainly change how nested composites are represented on the
'input side'. I would argue its a bug the way it is now and also
violates the POLA. I think we should also remain backwards compatible
on the output side so you could still return a string:
-- how things are currently, manually assigning a composite-literal
(would continue to work)
= create or replace function trigger_func() returns trigger as $$
$_TD-{'new'}{'nested_composite'} = {'a'='(1,2)'}';
return 'MODIFY';
-- it would also work with perl nested structures (currently broken)
= create or replace function trigger_func() returns trigger as $$
$_TD-{'new'}{'nested_composite'} = {'a'={'b'=1, 'c'=2}};
return 'MODIFY';
$$
Or perhaps you are saying we should do something similar with what we
now do with arrays? The pseudo array dance that is.
--
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] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: Actually, I was about to pick up and start working on the whole extensions patch series, but now I'm confused as to what and where is the latest version. Ok, here's what I have, attached, as patch v30. What Itagaki just sent applies on top of that. What are the guc.c changes in this patch? They appear completely unrelated to the topic. 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] Add ENCODING option to COPY
2011/2/5 Tom Lane t...@sss.pgh.pa.us: Hitoshi Harada umi.tan...@gmail.com writes: 2011/2/5 Tom Lane t...@sss.pgh.pa.us: Yeah, putting backend-only stuff into that header is a nonstarter. Do you mean you think it' all right to define pg_cached_encoding_conversion() in pg_conversion_fn.h? That seems pretty random too. I still think you've designed this API badly and it'd be better to avoid exposing the FmgrInfo to callers by letting the function manage the cache internally. I'll try it in a few days, but only making struct that holds FmgrInfo in a file local like tuplestorestate comes up with so far Regards, -- Hitoshi Harada -- 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] Spread checkpoint sync
As already mentioned in the broader discussion at http://archives.postgresql.org/message-id/4d4c4610.1030...@2ndquadrant.com , I'm seeing no solid performance swing in the checkpoint sorting code itself. Better sometimes, worse others, but never by a large amount. Here's what the statistics part derived from the sorted data looks like on a real checkpoint spike: 2011-02-04 07:02:51 EST: LOG: checkpoint starting: xlog 2011-02-04 07:02:51 EST: DEBUG: BufferSync 10 dirty blocks in relation.segment_fork 17216.0_2 2011-02-04 07:02:51 EST: DEBUG: BufferSync 159 dirty blocks in relation.segment_fork 17216.0_1 2011-02-04 07:02:51 EST: DEBUG: BufferSync 10 dirty blocks in relation.segment_fork 17216.3_0 2011-02-04 07:02:51 EST: DEBUG: BufferSync 548 dirty blocks in relation.segment_fork 17216.4_0 2011-02-04 07:02:51 EST: DEBUG: BufferSync 808 dirty blocks in relation.segment_fork 17216.5_0 2011-02-04 07:02:51 EST: DEBUG: BufferSync 799 dirty blocks in relation.segment_fork 17216.6_0 2011-02-04 07:02:51 EST: DEBUG: BufferSync 807 dirty blocks in relation.segment_fork 17216.7_0 2011-02-04 07:02:51 EST: DEBUG: BufferSync 716 dirty blocks in relation.segment_fork 17216.8_0 2011-02-04 07:02:51 EST: DEBUG: BufferSync 3857 buffers to write, 8 total dirty segment file(s) expected to need sync 2011-02-04 07:03:31 EST: DEBUG: checkpoint sync: number=1 file=base/16384/17216.5 time=1324.614 msec 2011-02-04 07:03:31 EST: DEBUG: checkpoint sync: number=2 file=base/16384/17216.4 time=0.002 msec 2011-02-04 07:03:31 EST: DEBUG: checkpoint sync: number=3 file=base/16384/17216_fsm time=0.001 msec 2011-02-04 07:03:47 EST: DEBUG: checkpoint sync: number=4 file=base/16384/17216.10 time=16446.753 msec 2011-02-04 07:03:53 EST: DEBUG: checkpoint sync: number=5 file=base/16384/17216.8 time=5804.252 msec 2011-02-04 07:03:53 EST: DEBUG: checkpoint sync: number=6 file=base/16384/17216.7 time=0.001 msec 2011-02-04 07:03:54 EST: DEBUG: compacted fsync request queue from 32768 entries to 2 entries 2011-02-04 07:03:54 EST: CONTEXT: writing block 1642223 of relation base/16384/17216 2011-02-04 07:04:00 EST: DEBUG: checkpoint sync: number=7 file=base/16384/17216.11 time=6350.577 msec 2011-02-04 07:04:00 EST: DEBUG: checkpoint sync: number=8 file=base/16384/17216.9 time=0.001 msec 2011-02-04 07:04:00 EST: DEBUG: checkpoint sync: number=9 file=base/16384/17216.6 time=0.001 msec 2011-02-04 07:04:00 EST: DEBUG: checkpoint sync: number=10 file=base/16384/17216.3 time=0.001 msec 2011-02-04 07:04:00 EST: DEBUG: checkpoint sync: number=11 file=base/16384/17216_vm time=0.001 msec 2011-02-04 07:04:00 EST: LOG: checkpoint complete: wrote 3813 buffers (11.6%); 0 transaction log file(s) added, 0 removed, 64 recycled; write=39.073 s, sync=29.926 s, total=69.003 s; sync files=11, longest=16.446 s, average=2.720 s You can see that it ran out of fsync absorption space in the middle of the sync phase, which is usually when compaction is needed, but the recent patch to fix that kicked in and did its thing. Couple of observations: -The total number of buffers I'm computing based on the checkpoint writes being sorted it not a perfect match to the number reported by the checkpoint complete status line. Sometimes they are the same, sometimes not. Not sure why yet. -The estimate for expected to need sync computed as a by-product of the checkpoint sorting is not completely accurate either. This particular one has a fairly large error in it, percentage-wise, being off by 3 with a total of 11. Presumably these are absorbed fsync requests that were already queued up before the checkpoint even started. So any time estimate I drive based off of this count is only going to be approximate. -The order in which the sync phase processes files is unrelated to the order in which they are written out. Note that 17216.10 here, the biggest victim (cause?) of the I/O spike, isn't even listed among the checkpoint writes! The fuzziness here is a bit disconcerting, and I'll keep digging for why it happens. But I don't see any reason not to continue forward using the rough count here to derive a nap time from, which I can then feed into the useful leftovers patch that Robert already refactored here. Can always sharpen up that estimate later, I need to get some solid results I can share on what the delay time does to the throughput/latency pattern next. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Use a separate pg_depend.deptype for extension membership?
The extensions patch currently records that an object is part of an extension by making a pg_depend entry with deptype 'i' (INTERNAL). While that has the behavior we want, I wonder whether it wouldn't be smarter in the long run to invent a new deptype for this purpose. We do not want people confusing module membership with actual internal dependencies, particularly not if kluges like pg_extension_flag_dump are going to be around. (I'm currently feeling that that function isn't going to make it into the committed patch, but sooner or later there are likely to be similar operations.) The main objection I can see to a new deptype is that it might confuse client-side code that examines pg_depend. But adding all these internal dependencies that don't mean quite what other internal dependencies do would likely confuse such code in more subtle ways anyhow. If we go with a new deptype, I was thinking of using 'm' (macro DEPENDENCY_MEMBER) but am not set on that. Have we been using any particular term to refer to the objects that belong to an extension? 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] multiset patch review
Robert Haas robertmh...@gmail.com writes: On Fri, Feb 4, 2011 at 1:15 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: Multisets. But I chose the same function name and syntax because arrays *are* multisets by definition. In math class, maybe. But in programming, no. Multiset is a datatype. Array is a different datatype. There is no reason why we need to clutter our parser with extra keywords to support a non-standard feature extension. My understanding is that we will have to have those functions defined and user visible, and that we benefit from function overloading which is not in the standard. So there's no reason not to provide those function for arrays already, then extend to full multiset support. Given PostgreSQL overloading, yes, arrays are multisets as far as defining those standard compliant APIs is concerned. AFAIUI. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Use a separate pg_depend.deptype for extension membership?
On 02/04/2011 02:08 PM, Tom Lane wrote: The extensions patch currently records that an object is part of an extension by making a pg_depend entry with deptype 'i' (INTERNAL). While that has the behavior we want, I wonder whether it wouldn't be smarter in the long run to invent a new deptype for this purpose. We do not want people confusing module membership with actual internal dependencies, particularly not if kluges like pg_extension_flag_dump are going to be around. (I'm currently feeling that that function isn't going to make it into the committed patch, but sooner or later there are likely to be similar operations.) The main objection I can see to a new deptype is that it might confuse client-side code that examines pg_depend. But adding all these internal dependencies that don't mean quite what other internal dependencies do would likely confuse such code in more subtle ways anyhow. +1. I don't think we've ever promised not to invent new deptypes, nor should we. 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] Extensions support for pg_dump, patch v27
Tom Lane t...@sss.pgh.pa.us writes: What are the guc.c changes in this patch? They appear completely unrelated to the topic. Right. Didn't spot them. Sorry for the noise in the patch, it must be a merge problem in my repository. Do you want me to clean that up or is it already to late? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Use a separate pg_depend.deptype for extension membership?
On Fri, Feb 4, 2011 at 2:08 PM, Tom Lane t...@sss.pgh.pa.us wrote: The extensions patch currently records that an object is part of an extension by making a pg_depend entry with deptype 'i' (INTERNAL). While that has the behavior we want, I wonder whether it wouldn't be smarter in the long run to invent a new deptype for this purpose. +1. If we go with a new deptype, I was thinking of using 'm' (macro DEPENDENCY_MEMBER) but am not set on that. Have we been using any particular term to refer to the objects that belong to an extension? DEPENDENCY_EXTENSION? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Spread checkpoint sync
On Fri, Feb 4, 2011 at 2:08 PM, Greg Smith g...@2ndquadrant.com wrote: -The total number of buffers I'm computing based on the checkpoint writes being sorted it not a perfect match to the number reported by the checkpoint complete status line. Sometimes they are the same, sometimes not. Not sure why yet. My first guess would be that in the cases where it's not the same, some backend evicted the buffer before the background writer got to it. That's expected under heavy contention for shared_buffers. -The estimate for expected to need sync computed as a by-product of the checkpoint sorting is not completely accurate either. This particular one has a fairly large error in it, percentage-wise, being off by 3 with a total of 11. Presumably these are absorbed fsync requests that were already queued up before the checkpoint even started. So any time estimate I drive based off of this count is only going to be approximate. As previously noted, I wonder if we ought sync queued-up requests that don't require writes before beginning the write phase. -The order in which the sync phase processes files is unrelated to the order in which they are written out. Note that 17216.10 here, the biggest victim (cause?) of the I/O spike, isn't even listed among the checkpoint writes! That's awful. If more than 50% of the I/O is going to happen from one fsync() call, that seems to put a pretty pessimal bound on how much improvement we can hope to achieve here. Or am I missing something? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Use a separate pg_depend.deptype for extension membership?
Tom Lane t...@sss.pgh.pa.us writes:
The extensions patch currently records that an object is part of an
extension by making a pg_depend entry with deptype 'i' (INTERNAL).
While that has the behavior we want, I wonder whether it wouldn't
be smarter in the long run to invent a new deptype for this purpose.
I gave that some thoughs but as the expected behaviour already exists, I
figured it would be better for my patch to bend its head and see that
the extension's install script somehow creates INTERNAL objects.
Meaning, +1.
We do not want people confusing module membership with actual internal
dependencies, particularly not if kluges like pg_extension_flag_dump
are going to be around. (I'm currently feeling that that function
isn't going to make it into the committed patch, but sooner or later
there are likely to be similar operations.)
This function is there so that PostGIS is able to flag some of the
objects it installs as user data that needs to be part of the backups.
I'll take any better design, that's only what I was able to come up
with.
If we go with a new deptype, I was thinking of using 'm' (macro
DEPENDENCY_MEMBER) but am not set on that. Have we been using any
particular term to refer to the objects that belong to an extension?
Do you really think the new dependency type has to be re-usable easily
in the future? DEPENDENCY_EXTENSION ('e') would look fine by me.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] OCLASS_FOREIGN_TABLE support is incomplete
While poking at the extensions patch I noticed that doDeletion() and getObjectDescription() both seem to be missing cases for OCLASS_FOREIGN_TABLE. Surely this is broken. 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] OCLASS_FOREIGN_TABLE support is incomplete
On Fri, Feb 4, 2011 at 2:44 PM, Tom Lane t...@sss.pgh.pa.us wrote: While poking at the extensions patch I noticed that doDeletion() and getObjectDescription() both seem to be missing cases for OCLASS_FOREIGN_TABLE. Surely this is broken. I'll look into fixing this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Use a separate pg_depend.deptype for extension membership?
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
Tom Lane t...@sss.pgh.pa.us writes:
If we go with a new deptype, I was thinking of using 'm' (macro
DEPENDENCY_MEMBER) but am not set on that. Have we been using any
particular term to refer to the objects that belong to an extension?
Do you really think the new dependency type has to be re-usable easily
in the future? DEPENDENCY_EXTENSION ('e') would look fine by me.
Hmm ... Haas suggested that too, but to me it seems confusing: which way
does such a dependency point? But if others don't find it so, I'm
willing to go with the majority.
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] Extensions support for pg_dump, patch v27
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: What are the guc.c changes in this patch? They appear completely unrelated to the topic. Right. Didn't spot them. Sorry for the noise in the patch, it must be a merge problem in my repository. Do you want me to clean that up or is it already to late? No need, I'll just drop that file from the patch. 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] Re: patch: fix performance problems with repated decomprimation of varlena values in plpgsql
2011/2/4 Robert Haas robertmh...@gmail.com:
On Fri, Jan 28, 2011 at 2:19 PM, Pavel Stehule pavel.steh...@gmail.com
wrote:
2011/1/28 Robert Haas robertmh...@gmail.com:
On Tue, Jan 25, 2011 at 11:29 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
variant a) my first patch - detoast on first usage with avoiding to
useless detoast checking
variant b) my first patch - detoast on first usage without avoiding to
useless detoast checking
time for 1 - about 300 ms, a is bout 1.5% faster than b
time for 2 - about 3 ms, a is about 3% faster than b
This makes your approach sound pretty good, but it sounds like we
might need to find a better way to structure the code.
do you have a any idea?
At first blush, the should_be_detoasted flag looks completely
unnecessary to me. I don't see why we have to set a flag in one part
of the code to tell some other part of the code whether or not it
should detoast. Why is that necessary or a good idea? Why can't we
just make the decision at the point where we're detoasting?
a logic about detoasting isn't exactly simple, and you see on tests,
so bypass save a 3%. I can't to say, if it's much or less. Just take
a 3%. Isn't nothing simpler than remove these flags.
Also, I believe I'm agreeing with Tom when I say that
exec_eval_datum() doesn't look like the right place to do this. Right
now that function is a very simple switch, and I've found that it's
usually a bad idea to stick complex code into the middle of such
things. It looks like function only has three callers, so maybe we
should look at each caller individually and see whether it needs this
logic or not. For example, I'm guessing make_tuple_from_row()
doesn't, because it's only calling exec_eval_datum() so that it can
pass the results to heap_form_tuple(), which already contains some
logic to detoast in certain cases. It's not clear why we'd want
different logic here than we do anywhere else. The other callers are
exec_assign_value(), where this looks like it could be important to
first we have to decide when we will do a detoasting - there are two variants
a) when we write a toasted data to variable
b) when we use a toasted data
@a needs to modify: copy parameters and exec_assign_value,
@b needs to modify plpgsql_param_fetch or exec_eval_datum. Really
important is plpgsql_param_fetch, because it is only point, where we
know, so some Datum will be used, but wasn't detoasted yet. Problem is
wrong memory context. And plpgsql_param_fetch is callback, that is
emitted from different SPI functions so is hard to move a some lines
to function, that will run under good context.
There isn't necessary to detoast Datum, when value is only copied.
The disadvantage of @a can be so we remove 95% of pathologic cases,
and we create a 5% new. @b is terrible because the most important
point (main work) is done under parser memory context. I don't
understand you what is complex on (this code can be moved to
function). But @b needs only one place for detosting, because it's
nice centralised, that you and Tom dislike.
oldctx = MemoryContextSwitchTo(fcecxt);
if (!var-typbyval var-typlen == -1)
x = detoast_datum(var);
if (x != var)
{
pfree(var)
var = x;
}
MemoryContextSwitchTo(oldctx);
You can put this code to plpgsql_param_fetch or exec_eval_datum or
you have to copy this code two or three times.
I have not a idea how to design it well to by you and Tom happy :(
When I thinking about it, then I am sure, so best solution is really
global cache of detoasted values. It cannot be well solved on local
area. On local area I can do correct decision only when I know, so I
am inside a cycle and I work with some Datum repeatedly. Detoasting
on assign can has impacts on FOR stmt, iteration over cursors ...
FOR a,b,c IN SELECT * FROM foo
LOOP
if a THEN CONTINUE; END IF;
process b,c -- b and c are toastable values.
END LOOP;
avoid repeatedly detoasting the array, and plpgsql_param_fetch(),
which I'm not sure about, but perhaps you have an idea or can
benchmark it.
It's hard to benchmark it. I can create a use case, where @a win or @b
win. I believe so in almost all cases @a or @b will be better than
current state. And I afraid, so there can be situation where implicit
detoast can be bad.
Regards
Pavel Stehule
p.s. as one idea is a flag for function, that can to specify, if
inside function can be detoasting lazy or aggressive
.
--
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] Use a separate pg_depend.deptype for extension membership?
Tom Lane t...@sss.pgh.pa.us writes:
Dimitri Fontaine dimi...@2ndquadrant.fr writes:
Do you really think the new dependency type has to be re-usable easily
in the future? DEPENDENCY_EXTENSION ('e') would look fine by me.
Hmm ... Haas suggested that too, but to me it seems confusing: which way
does such a dependency point? But if others don't find it so, I'm
willing to go with the majority.
Well the behavior we want is the same as the DEPENDENCY_INTERNAL one, in
about all cases (e.g. DROP SCHEMA CASCADE). So I think it'd be easier
to stick with doing it the same. And then the need for specializing the
dependency kind name just raises too…
My 2¢ anyway,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] more buildfarm breakage
On Fri, Feb 4, 2011 at 1:16 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Feb 4, 2011 at 1:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: I agree, that is not cool. OK, changed. It looks like pipe.c is full of backend-specific error-reporting code. I could rewrite it to work in the frontend, but since it's obviously not used anywhere there it seems like an exercise in futility. The viable options appear to be: 1. Use #ifndef FRONTEND to dike out the entire file. 2. Make a small change to the Makefile to only compile this into the backend version of the library, and just skip it for the frontend version. 3. Move it into src/backend/port. My first thought is to go with #3. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Extensions support for pg_dump, patch v27
While I'm looking at this ... what is the rationale for treating rewrite rules as members of extensions, ie, why does the patch touch rewriteDefine.c? ISTM a rule is a property of a table and could not sensibly be an independent member of an extension. If there is a use for that, why are table constraints and triggers not given the same treatment? 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] Extensions support for pg_dump, patch v27
Tom Lane t...@sss.pgh.pa.us writes: While I'm looking at this ... what is the rationale for treating rewrite rules as members of extensions, ie, why does the patch touch rewriteDefine.c? ISTM a rule is a property of a table and could not sensibly be an independent member of an extension. If there is a use for that, why are table constraints and triggers not given the same treatment? I remember thinking I needed to do that for CREATE VIEW support while discovering PostgreSQL internals. Regards. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Per-column collation, the finale
Noah Misch wrote: On Thu, Feb 03, 2011 at 05:53:28PM +0200, Peter Eisentraut wrote: On tor, 2011-02-03 at 00:10 -0500, Noah Misch wrote: This is good stuff. I'll send you a new patch in a day or three for perhaps another round of performance tests. Some of the other issues above can perhaps be postponed for follow-up patches. I agree -- if the performance-when-unused gets solid, none of my other comments ought to hold things up. Here is a new patch. The main change is in varstr_cmp(), avoiding the calls to pg_newlocale_from_collation() when the default locale is used. This accounts for the performance regression in my tests. It also addresses some of your refactoring ideas. Looks good and tests well. I've attached the same benchmark script with updated timings, and I've marked the patch Ready for Committer. Nice to see that performance hit is removed now! -- 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] is_absolute_path incorrect on Windows
Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
I have reviewed is_absolute_path() and have implemented
path_is_relative_and_below_cwd() to cleanly handle cases like 'E:abc' on
Win32; patch attached.
This patch appears to remove some security-critical restrictions.
Why did you delete the path_contains_parent_reference calls?
They are now in path_is_relative_and_below_cwd(),
... and thus not invoked in the absolute-path case. This is a security
hole.
I don't see a general reason to prevent
.. in absolute paths, only relative ones.
load '/path/to/database/../../../path/to/anywhere'
Ah, good point. I was thinking about someone using .. in the part of
the path that is compared to /data or /log, but using it after that
would clearly be a security problem.
I have attached an updated patch that restructures the code to be
clearer and adds the needed checks.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c
index 381554d..afae9cb 100644
*** a/contrib/adminpack/adminpack.c
--- b/contrib/adminpack/adminpack.c
*** convert_and_check_filename(text *arg, bo
*** 73,85
canonicalize_path(filename); /* filename can change length here */
! /* Disallow .. in the path */
if (path_contains_parent_reference(filename))
ereport(ERROR,
! (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(reference to parent directory (\..\) not allowed;
!
! if (is_absolute_path(filename))
{
/* Allow absolute references within DataDir */
if (path_is_prefix_of_path(DataDir, filename))
--- 73,84
canonicalize_path(filename); /* filename can change length here */
! /* Disallow '/a/b/data/..' and 'a/b/..' */
if (path_contains_parent_reference(filename))
ereport(ERROR,
! (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(reference to parent directory (\..\) not allowed;
! else if (is_absolute_path(filename))
{
/* Allow absolute references within DataDir */
if (path_is_prefix_of_path(DataDir, filename))
*** convert_and_check_filename(text *arg, bo
*** 93,104
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(absolute path not allowed;
- return NULL; /* keep compiler quiet */
- }
- else
- {
- return filename;
}
}
--- 92,104
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(absolute path not allowed;
}
+ else if (!path_is_relative_and_below_cwd(filename))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg(path must be in or below the current directory;
+
+ return filename;
}
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 93bc401..63fc517 100644
*** a/src/backend/utils/adt/genfile.c
--- b/src/backend/utils/adt/genfile.c
*** convert_and_check_filename(text *arg)
*** 51,63
filename = text_to_cstring(arg);
canonicalize_path(filename); /* filename can change length here */
! /* Disallow .. in the path */
if (path_contains_parent_reference(filename))
ereport(ERROR,
! (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(reference to parent directory (\..\) not allowed;
!
! if (is_absolute_path(filename))
{
/* Allow absolute references within DataDir */
if (path_is_prefix_of_path(DataDir, filename))
--- 51,62
filename = text_to_cstring(arg);
canonicalize_path(filename); /* filename can change length here */
! /* Disallow '/a/b/data/..' and 'a/b/..' */
if (path_contains_parent_reference(filename))
ereport(ERROR,
! (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(reference to parent directory (\..\) not allowed;
! else if (is_absolute_path(filename))
{
/* Allow absolute references within DataDir */
if (path_is_prefix_of_path(DataDir, filename))
*** convert_and_check_filename(text *arg)
*** 70,81
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(absolute path not allowed;
- return NULL; /* keep compiler quiet */
- }
- else
- {
- return filename;
}
}
--- 69,81
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(absolute path not allowed;
}
+ else if (!path_is_relative_and_below_cwd(filename))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg(path must be in or below the current directory;
+
+ return filename;
}
diff --git a/src/include/port.h b/src/include/port.h
index 2020a26..5be42f5 100644
*** a/src/include/port.h
--- b/src/include/port.h
*** extern void join_path_components(char *r
***
Re: [HACKERS] Linux filesystem performance and checkpoint sorting
Greg, Thanks for doing these tests! So: Linux flavor? Kernel version? Disk system and PG directory layout? -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.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] SSI performance
I wrote: If this works, it would be a very minor change, which might eliminate a lot of that overhead for many common cases. With that change in place, I loaded actual data from one county for our most heavily searched table and searched it on the most heavily searched index. I returned actual data rather than a count, from 3654 rows. On a less trivial and more common query like this, the overhead of SSI tends to be lost in the noise: select nameL, nameF, nameM, suffix, countyNo, caseNo, sex from Party where searchName like 'WILLIAMS,%' order by countyNo, caseNo; repeatable read Time: 51.150 ms Time: 54.809 ms Time: 53.495 ms Time: 51.458 ms Time: 82.656 ms serializable Time: 54.105 ms Time: 52.765 ms Time: 52.852 ms Time: 69.449 ms Time: 52.089 ms Unfortunately for those who rely on count(*), it is about the worst case possible for highlighting SSI predicate locking overhead. -Kevin -- 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] SSI performance
I wrote: repeatable read [best] Time: 51.150 ms serializable [best] Time: 52.089 ms It occurred to me that taking the best time from each was likely to give a reasonable approximation of the actual overhead of SSI in this situation. That came out to about 1.8% in this (small) set of tests, which is right where earlier benchmarks of a heavy read load against fully-cached data put the SSI predicate locking overhead. That previous benchmarking involved letting things run overnight for several days in a row to accumulate hundreds of runs of decent length. While today's little test doesn't prove much, because of its size, the fact that it matches the numbers from the earlier, more rigorous tests suggests that we're probably still in the same ballpark. If you get into a load where there's actual disk access, I suspect that this overhead will be very hard to spot; the transaction rollback rate is going to become the dominant performance issue. -Kevin -- 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] Linux filesystem performance and checkpoint sorting
On 05/02/11 07:31, Greg Smith wrote: Switching to a new thread for this summary since there's some much more generic info here...at this point I've finished exploring the major Linux filesystem and tuning options I wanted to, as part of examining changes to the checkpoint code. You can find all the raw data at http://www.2ndquadrant.us/pgbench-results/index.htm Awesome! Very useful results. Are you going to do some runs with ext4? I'd be very interested to see how it compares (assuming that you are on a kernel version 2.6.32 or later so ext4 is reasonably stable...). Cheers Mark -- 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] Linux filesystem performance and checkpoint sorting
On Fri, Feb 4, 2011 at 12:31 PM, Greg Smith g...@2ndquadrant.com wrote: -Switching from ext3 to xfs gave over a 3X speedup on the smaller test set: from the 600-700 TPS range to around 2200 TPS. TPS rate on the larger data set actually slowed down a touch on XFS, around 10%. Still, such a huge win when it's better makes it easy to excuse the occasional cases where it's a bit slower. Did you see that they improved XFS scalability in 2.6.37? http://kernelnewbies.org/Linux_2_6_37#head-dfa29df2b21f5a72fb17f041a7356deeea3d159e Looks like there's more XFS improvements in store for 2.6.38. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] OpenVMS - an effort which needs guidance and support.
I see there was significant discussion back in February 2010 as to an OpenVMS port of PostgreSQL. I am interested in seeing PostgreSQL ported to OpenVMS as it will then faciliate other Open Source ports - like OpenBravo. HP OpenVMS Engineering is encouraging Open Source ports. A set of machines are available which are internet accessable. I would be willing to act as the focal point in the effort as I have significant OpenVMS experience as well as access to others who would be willing to lend an ear and possibly suggestions as well as to provide event support. In my research it appears that there have been maybe three or four efforts to get this effort started over the last 10 years or so. I have attempted to reach out to people in these efforts to get some understanding of what they tried - what worked and what did not. I personally would recommend an initial port via GNV. Then look to what might be necessary to move PostgreSQL outside of that environment. I would not recommend support for ODS-2 given the inherent Unix/Linux directory implementation within the code. I look forward to hearing from people in the PostgreSQL community as well as from others interested in this effort. Thanks, Bill. Bill Pedersen CCSS - Computer Consulting System Services 211 Ruth Drive Gaffney, SC 29341 Telephone: 864-490-8863 Mobile:408-892-5204 Facsimile: 866-394-9148 www: www.ccsscorp.com Skype Name: william.a.pedersen LinkedIn Profile: www.linkedin.com/in/billpedersen -- 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] multiset patch review
On Sat, Feb 5, 2011 at 04:24, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: In math class, maybe. But in programming, no. Multiset is a datatype. Array is a different datatype. There is no reason why we need to clutter our parser with extra keywords to support a non-standard feature extension. My understanding is that we will have to have those functions defined and user visible, and that we benefit from function overloading which is not in the standard. So there's no reason not to provide those function for arrays already, then extend to full multiset support. Given PostgreSQL overloading, yes, arrays are multisets as far as defining those standard compliant APIs is concerned. AFAIUI. Yes, I'd like to use overloading. Choosing arbitrary names increases learning costs for users. -- Itagaki Takahiro -- 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] Named restore points
On Tue, Feb 1, 2011 at 10:02 AM, Euler Taveira de Oliveira
eu...@timbira.com wrote:
Em 14-01-2011 17:41, Jaime Casanova escreveu:
Here is a patch that implements named restore points.
Sorry, I was swamped with work. :(
Your patch no longer applied so I rebased it and slightly modified it.
Review is below...
Hi,
Thanks for the review, i've been without internet connection for 4
days so i haven't seen the review until now...
+ The default is to recover to the end of the WAL log.
+ The precise stopping point is also influenced by
+ xref linkend=recovery-target-inclusive.
+ /para
This isn't valid. recovery_target_name are not influenced by
recovery_target_inclusive. Sentence removed.
good point! docs are boring so i was in automatic mode ;)
+ static char recoveryStopNamedRestorePoint[MAXFNAMELEN];
Is MAXFNAMELEN appropriate? AFAICS it is used for file name length. [Looking
at code...] It seems to be used for backup label too so it is not so
inappropriate.
right, i used it because it is used for backup label
+ else if (recoveryTarget == RECOVERY_TARGET_NAME)
+ snprintf(buffer, sizeof(buffer),
+ %s%u\t%s\t%s named restore point %s\n,
+ (srcfd 0) ? : \n,
+ parentTLI,
+ xlogfname,
+ recoveryStopAfter ? after : before,
+ recoveryStopNamedRestorePoint);
It doesn't matter if it is after or before the restore point. After/Before
only make sense when we're dealing with transaction or time. Removed.
you're right
else if (strcmp(item-name, recovery_target_xid) == 0)
{
+ /*
+ * if recovery_target_name specified, then this
overrides
+ * recovery_target_xid
+ */
+ if (recoveryTarget == RECOVERY_TARGET_NAME)
+ continue;
+
IMHO the right recovery precedence is xid - name - time. If you're
specifying xid that's because you know what you are doing. Name takes
precedence over time because it is easier to remember a name than a time. I
implemented this order in the updated patch.
actually i was expecting to hear opinions about this and i agree with you
+ recoveryTargetName = pstrdup(item-value);
I also added a check for long names.
ok
+ if ((record-xl_rmid == RM_XLOG_ID) (record_info ==
XLOG_RESTORE_POINT))
+ couldStop = true;
+
+ if (!couldStop)
+ return false;
+
I reworked this code path because it seems confusing.
it is... it was the result of debugging an stupid error on my side...
+ recordNamedRestorePoint = (xl_named_restore_points *)
XLogRecGetData(record);
+ recordXtime = recordNamedRestorePoint-xtime;
Why don't you store the named restore point here too? You will need it a few
lines below.
don't remember, will see
+ Datum
+ pg_create_restore_point(PG_FUNCTION_ARGS)
+ {
You should have added a check for long restore point names. Added in the
updated patch.
ok
+ ereport(NOTICE,
+ (errmsg(WAL archiving is not enabled; you must ensure
that WAL segments are copied through other means for restore points to be
usefull for you)));
+
Sentence was rewritten as WAL archiving is not enabled; you must ensure
that WAL segments are copied through other means to recover up to named
restore point.
sounds better, thanks
Finally, this is a nice feature iif we have a way to know what named restore
points are available. DBAs need to take note of this list (that is not good)
and the lazy ones will have a hard time to recover the right name (possibly
with a xlog dump tool).
So how could we store this information? Perhaps a file in
$PGDATA/pg_xlog/restore_label that contains the label (and possibly the WAL
location). Also it must have a way to transmit the restore_label when we add
another restore point. I didn't implement this part (Jaime?) and it seems as
important as the new xlog record type that is in the patch. It seems
complicate but I don't have ideas. Anyone? The restore point names could be
obtained by querying a function (say, pg_restore_point_names or
pg_restore_point_list).
IMHO, probably the best answer is a tool to retrieve that info... the
problem is that a restore_label file should be closely attached to
the WAL segment where the named restore point is... and a sql function
won't say anything about named restore points that are in archived WAL
segments...
I will mark this patch waiting on author because of those open issues.
I'm attaching the updated patch and two scripts that I used to play with the
patch.
ok, i will see you're reviewed version later today
--
Jaime Casanova
Re: [HACKERS] Unexpected page allocation behavior on insert-only tables
Tom Lane wrote: I wrote: In particular, now that there's a distinction between smgr flush and relcache flush, maybe we could associate targblock reset with smgr flush (only) and arrange to not flush the smgr level during ANALYZE --- basically, smgr flush would only be needed when truncating or reassigning the relfilenode. I think this might work out nicely but haven't chased the details. I looked into that a bit more and decided that it'd be a ticklish change: the coupling between relcache and smgr cache is pretty tight, and there just isn't any provision for having an smgr cache entry live longer than its owning relcache entry. Even if we could fix it to work reliably, this approach does nothing for the case where a backend actually exits after filling just part of a new page, as noted by Takahiro-san. The next most promising fix is to have RelationGetBufferForTuple tell the FSM about the new page immediately on creation. I made a draft patch for that (attached). It fixes Michael's scenario nicely --- all pages get filled completely --- and a simple test with pgbench didn't reveal any obvious change in performance. However there is clear *potential* for performance loss, due to both the extra FSM access and the potential for increased contention because of multiple backends piling into the same new page. So it would be good to do some real performance testing on insert-heavy scenarios before we consider applying this. Any volunteers? Note: patch is against HEAD but should work in 8.4, if you reverse out the use of the rd_targblock access macros. Is this something we want to address or should I just add it to the TODO? -- 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_dump directory archive format / parallel pg_dump
On Thu, Feb 3, 2011 at 11:46 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: I think we have 2 important technical issues here: * The consistency is not perfect. Each transaction is started with small delays in step 1, but we cannot guarantee no other transaction between them. This is exactly where the patch for synchronized snapshot comes into the game. See https://commitfest.postgresql.org/action/patch_view?id=480 * Can we inherit connections to child processes with fork() ? Moreover, we also need to pass running transactions to children. I wonder libpq is designed for such usage. As far as I know you can inherit sockets to a child program, as long as you make sure that after the fork only one, father or child, uses the socket, the other one should close it. But this wouldn't be a matter with the above mentioned patch anyway. It might be better to remove Windows-specific codes from the first try. I doubt Windows message queue is the best API in such console-based application. I hope we could use the same implementation for all platforms for inter-process/thread communication. Windows doesn't support pipes, but offers the message queues to exchange messages. Parallel pg_dump only exchanges messages in the form of DUMP 39209 or RESTORE OK 48 23 93, it doesn't exchange any large chunks of binary data, just these small textual messages. The messages also stay within the same process, they are just sent between the different threads. The windows part worked just fine when I tested it last time. Do you have any other technology in mind that you think is better suited? Joachim -- 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] compiler warning
Magnus Hagander wrote:
On Thu, Feb 3, 2011 at 04:40, Bruce Momjian br...@momjian.us wrote:
I am seeing the following compiler warning for the past few days:
? ? ? ?basebackup.c:213: warning: variable `ptr' might be clobbered by
? ? ? ?`longjmp' or `vfork'
and I see this comment in the file:
? ? ? ?/*
? ? ? ? * Actually do a base backup for the specified tablespaces.
? ? ? ? *
? ? ? ? * This is split out mainly to avoid complaints about variable
might be
? ? ? ? * clobbered by longjmp from stupider versions of gcc.
? ? ? ? */
Seems that isn't working as expected. ?I am using:
? ? ? ?gcc version 2.95.3 20010315 (release)
with -O1.
This is the same warning Tom fixed earlier. I have no idea what
actually causes it :(
I think it's somehow caused by the PG_ENSURE_ERROR_CLEANUP stuff.
Does it go away if you break everything inside the if
(opt-includewal) into it's own function? (Or at least everything
except the pq_putemptymessage() call, because moving that would make
the code very confusing)
I added the attached C comment so we know why the warning is generated.
We can remove it later if we want, but I want to have it if we get
reports about 9.1 problems.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index b5cda50..d94b61f 100644
*** a/src/backend/replication/basebackup.c
--- b/src/backend/replication/basebackup.c
*** perform_base_backup(basebackup_options *
*** 217,222
--- 217,228
ptr.xlogid = logid;
ptr.xrecoff = logseg * XLogSegSize + TAR_SEND_SIZE * i;
+ /*
+ * Some old compilers, e.g. 2.95.3/x86, think that passing
+ * a struct in the same function as a longjump might clobber
+ * a variable. bjm 2011-02-04
+ * http://lists.apple.com/archives/xcode-users/2003/Dec//msg00051.html
+ */
XLogRead(buf, ptr, TAR_SEND_SIZE);
if (pq_putmessage('d', buf, TAR_SEND_SIZE))
ereport(ERROR,
--
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] Linux filesystem performance and checkpoint sorting
Mark Kirkwood wrote: Are you going to do some runs with ext4? I'd be very interested to see how it compares (assuming that you are on a kernel version 2.6.32 or later so ext4 is reasonably stable...). Yes, before I touch this system significantly I'll do ext4 as well, and this is running the Ubuntu 10.04 2.6.32 kernel so ext4 should be stable enough. I have some PostgreSQL work that needs to get finished first though. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books -- 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] Linux filesystem performance and checkpoint sorting
Josh Berkus wrote: So: Linux flavor? Kernel version? Disk system and PG directory layout? OS configuration and PostgreSQL settings are saved into the output from the later runs (I added that somewhere in the middle): http://www.2ndquadrant.us/pgbench-results/294/pg_settings.txt That's Ubuntu 10.04, kernel 2.6.32. There is a test rig bug that queries the wrong PostgreSQL settings in the later ones, but they didn't change after #294 here. The kernel configuration stuff is accurate through, which confirms exactly what settings for the dirty_* parameters was effective for each during the tests I was changing those around. 16GB of RAM, 8 Hyperthreaded cores (4 real ones) via Intel i7-870. Areca ARC-1210 controller, 256MB of cache. Filesystem 1K-blocks Used Available Use% Mounted on /dev/sda1 40G 7.5G 30G 20% / /dev/md1 838G 15G 824G 2% /stripe /dev/sdd1 149G 2.1G 147G 2% /xlog /stripe is a 3 disk RAID0, setup to only use the first section of the drive (short-stroked). That makes its performance a little more like a small SAS disk, rather than the cheapo 7200RPM SATA drives they actually are (Western Digital 640GB WD6400AAKS-65A7B). /xlog is a single disk, 160GB WD1600AAJS-00WAA. OS, server logs, and test results information all go to the root filesystem on a different drive. My aim was to get similar performance to what someone with an 8-disk RAID10 array might see, except without the redundancy. Basic entry-level database server here in 2011. bonnie++ on the main database disk: read 301MB/s write 215MB/s, seeks 423.4/second. Measured around 10K small commits/second to prove the battery-backed write cache works fine. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books -- 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] ALTER TYPE 2: skip already-provable no-work rewrites
On Thu, Jan 27, 2011 at 2:48 PM, Noah Misch n...@leadboat.com wrote: Done as attached. This preserves compatibility with our current handling of composite type dependencies. The rest you've seen before. OK, so I took a look at this in more detail today. The current logic for table rewrites looks like this: 1. If we're changing the data type of a column, or adding a column with a default, or adding/dropping OIDs, rewrite the table. Stop. 2. Otherwise, if we're adding a constraint or NOT NULL, scan the table and check constraints. 3. If we're changing tablespaces, copy the table block-by-block. It seems to me that the revised logic needs to look like this: 1. If we're changing the data type of a column and the existing contents are not binary coercible to the new contents, or if we're adding a column with a default or adding/dropping OIDs, rewrite the table. Stop. 2. Otherwise, if we're adding a constraint or NOT NULL, scan the table and check constraints. 3. If we're changing the data type of a column in the table, reindex the table. 4. If we're changing tablespaces, copy the table block-by-block. I might be missing something, but I don't see that the patch includes step #3, which I think is necessary. For example, citext is binary coercible to text, but you can't reuse the index because the comparison function is different. Of course, if you're changing the type of a column to its already-current type, you can skip #3, but if that's the only case we can optimize, it's not much of an accomplishment. I guess this gets back to the ALTER TYPE 7 patch, which I haven't looked at in detail, but I have a feeling it may be controversial. Another problem here is that if you have to do both #2 and #3, you might have been better off (or just as well off) doing #1, unless you can somehow jigger things so that the same scan does both the constraint checks and the index rebuild. That doesn't look simple. Thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] OCLASS_FOREIGN_TABLE support is incomplete
On Fri, Feb 4, 2011 at 2:46 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Feb 4, 2011 at 2:44 PM, Tom Lane t...@sss.pgh.pa.us wrote: While poking at the extensions patch I noticed that doDeletion() and getObjectDescription() both seem to be missing cases for OCLASS_FOREIGN_TABLE. Surely this is broken. I'll look into fixing this. Err... wait. Actually, I think the right thing to do might be to remove OCLASS_FOREIGN_TABLE altogether. I don't think it's actually used for anything. For a foreign table, we use OCLASS_CLASS, just as we do for indexes and sequences. I think that's just leftover crap that I failed to rip out. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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_dump directory archive format / parallel pg_dump
On Sat, Feb 5, 2011 at 04:50, Joachim Wieland j...@mcknight.de wrote: On Thu, Feb 3, 2011 at 11:46 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: It might be better to remove Windows-specific codes from the first try. I doubt Windows message queue is the best API in such console-based application. I hope we could use the same implementation for all platforms for inter-process/thread communication. Windows doesn't support pipes, but offers the message queues to exchange messages. Parallel pg_dump only exchanges messages in the form of DUMP 39209 or RESTORE OK 48 23 93, it doesn't exchange any large chunks of binary data, just these small textual messages. The messages also stay within the same process, they are just sent between the different threads. The windows part worked just fine when I tested it last time. Do you have any other technology in mind that you think is better suited? Haven't been following this thread in details or read the code.. But our /port directory contains a pipe() implementation for Windows, that's used for the syslogger at least. Look in the code for pgpipe(). If using that one works, then that should probably be used rather than something completely custom. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
