Re: [HACKERS] Doc patch to note which system catalogs have oids
On Tue, 2012-10-02 at 10:46 -0500, Karl O. Pinc wrote: I am now submitting patches to the commitfest for review. (I'm not sure how I missed this.) I prefer this version of the patch. I also attached an alternative version that may address Tom's concern by noting that the OIDs are hidden in the description. Marking ready for committer. Regards, Jeff Davis *** a/doc/src/sgml/catalogs.sgml --- b/doc/src/sgml/catalogs.sgml *** *** 427,432 --- 427,439 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldamname/structfield/entry entrytypename/type/entry entry/entry *** *** 683,688 --- 690,702 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldamopfamily/structfield/entry entrytypeoid/type/entry entryliterallink linkend=catalog-pg-opfamilystructnamepg_opfamily/structname/link.oid/literal/entry *** *** 819,824 --- 833,845 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldamprocfamily/structfield/entry entrytypeoid/type/entry entryliterallink linkend=catalog-pg-opfamilystructnamepg_opfamily/structname/link.oid/literal/entry *** *** 902,907 --- 923,935 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldadrelid/structfield/entry entrytypeoid/type/entry entryliterallink linkend=catalog-pg-classstructnamepg_class/structname/link.oid/literal/entry *** *** 1257,1262 --- 1285,1298 /thead tbody + + row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + row entrystructfieldrolname/structfield/entry entrytypename/type/entry *** *** 1462,1467 --- 1498,1510 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldcastsource/structfield/entry entrytypeoid/type/entry entryliterallink linkend=catalog-pg-typestructnamepg_type/structname/link.oid/literal/entry *** *** 1577,1582 --- 1620,1632 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldrelname/structfield/entry entrytypename/type/entry entry/entry *** *** 1984,1989 --- 2034,2046 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldconname/structfield/entry entrytypename/type/entry entry/entry *** *** 2250,2255 --- 2307,2319 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldcollname/structfield/entry entrytypename/type/entry entry/entry *** *** 2350,2355 --- 2414,2426 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfieldconname/structfield/entry entrytypename/type/entry entry/entry *** *** 2443,2448 --- 2514,2526 tbody row + entrystructfieldoid/structfield/entry + entrytypeoid/type/entry + entry/entry + entryRow identifier (hidden attribute; must be explicitly selected)/entry + /row + + row entrystructfielddatname/structfield/entry
Re: [HACKERS] gistchoose vs. bloat
On Fri, 2012-12-14 at 01:03 +0400, Alexander Korotkov wrote: Hi! On Sat, Dec 8, 2012 at 7:05 PM, Andres Freund and...@2ndquadrant.com wrote: I notice there's no documentation about the new reloption at all? Thanks for notice! I've added small description to docs in the attached patch. Here is an edited version of the documentation note. Please review to see if you like my version. Also, I fixed a compiler warning. My tests showed a significant reduction in the size of a gist index with many of the same penalty values. The run times showed mixed results, however, and I didn't dig in much further because you've already done significant testing. Marking this one ready again. Regards, Jeff Davis gist_choose_bloat-0.5A.patch.gz Description: GNU Zip compressed 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] Performance Improvement by reducing WAL for Update Operation
Hello, I took the perfomance figures for this patch. CentOS6.3/Core i7 wal_level = archive, checkpoint_segments = 30 / 5min A. Vanilla pgbench, postgres is HEAD B. Vanilla pgbench, postgres is with this patch (wal_update_changes_lz_v5) C. Modified pgbench(Long text), postgres is HEAD D. Modified pgbench(Long text), postgres is with this patch Running doing pgbench -s 10 -i, pgbench -c 20 -T 2400 #trans/s WAL MB WAL kB/tran 1A 437 1723 1.68 1B 435 (1% slower than A) 1645 1.61 (96% of A) 1C 149 507314.6 1D 174 (17% faster than C) 523212.8(88% of C) Restoring with the wal archives yielded during the first test. Recv sec s/trans 2A 61 0.0581 2B 62 0.0594 (2% slower than A) 2C 287 0.805 2D 314 0.750 (7% faster than C) For vanilla pgbench, WAL size shrinks slightly and performance seems very slightly worse than unpatched postgres(1A vs. 1B). It can be safely say that no harm on performance even outside of the effective range of this patch. On the other hand, the performance gain becomes 17% within the effective range (1C vs. 1D). Recovery performance looks to have the same tendency. It looks to produce very small loss outside of the effective range (2A vs. 2B) and significant gain within (2C vs. 2D ). As a whole, this patch brings very large gain in its effective range - e.g. updates of relatively small portions of tuples, but negligible loss of performance is observed outside of its effective range. I'll mark this patch as 'Ready for Committer' as soon as I get finished confirming the mod patch. == I think new naming I have done are more meaningful, do you think I should revert to previous patch one's. New naming is more meaningful, and a bit long. I don't think it should be reverted. Looking into wal_update_changes_mod_lz_v6.patch, I understand that this patch experimentally adds literal data segment which have more than single byte in PG-LZ algorithm. According to pglz_find_match, memCMP is slower than 'while(*s *s == *d)' if len 16 and I suppose it is probably true at least for 4 byte length data. This is also applied on encoding side. If this mod does no harm to performance, I want to see this applied also to pglz_comress. Where in pglz_comress(), do you want to see similar usage? Or do you want to see such use in function heap_attr_get_length_and_check_equals(), where it compares 2 attributes. My point was the format for literal segments. It seems to reduce about an eighth of literal segments. But the effectiveness under real environment does not promising.. Forget it. It's just a fancy. regards, -- Kyotaro Horiguchi 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] gistchoose vs. bloat
On Fri, Dec 14, 2012 at 12:46 PM, Jeff Davis pg...@j-davis.com wrote: Thanks for notice! I've added small description to docs in the attached patch. Here is an edited version of the documentation note. Please review to see if you like my version. Edited version looks good for me. Also, I fixed a compiler warning. Thanks! -- With best regards, Alexander Korotkov.
Re: [HACKERS] Identity projection
On 12.11.2012 12:07, Kyotaro HORIGUCHI wrote: Hello, This is new version of identity projection patch. Reverted projectionInfo and ExecBuildProjectionInfo. Identity projection is recognized directly in ExecGroup, ExecResult, and ExecWindowAgg. nodeAgg is reverted because I couldn't make it sane.. The following is the result of performance test posted before in order to show the source of the gain. Hmm, this reminds me of the discussion on removing useless Limit nodes: http://archives.postgresql.org/pgsql-performance/2012-12/msg00127.php. The optimization on Group, WindowAgg and Agg nodes doesn't seem that important, the cost of doing the aggregation/grouping is likely overwhelming the projection cost, and usually you do projection in grouping/aggregation anyway. But makes sense for Result. For Result, I think you should aim to remove the useless Result node from the plan altogether. And do the same for useless Limit nodes. - Heikki -- 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] logical decoding - GetOldestXmin
On 2012-12-13 23:35:00 +, Simon Riggs wrote: On 13 December 2012 22:37, Andres Freund and...@2ndquadrant.com wrote: On 2012-12-13 17:29:06 -0500, Robert Haas wrote: On Thu, Dec 13, 2012 at 3:03 PM, Andres Freund and...@2ndquadrant.com wrote: It moves a computation of the sort of: result -= vacuum_defer_cleanup_age; if (!TransactionIdIsNormal(result)) result = FirstNormalTransactionId; inside ProcArrayLock. But I can't really imagine that to be relevant... I can. Go look at some of the 9.2 optimizations around GetSnapshotData(). Those made a BIG difference under heavy concurrency and they were definitely micro-optimization. For example, the introduction of NormalTransactionIdPrecedes() was shockingly effective. But GetOldestXmin() should be called less frequently than GetSnapshotData() by several orders of magnitudes. I don't really see it being used in any really hot code paths? Maybe, but that calculation doesn't *need* to be inside the lock, that is just a consequence of the current coding. I am open to suggestion how to do that in a way we a) can hold the lock already (to safely nail the global xmin to the current value) b) without duplicating all the code. Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Logical decoding exported base snapshot
On 2012-12-13 21:40:43 +0100, Andres Freund wrote: On 2012-12-13 11:02:06 -0500, Steve Singer wrote: On 12-12-12 06:20 AM, Andres Freund wrote: Possible solutions: 1) INIT_LOGICAL_REPLICATION waits for an answer from the client that confirms that logical replication initialization is finished. Before that the walsender connection cannot be used for anything else. 2) we remove the snapshot as soon as any other commend is received, this way the replication connection stays usable, e.g. to issue a START_LOGICAL_REPLICATION in parallel to the initial data dump. In that case the snapshot would have to be imported *before* the next command was received as SET TRANSACTION SNAPSHOT requires the source transaction to be still open. Option 2 sounds more flexible. Is it more difficult to implement? No, I don't think so. It's a bit more intrusive in that it requires knowledge about logical replication in more parts of walsender, but it should be ok. Note btw, that my description of 1) was easy to misunderstand. The that in Before that the walsender connection cannot be used for anything else. is the answer from the client, not the usage of the exported snapshot. Once the snapshot has been iimported into other session(s) the source doesn't need to be alive anymore. Does that explanation change anything? I think I understood you were saying the walsender connection can't be used for anything else (ie streaming WAL) until the exported snapshot has been imported. I think your clarification is still consistent with this? Yes, thats correct. WIth option 2 I can still get the option 1 behaviour by not sending the next command to the walsender until I am done importing the snapshot. However if I want to start processing WAL before the snapshot has been imported I can do that with option 2. I am not sure I would need to do that, I'm just saying having the option is more flexible. True. Still not sure whats better, but since youre slightly leaning towards 2) I am going to implement that. Pushed and lightly tested. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Performance Improvement by reducing WAL for Update Operation
On Friday, December 14, 2012 2:32 PM Kyotaro HORIGUCHI wrote: Hello, I took the perfomance figures for this patch. CentOS6.3/Core i7 wal_level = archive, checkpoint_segments = 30 / 5min A. Vanilla pgbench, postgres is HEAD B. Vanilla pgbench, postgres is with this patch (wal_update_changes_lz_v5) C. Modified pgbench(Long text), postgres is HEAD D. Modified pgbench(Long text), postgres is with this patch Running doing pgbench -s 10 -i, pgbench -c 20 -T 2400 #trans/s WAL MB WAL kB/tran 1A 437 1723 1.68 1B 435 (1% slower than A) 1645 1.61 (96% of A) 1C 149 507314.6 1D 174 (17% faster than C) 523212.8(88% of C) Restoring with the wal archives yielded during the first test. Recv sec s/trans 2A 61 0.0581 2B 62 0.0594 (2% slower than A) 2C 287 0.805 2D 314 0.750 (7% faster than C) For vanilla pgbench, WAL size shrinks slightly and performance seems very slightly worse than unpatched postgres(1A vs. 1B). It can be safely say that no harm on performance even outside of the effective range of this patch. On the other hand, the performance gain becomes 17% within the effective range (1C vs. 1D). Recovery performance looks to have the same tendency. It looks to produce very small loss outside of the effective range (2A vs. 2B) and significant gain within (2C vs. 2D ). As a whole, this patch brings very large gain in its effective range - e.g. updates of relatively small portions of tuples, but negligible loss of performance is observed outside of its effective range. I'll mark this patch as 'Ready for Committer' as soon as I get finished confirming the mod patch. Thank you very much. With Regards, Amit Kapila. -- 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] WIP patch for hint bit i/o mitigation
On Thursday, December 13, 2012 8:02 PM Merlin Moncure wrote: On Thu, Dec 13, 2012 at 7:06 AM, Hari Babu haribabu.ko...@huawei.com wrote: Please find the review of the patch. Thanks for detailed review! Basic stuff: - Patch applies with offsets. - Compiles cleanly with no warnings - Regression Test pass. Code Review: - 1. Better to set the hint bits for the tuples in a page, if the page is already dirty. This is true today but likely less true if/when page checksums come out. Also it complicates the code a little bit. Default tables select : 64980.73614964550.118693 Unlogged tables select: 64874.97433464550.118693 So it looks like the extra tests visibility routines are causing 0.7% performance hit. Multiple transaction bulk inserts: Select performance (refer script -1 2 which attached) sequential scan: 6.4926806.382014 Index scan: 1.3868511.36234 Single transaction bulk inserts: Select performance (refer script - 3 4 which attached) sequential scan: 6.49319 6.3800147 Index scan: 1.3841211.3615277 The performance hit is higher here. Almost 2%. This is troubling. Long transaction open then Vacuum select performance in milli seconds. (refer reports output) Testcase - 3: Single Vacuum Perf : 128.302 ms 181.292 ms Single select perf : 214.107 ms 177.057 ms Total: 342.409 ms 358.349 ms I was not able to find the reason why in some of cases results are low so please use the attached scripts to validate the same. I need to validate the vacuum results. It's possible that this is solvable by tweaking xmin check inside vacuum. Assuming that's fixed, the question stands: do the results justify the change? I'd argue 'maybe' We can try with change (assuming change is small) and see if the performance gain is good, then discuss whether it really justifies. I think the main reason for Vacuum performance hit is that in the test pages are getting dirty only due to setting of hint bit by Vacuum. -- I'd like to see the bulk insert performance hit reduced if possible. I think if we can improve performance for bulk-insert case, then this patch has much more value. With Regards, Amit Kapila. -- 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
Did any (committed?) code result from this thread ? On 11/10/2011 09:13 PM, Peter Eisentraut wrote: On tis, 2011-11-08 at 16:08 -0500, Andrew Dunstan wrote: On 03/01/2011 11:50 AM, Peter Eisentraut wrote: On fre, 2011-02-11 at 16:49 +0100, Jan Urbański wrote: I believe it's (b). But as we don't have time for that discussion that late in the release cycle, I think we need to consider it identical to (c). As I previously mentioned, I think that there should be an SQL-level way to tie together languages and types. I previously mentioned the SQL-standard command CREATE TRANSFORM as a possibility. I've had this on my PL/Python TOTHINK list for a while. Thankfully you removed all the items ahead of this one, so I'll think of something to do in 9.2. Of course we'll be able to use the actual transform code that you already wrote. Peter, Did you make any progress on this? No, but it's still somewhere on my list. I saw your blog post related to this. I think the first step would be to set up some catalog infrastructure (without DDL commands and all that overhead), and try to adapt the big case statement of an existing language to that, and then check whether that works, performance, etc. Some other concerns of the top of my head: - Arrays: Would probably not by handled by that. So this would not be able to handle, for example, switching the array handling behavior in PL/Perl to ancient compatible mode. - Range types: no idea I might work on this, but not before December, would be my guess. -- 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] Multiple --table options for other commands
On 12/13/2012 11:02:56 PM, Josh Kupershmidt wrote: On Thu, Dec 13, 2012 at 9:03 PM, Karl O. Pinc k...@meme.com wrote: Sorry to be so persnickety, and unhelpful until now. It seemed like it should be doable, but something was going wrong between keyboard and chair. I guess I should be doing this when better rested. No problem, here is v5 with changed synopses. The clusterdb synopsis had tabs in it, which I understand is frowned upon in the docs. I've fixed this. It looks good to me, passes check and so forth. Attached is a v6 patch, with no tabs in docs and based off the latest head. I'm marking it ready for committer. Regards, Karl k...@meme.com Free Software: You don't pay back, you pay forward. -- Robert A. Heinlein diff --git a/doc/src/sgml/ref/clusterdb.sgml b/doc/src/sgml/ref/clusterdb.sgml index 097ea91..1316932 100644 --- a/doc/src/sgml/ref/clusterdb.sgml +++ b/doc/src/sgml/ref/clusterdb.sgml @@ -24,7 +24,17 @@ PostgreSQL documentation commandclusterdb/command arg rep=repeatreplaceableconnection-option/replaceable/arg group choice=optarg choice=plainoption--verbose/option/argarg choice=plainoption-v/option/arg/group - arg choice=optgroup choice=plainarg choice=plainoption--table/option/argarg choice=plainoption-t/option/arg/group replaceabletable/replaceable /arg + + arg choice=plain rep=repeat + arg choice=opt + group choice=plain + arg choice=plainoption--table/option/arg + arg choice=plainoption-t/option/arg + /group + replaceabletable/replaceable + /arg + /arg + arg choice=optreplaceabledbname/replaceable/arg /cmdsynopsis @@ -117,6 +127,8 @@ PostgreSQL documentation listitem para Cluster replaceable class=parametertable/replaceable only. +Multiple tables can be clustered by writing multiple +option-t/ switches. /para /listitem /varlistentry diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml index f4668e7..0d73294 100644 --- a/doc/src/sgml/ref/pg_restore.sgml +++ b/doc/src/sgml/ref/pg_restore.sgml @@ -400,7 +400,8 @@ termoption--table=replaceable class=parametertable/replaceable/option/term listitem para -Restore definition and/or data of named table only. This can be +Restore definition and/or data of named table only. Multiple tables +may be specified with multiple option-t/ switches. This can be combined with the option-n/option option to specify a schema. /para /listitem diff --git a/doc/src/sgml/ref/reindexdb.sgml b/doc/src/sgml/ref/reindexdb.sgml index 781012f..3ba9951 100644 --- a/doc/src/sgml/ref/reindexdb.sgml +++ b/doc/src/sgml/ref/reindexdb.sgml @@ -23,20 +23,27 @@ PostgreSQL documentation cmdsynopsis commandreindexdb/command arg rep=repeatreplaceableconnection-option/replaceable/arg - group choice=opt -group choice=plain - arg choice=plainoption--table/option/arg - arg choice=plainoption-t/option/arg -/group -replaceabletable/replaceable - /group - group choice=opt -group choice=plain - arg choice=plainoption--index/option/arg - arg choice=plainoption-i/option/arg -/group -replaceableindex/replaceable - /group + + arg choice=plain rep=repeat +arg choice=opt + group choice=plain + arg choice=plainoption--table/option/arg + arg choice=plainoption-t/option/arg + /group + replaceabletable/replaceable +/arg + /arg + + arg choice=plain rep=repeat +arg choice=opt + group choice=plain + arg choice=plainoption--index/option/arg + arg choice=plainoption-i/option/arg + /group + replaceableindex/replaceable +/arg + /arg + arg choice=optreplaceabledbname/replaceable/arg /cmdsynopsis @@ -128,6 +135,8 @@ PostgreSQL documentation listitem para Recreate replaceable class=parameterindex/replaceable only. +Multiple indexes can be recreated by writing multiple +option-i/ switches. /para /listitem /varlistentry @@ -158,6 +167,8 @@ PostgreSQL documentation listitem para Reindex replaceable class=parametertable/replaceable only. +Multiple tables can be reindexed by writing multiple +option-t/ switches. /para /listitem /varlistentry diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml index c60ba44..a5216ec 100644 --- a/doc/src/sgml/ref/vacuumdb.sgml +++ b/doc/src/sgml/ref/vacuumdb.sgml @@ -24,14 +24,18 @@ PostgreSQL documentation commandvacuumdb/command arg rep=repeatreplaceableconnection-option/replaceable/arg arg rep=repeatreplaceableoption/replaceable/arg - arg choice=opt -group choice=plain - arg choice=plainoption--table/option/arg - arg choice=plainoption-t/option/arg -/group -replaceabletable/replaceable -arg
[HACKERS] Assert for frontend programs?
As I'm working through the parallel dump patch, I notice this in one of
the header files:
#ifdef USE_ASSERT_CHECKING
#define Assert(condition) \
if (!(condition)) \
{ \
write_msg(NULL, Failed assertion in %s, line %d\n, \
__FILE__, __LINE__); \
abort();\
}
#else
#define Assert(condition)
#endif
I'm wondering if we should have something like this centrally (e.g. in
postgres_fe.h)? I can certainly see people wanting to be able to use
Assert in frontend programs generally, and it makes sense to me not to
make everyone roll their own.
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] Use gcc built-in atomic inc/dec in lock.c
On 12/13/2012 12:19 AM, Peter Geoghegan wrote: On 12 December 2012 22:11, Mikko Tiihonen mikko.tiiho...@nitorcreations.com wrote: noticed a XXX: It might be worth considering using an atomic fetch-and-add instruction here, on architectures where that is supported. in lock.c Here is my first try at using it. That's interesting, but I have to wonder if there is any evidence that this *is* actually helpful to performance. One of my open questions listed in the original email was request for help on creating a test case that exercise the code path enough so that it any improvements can be measured. But apart from performance I think there are two other aspects to consider: 1) Code clarity: I think the lock.c code is easier to understand after the patch 2) Future possibilities: having the atomic_inc/dec generally available allows other performance critical parts of postgres take advantage of them in the future -Mikko -- 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] Doc patch to note which system catalogs have oids
On 12/14/2012 02:04:45 AM, Jeff Davis wrote: On Tue, 2012-10-02 at 10:46 -0500, Karl O. Pinc wrote: I am now submitting patches to the commitfest for review. (I'm not sure how I missed this.) I prefer this version of the patch. I also attached an alternative version that may address Tom's concern by noting that the OIDs are hidden in the description. For the record, the preferred version referred to above is: oid_doc_v4.patch Marking ready for committer. Thanks! Karl k...@meme.com Free Software: You don't pay back, you pay forward. -- Robert A. Heinlein -- 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] Doc patch, index search_path where it's used to secure functions
On 12/13/2012 10:05:15 PM, Peter Eisentraut wrote: The other configuration parameters are all indexed as x_y_z configuration parameter, so I've kept search_path aligned with that. I have applied your other changes, so I think it's good now. Let me know if you feel additional changes should be made. I like the way it has come out. Thanks! Karl k...@meme.com Free Software: You don't pay back, you pay forward. -- Robert A. Heinlein -- 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] Assert for frontend programs?
Andrew Dunstan and...@dunslane.net writes:
As I'm working through the parallel dump patch, I notice this in one of
the header files:
#ifdef USE_ASSERT_CHECKING
#define Assert(condition) \
if (!(condition)) \
{ \
write_msg(NULL, Failed assertion in %s, line %d\n, \
__FILE__, __LINE__); \
abort();\
}
#else
#define Assert(condition)
#endif
I'm wondering if we should have something like this centrally (e.g. in
postgres_fe.h)? I can certainly see people wanting to be able to use
Assert in frontend programs generally, and it makes sense to me not to
make everyone roll their own.
+1, especially if the hand-rolled versions are likely to be as bad as
that one (dangling else, maybe some other issues I'm not spotting
in advance of caffeine consumption). I've wished for frontend Assert
a few times myself, but never bothered to make it happen.
Although I think we had this discussion earlier and it stalled at
figuring out exactly what the print error part of the macro ought
to be. The above is obviously pg_dump-specific. Perhaps
fprintf(stderr,...) would be sufficient, though -- it's not like
tremendous user friendliness ought to be necessary here.
Also, I think the message really has to include some string-ified
version of the assertion condition --- the line number alone is pretty
unhelpful when looking at field reports of uncertain provenance.
BTW, I think psql already has a psql_assert.
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] Use gcc built-in atomic inc/dec in lock.c
On Fri, Dec 14, 2012 at 9:33 AM, Mikko Tiihonen mikko.tiiho...@nitorcreations.com wrote: On 12/13/2012 12:19 AM, Peter Geoghegan wrote: On 12 December 2012 22:11, Mikko Tiihonen mikko.tiiho...@nitorcreations.com wrote: noticed a XXX: It might be worth considering using an atomic fetch-and-add instruction here, on architectures where that is supported. in lock.c Here is my first try at using it. That's interesting, but I have to wonder if there is any evidence that this *is* actually helpful to performance. One of my open questions listed in the original email was request for help on creating a test case that exercise the code path enough so that it any improvements can be measured. But apart from performance I think there are two other aspects to consider: 1) Code clarity: I think the lock.c code is easier to understand after the patch 2) Future possibilities: having the atomic_inc/dec generally available allows other performance critical parts of postgres take advantage of them in the future This was actually attempted a little while back; a spinlock was replaced with a few atomic increment and decrement calls for managing the refcount and other things on the freelist. It helped or hurt depending on contention but the net effect was negative. On reflection I think that was because that the assembly 'lock' instructions are really expensive relative to the others: so it's not safe to assume that say 2-3 gcc primitive increment calls are cheaper that a spinlock. merlin -- 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] Assert for frontend programs?
On 14.12.2012 17:54, Tom Lane wrote:
Andrew Dunstanand...@dunslane.net writes:
As I'm working through the parallel dump patch, I notice this in one of
the header files:
#ifdef USE_ASSERT_CHECKING
#define Assert(condition) \
if (!(condition)) \
{ \
write_msg(NULL, Failed assertion in %s, line %d\n, \
__FILE__, __LINE__); \
abort();\
}
#else
#define Assert(condition)
#endif
I'm wondering if we should have something like this centrally (e.g. in
postgres_fe.h)? I can certainly see people wanting to be able to use
Assert in frontend programs generally, and it makes sense to me not to
make everyone roll their own.
+1, especially if the hand-rolled versions are likely to be as bad as
that one (dangling else, maybe some other issues I'm not spotting
in advance of caffeine consumption). I've wished for frontend Assert
a few times myself, but never bothered to make it happen.
+1, I just ran into this while working on Andres' xlogreader patch.
xlogreader uses Assert(), and it's supposed to work in a stand-alone
program.
Although I think we had this discussion earlier and it stalled at
figuring out exactly what the print error part of the macro ought
to be. The above is obviously pg_dump-specific. Perhaps
fprintf(stderr,...) would be sufficient, though -- it's not like
tremendous user friendliness ought to be necessary here.
Also, I think the message really has to include some string-ified
version of the assertion condition --- the line number alone is pretty
unhelpful when looking at field reports of uncertain provenance.
BTW, I think psql already has a psql_assert.
psql_assert looks like this:
#ifdef USE_ASSERT_CHECKING
#include assert.h
#define psql_assert(p) assert(p)
#else
...
On my Linux system, a failure looks like this:
~$ ./a.out
a.out: a.c:5: main: Assertion `1==2' failed.
Aborted
That seems fine to me.
- Heikki
--
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 gcc built-in atomic inc/dec in lock.c
On 12/14/2012 05:55 PM, Merlin Moncure wrote: On Fri, Dec 14, 2012 at 9:33 AM, Mikko Tiihonen mikko.tiiho...@nitorcreations.com wrote: On 12/13/2012 12:19 AM, Peter Geoghegan wrote: On 12 December 2012 22:11, Mikko Tiihonen mikko.tiiho...@nitorcreations.com wrote: noticed a XXX: It might be worth considering using an atomic fetch-and-add instruction here, on architectures where that is supported. in lock.c Here is my first try at using it. That's interesting, but I have to wonder if there is any evidence that this *is* actually helpful to performance. One of my open questions listed in the original email was request for help on creating a test case that exercise the code path enough so that it any improvements can be measured. But apart from performance I think there are two other aspects to consider: 1) Code clarity: I think the lock.c code is easier to understand after the patch 2) Future possibilities: having the atomic_inc/dec generally available allows other performance critical parts of postgres take advantage of them in the future This was actually attempted a little while back; a spinlock was replaced with a few atomic increment and decrement calls for managing the refcount and other things on the freelist. It helped or hurt depending on contention but the net effect was negative. On reflection I think that was because that the assembly 'lock' instructions are really expensive relative to the others: so it's not safe to assume that say 2-3 gcc primitive increment calls are cheaper that a spinlock. The spinlock uses one 'lock' instruction when taken, and no lock instructions when released. Thus I think replacing one spinlock protected add/sub with atomic 'lock' add/sub not perform worse. But if you replace mutex-lock,add,add,unlock with atomic add, atomic add you already have more hw level synchronization and thus risk lower performance if they are on separate cache lines. This of course limits the use cases of the atomic operations. -Mikko -- 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] Assert for frontend programs?
Heikki Linnakangas hlinnakan...@vmware.com writes: On 14.12.2012 17:54, Tom Lane wrote: BTW, I think psql already has a psql_assert. psql_assert looks like this: #ifdef USE_ASSERT_CHECKING #include assert.h #define psql_assert(p) assert(p) #else ... On my Linux system, a failure looks like this: ~$ ./a.out a.out: a.c:5: main: Assertion `1==2' failed. Aborted That seems fine to me. Works for me. So just rename that to Assert() and move it into postgres-fe.h? 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] Assert for frontend programs?
On 12/14/2012 11:33 AM, Tom Lane wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: On 14.12.2012 17:54, Tom Lane wrote: BTW, I think psql already has a psql_assert. psql_assert looks like this: #ifdef USE_ASSERT_CHECKING #include assert.h #define psql_assert(p) assert(p) #else ... On my Linux system, a failure looks like this: ~$ ./a.out a.out: a.c:5: main: Assertion `1==2' failed. Aborted That seems fine to me. Works for me. So just rename that to Assert() and move it into postgres-fe.h? Seems so simple it's a wonder we didn't do it before. +1. 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] gistchoose vs. bloat
One question: does the randomization ever help when building a new index? In the original test case, you repeatedly delete and insert tuples, and I can see how the index can get bloated in that case. But I don't see how bloat would occur when building the index from scratch. BTW, I don't much like the option name randomization. It's not clear what's been randomized. I'd prefer something like distribute_on_equal_penalty, although that's really long. Better ideas? - Heikki -- 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] gistchoose vs. bloat
On Fri, 2012-12-14 at 18:36 +0200, Heikki Linnakangas wrote: One question: does the randomization ever help when building a new index? In the original test case, you repeatedly delete and insert tuples, and I can see how the index can get bloated in that case. But I don't see how bloat would occur when building the index from scratch. When building an index on a bunch of identical int4range values (in my test, [1,10) ), the resulting index was about 17% smaller. If the current algorithm always chooses to insert on the left-most page, then it seems like there would be a half-filled right page for every split that occurs. Is that reasoning correct? However, I'm having some second thoughts about the run time for index builds. Maybe we should have a few more tests to determine if this should really be the default or just an option? BTW, I don't much like the option name randomization. It's not clear what's been randomized. I'd prefer something like distribute_on_equal_penalty, although that's really long. Better ideas? I agree that randomization is vague, but I can't think of anything better. Regards, Jeff Davis -- 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 of systable_beginscan_ordered in event trigger patch
On Thu, Dec 13, 2012 at 6:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Dec 12, 2012 at 3:51 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert, does that ring a bell to you? I'm going to crawl the archives tomorrow if not… Yeah, I'm pretty sure you can't set event triggers of any kind on event triggers. You proposed to allow some stuff that would affect every command, but I yelled and screamed about that until we got rid of it, 'cuz it just seemed way too dangerous. In that case the docs should probably mention it more prominently; the example in CREATE EVENT TRIGGER is misleadingly described, for sure. I suspect there are still ways to shoot yourself in the foot with a broken event trigger, but it's not quite as trivial as I thought. I'm smart enough not to doubt you, but I'd sure appreciate a hint as to what you're worried about before we start building more on top of it. I don't want to sink a lot of work into follow-on commits and then discover during beta we have to rip it all out or disable it. It seems to me that if you can always drop an event trigger without interference from the event trigger system, you should at least be able to shut it off if you don't like what it's doing. I'm a little worried that there could be ways to crash the server or corrupt data, but I don't know what they are. ISTM that, at least for the firing point we have right now, it's not much different than executing the event trigger code before you execute the DDL command, and therefore it should be safe. But I'm very aware that I might be wrong, hence the extremely conservative first commit. -- 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_upgrade problem with invalid indexes
On Tue, Dec 11, 2012 at 03:12:37PM -0500, Bruce Momjian wrote: On Fri, Dec 7, 2012 at 04:49:19PM -0500, Bruce Momjian wrote: On Fri, Dec 7, 2012 at 10:38:39PM +0100, Andres Freund wrote: On 2012-12-07 16:30:36 -0500, Bruce Momjian wrote: On Fri, Dec 7, 2012 at 04:21:48PM -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2012-12-07 13:59:41 -0500, Tom Lane wrote: indisvalid should be sufficient. If you try to test more than that you're going to make the code more version-specific, without actually buying much. Doesn't the check need to be at least indisvalid indisready? Given that 9.2 represents !indislive as indisvalid !indisready? Um, good point. It's annoying that we had to do it like that ... So, does this affect pg_upgrade? Which PG versions? Only 9.2 :(. Before that there was no DROP INDEX CONCURRENTLY and in 9.3 there's an actual indislive field and indisready is always set to false there if indislive is false. But I see no problem using !indisvalid || !indisready as the condition in all (supported) versions. OK, updated patch attached. Patch applied back to 9.0. Now that it is applied, I need to publicize this. How do I do that? Josh mentioned my blog. What would cause these invalid indexes? Just CREATE INDEX CONCURRENTLY failures? What types of failures would users have if these invalid indexes had been upgraded by pg_upgrade? Can they test their indexes in any way? I assume they can't run queries on the old cluster to check. Blog entry posted explaining the bug and fix: http://momjian.us/main/blogs/pgblog/2012.html#December_14_2012 -- 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] MySQL search query is not executing in Postgres DB
On Tue, Dec 11, 2012 at 12:59 AM, Jeff Davis pg...@j-davis.com wrote: For every developer who says wow, that mysql query just worked without modification there is another one who says oh, I forgot to test with option XYZ... postgres is too complex to support, I'm going to drop it from the list of supported databases. Perhaps so. That's why my first choice is still to just fix this problem across the board. I think there is probably more than one way of doing that that is technically safe, and I currently believe that my patch is one of those. However, it seems that more people than not find the extra casts that PostgreSQL forces programmers to use to be a feature, not a bug. According to Tom, to allow people to call a non-overloaded function without casts will completely destroy the type system; Peter Eisentraut was aghast at the idea of allowing someone to pass a non-text first argument to lpad without an explicit cast. I recognize that not everyone's going to agree on these things but I find those attitudes shockingly arrogant. We have regular evidence that users are coming to PostgreSQL and then abandoning it because these kinds of things don't work, and we know that numerous other popular and well-respected systems allow these sorts of things to Just Work. It is one thing to insist on casts when there is an ambiguity about which of several overloaded functions a user intended to call - but when there's only one, it's just masterminding. In more than ten years of working with PostgreSQL, I've never encountered where the restriction at issue here prevented a bug. It's only annoyed me and broken my application code (when moving from PostgreSQL 8.2 to PostgreSQL 8.3, never mind any other database!). There is ample evidence that I'm not the only one, but I think we have a clear consensus to continue ignoring the problem, or at least the solutions. I don't think there's much point in carrying this patch over to the next CommitFest; I'll mark it as Rejected. -- 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 of systable_beginscan_ordered in event trigger patch
Robert Haas robertmh...@gmail.com writes: On Thu, Dec 13, 2012 at 6:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: I suspect there are still ways to shoot yourself in the foot with a broken event trigger, but it's not quite as trivial as I thought. I'm smart enough not to doubt you, but I'd sure appreciate a hint as to what you're worried about before we start building more on top of it. I don't want to sink a lot of work into follow-on commits and then discover during beta we have to rip it all out or disable it. It seems to me that if you can always drop an event trigger without interference from the event trigger system, you should at least be able to shut it off if you don't like what it's doing. I doubt that not firing on DROP EVENT TRIGGER, per se, will be sufficient to guarantee that you can execute such a drop. Even if that's true in the current state of the code, we're already hearing requests for event triggers on lower-level events, eg http://archives.postgresql.org/pgsql-hackers/2012-12/msg00314.php Sooner or later there will be one somewhere in the code path involved in doing a heap_delete on pg_event_trigger, or one of the subsidiary catalogs such as pg_depend, or maybe one that just manages to fire someplace during backend startup, or who knows what. Hence, I want a kill switch for all event triggers that will most certainly work, and the just-committed patch provides one. 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] logical decoding - GetOldestXmin
On Fri, Dec 14, 2012 at 6:46 AM, Andres Freund and...@2ndquadrant.com wrote: Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. I'm not sure that would be so bad, but I guess I question the need to do it this way at all. Most of the time, if you need to advertise your global xmin, you use GetSnapshotData(), not GetOldestXmin(), and I guess I'm not seeing why that wouldn't also work here. Am I dumb? -- 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 of systable_beginscan_ordered in event trigger patch
On Fri, Dec 14, 2012 at 2:00 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Dec 13, 2012 at 6:25 PM, Tom Lane t...@sss.pgh.pa.us wrote: I suspect there are still ways to shoot yourself in the foot with a broken event trigger, but it's not quite as trivial as I thought. I'm smart enough not to doubt you, but I'd sure appreciate a hint as to what you're worried about before we start building more on top of it. I don't want to sink a lot of work into follow-on commits and then discover during beta we have to rip it all out or disable it. It seems to me that if you can always drop an event trigger without interference from the event trigger system, you should at least be able to shut it off if you don't like what it's doing. I doubt that not firing on DROP EVENT TRIGGER, per se, will be sufficient to guarantee that you can execute such a drop. Even if that's true in the current state of the code, we're already hearing requests for event triggers on lower-level events, eg http://archives.postgresql.org/pgsql-hackers/2012-12/msg00314.php Yep, true. Sooner or later there will be one somewhere in the code path involved in doing a heap_delete on pg_event_trigger, or one of the subsidiary catalogs such as pg_depend, or maybe one that just manages to fire someplace during backend startup, or who knows what. Yeah. :-( Hence, I want a kill switch for all event triggers that will most certainly work, and the just-committed patch provides one. I'm definitely not disputing the need for that patch. -- 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] MySQL search query is not executing in Postgres DB
Robert Haas robertmh...@gmail.com writes: ... In more than ten years of working with PostgreSQL, I've never encountered where the restriction at issue here prevented a bug. It's only annoyed me and broken my application code (when moving from PostgreSQL 8.2 to PostgreSQL 8.3, never mind any other database!). There are quite a few examples in our archives of application bugs that would have been prevented, or later were prevented, by the 8.3 changes that reduced the system's willingness to apply implicit casts to text. I recall for instance cases where people got wrong/unexpected answers because the system was sorting what-they-thought-were-timestamp values textually. So I find such sweeping claims to be demonstrably false, and I'm suspicious of behavioral changes that are proposed with such arguments backing them. There is ample evidence that I'm not the only one, but I think we have a clear consensus to continue ignoring the problem, or at least the solutions. Oh, I don't think we're ignoring the problem; people beat us up about it too often for that. But we need to pay attention to error detection not only ease-of-use, so it's hard to be sure what's a net improvement. 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] Enabling Checksums
On Wed, 2012-12-12 at 17:52 -0500, Greg Smith wrote: I can take this on, as part of the QA around checksums working as expected. The result would be a Python program; I don't have quite enough time to write this in C or re-learn Perl to do it right now. But this won't be a lot of code. If it's tossed one day as simply a prototype for something more permanent, I think it's still worth doing now. The UI I'm thinking of for what I'm going to call pg_corrupt is a CLI that asks for: -A relation name -Corruption type (an entry from this list) -How many blocks to touch I'll just loop based on the count, randomly selecting a block each time and messing with it in that way. The randomness seed should be printed as part of the output, so that it's possible re-create the damage exactly later. If the server doesn't handle it correctly, we'll want to be able to replicate the condition it choked on exactly later, just based on the tool's log output. Any other requests? After some thought, I don't see much value in introducing multiple instances of corruption at a time. I would think that the smallest unit of corruption would be the hardest to detect, so by introducing many of them in one pass makes it easier to detect. For example, if we introduce an all-ones page, and also transpose two pages, the all-ones error might be detected even if the transpose error is not being detected properly. And we'd not know that the transpose error was not being detected, because the error appears as soon as it sees the all-ones page. Does it make sense to have a separate executable (pg_corrupt) just for corrupting the data as a test? Or should it be part of a corruption-testing harness (pg_corruptiontester?), that introduces the corruption and then verifies that it's properly detected? Regards, Jeff Davis -- 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] Enabling Checksums
On 12/14/12 3:00 PM, Jeff Davis wrote: After some thought, I don't see much value in introducing multiple instances of corruption at a time. I would think that the smallest unit of corruption would be the hardest to detect, so by introducing many of them in one pass makes it easier to detect. That seems reasonable. It would eliminate a lot of issues with reproducing a fault too. I can just print the impacted block number presuming it will show up in a log, and make it possible to override picking one at random with a command line input. Does it make sense to have a separate executable (pg_corrupt) just for corrupting the data as a test? Or should it be part of a corruption-testing harness (pg_corruptiontester?), that introduces the corruption and then verifies that it's properly detected? Let me see what falls out of the coding, I don't think this part needs to get nailed down yet. Building a corruption testing harness is going to involve a lot of creating new clusters and test data to torture. It's a different style of problem than injecting faults in the first place. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.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] MySQL search query is not executing in Postgres DB
On Fri, Dec 14, 2012 at 2:11 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: ... In more than ten years of working with PostgreSQL, I've never encountered where the restriction at issue here prevented a bug. It's only annoyed me and broken my application code (when moving from PostgreSQL 8.2 to PostgreSQL 8.3, never mind any other database!). There are quite a few examples in our archives of application bugs that would have been prevented, or later were prevented, by the 8.3 changes that reduced the system's willingness to apply implicit casts to text. I recall for instance cases where people got wrong/unexpected answers because the system was sorting what-they-thought-were-timestamp values textually. So I find such sweeping claims to be demonstrably false, and I'm suspicious of behavioral changes that are proposed with such arguments backing them. I think you're mixing apples and oranges. The whole point of the patch on the table - which, if you recall, was designed originally by you and merely implemented by me - was to change the behavior only in the cases where there's only one function with the appropriate name and argument count. The ambiguous cases that 8.3+ helpfully prevent are those where overloading is in use and the choice of which function to call is somewhat arbitrary and perhaps incorrectly-foreseen by the user. Those changes also have the side-effect of preventing a straightforward function call from working without casts even in cases where no overloading is in use - and making that case work is completely different from making the ambiguous case arbitrarily pick one of the available answers. Oh, I don't think we're ignoring the problem; people beat us up about it too often for that. But we need to pay attention to error detection not only ease-of-use, so it's hard to be sure what's a net improvement. Well, that's not how the dynamic of this thread reads to me. There seems to be massive opposition - including from you - to allowing unambiguous function calls to resolve without casts, at least as a categorical matter, and maybe even in the specific cases that users most frequently care about. I simply disagree with the contention that there's a value in making people cast to text when the target function is not overloaded. Maybe there's some world where it's uncommon to want to pass the text representation of a non-text value to a non-overloaded function that accepts text, and therefore forcing a cast upon the user to warn them here be dragons is warranted, but I don't live in it. When the function IS overloaded, well, that's a completely different situation. I've written enough C++ over the years to understand what happens when you try too hard to be clever with tiebreak rules. But that's not got much to do with the question of whether the only candidate to put in an appearance can be declared the winner. -- 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] Assert for frontend programs?
On 12/14/2012 11:33 AM, Tom Lane wrote:
Heikki Linnakangas hlinnakan...@vmware.com writes:
On 14.12.2012 17:54, Tom Lane wrote:
BTW, I think psql already has a psql_assert.
psql_assert looks like this:
#ifdef USE_ASSERT_CHECKING
#include assert.h
#define psql_assert(p) assert(p)
#else
...
On my Linux system, a failure looks like this:
~$ ./a.out
a.out: a.c:5: main: Assertion `1==2' failed.
Aborted
That seems fine to me.
Works for me. So just rename that to Assert() and move it into
postgres-fe.h?
Here's a patch for that. I changed some of the psql assertions so they
all have explicit boolean expressions - I think that's better style for
use of assert.
I noticed, BTW, that there are one or two places in backend code that
seem to call plain assert unconditionally, notably src/port/open.c,
src/backend/utils/adt/inet_net_pton.c and some contrib modules. That
seems undesirable. Should we need to look at turning these into Assert
calls?
cheers
andrew
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 8ccd00d..e605785 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -99,7 +99,7 @@ HandleSlashCmds(PsqlScanState scan_state,
char *cmd;
char *arg;
- psql_assert(scan_state);
+ Assert(scan_state != NULL);
/* Parse off the command name */
cmd = psql_scan_slash_command(scan_state);
@@ -1819,7 +1819,7 @@ editFile(const char *fname, int lineno)
char *sys;
int result;
- psql_assert(fname);
+ Assert(fname != NULL);
/* Find an editor to use */
editorName = getenv(PSQL_EDITOR);
@@ -2177,7 +2177,7 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
{
size_t vallen = 0;
- psql_assert(param);
+ Assert(param != NULL);
if (value)
vallen = strlen(value);
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 179c162..c2a2ab6 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1160,7 +1160,7 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
}
OK = AcceptResult(results);
- psql_assert(!OK);
+ Assert(!OK);
PQclear(results);
break;
}
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index f54baab..7f34290 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -12,13 +12,6 @@
#include setjmp.h
#include libpq-fe.h
-#ifdef USE_ASSERT_CHECKING
-#include assert.h
-#define psql_assert(p) assert(p)
-#else
-#define psql_assert(p)
-#endif
-
#define atooid(x) ((Oid) strtoul((x), NULL, 10))
/*
diff --git a/src/bin/psql/psqlscan.l b/src/bin/psql/psqlscan.l
index 6c14298..f8822cc 100644
--- a/src/bin/psql/psqlscan.l
+++ b/src/bin/psql/psqlscan.l
@@ -1184,8 +1184,8 @@ psql_scan_setup(PsqlScanState state,
const char *line, int line_len)
{
/* Mustn't be scanning already */
- psql_assert(state-scanbufhandle == NULL);
- psql_assert(state-buffer_stack == NULL);
+ Assert(state-scanbufhandle == NULL);
+ Assert(state-buffer_stack == NULL);
/* Do we need to hack the character set encoding? */
state-encoding = pset.encoding;
@@ -1245,7 +1245,7 @@ psql_scan(PsqlScanState state,
int lexresult;
/* Must be scanning already */
- psql_assert(state-scanbufhandle);
+ Assert(state-scanbufhandle != NULL);
/* Set up static variables that will be used by yylex */
cur_state = state;
@@ -1424,7 +1424,7 @@ psql_scan_slash_command(PsqlScanState state)
PQExpBufferData mybuf;
/* Must be scanning already */
- psql_assert(state-scanbufhandle);
+ Assert(state-scanbufhandle != NULL);
/* Build a local buffer that we'll return the data of */
initPQExpBuffer(mybuf);
@@ -1478,7 +1478,7 @@ psql_scan_slash_option(PsqlScanState state,
char local_quote;
/* Must be scanning already */
- psql_assert(state-scanbufhandle);
+ Assert(state-scanbufhandle != NULL);
if (quote == NULL)
quote = local_quote;
@@ -1512,7 +1512,7 @@ psql_scan_slash_option(PsqlScanState state,
* or LEXRES_EOL (the latter indicating end of string). If we were inside
* a quoted string, as indicated by YY_START, EOL is an error.
*/
- psql_assert(lexresult == LEXRES_EOL || lexresult == LEXRES_OK);
+ Assert(lexresult == LEXRES_EOL || lexresult == LEXRES_OK);
switch (YY_START)
{
@@ -1608,7 +1608,7 @@ void
psql_scan_slash_command_end(PsqlScanState state)
{
/* Must be scanning already */
- psql_assert(state-scanbufhandle);
+ Assert(state-scanbufhandle != NULL);
/* Set up static variables that will be used by yylex */
cur_state = state;
diff --git a/src/bin/psql/stringutils.c b/src/bin/psql/stringutils.c
index b557c5a..f0bed2b 100644
--- a/src/bin/psql/stringutils.c
+++ b/src/bin/psql/stringutils.c
@@ -245,8 +245,8 @@ strip_quotes(char *source, char quote, char escape, int encoding)
char *src;
char *dst;
- psql_assert(source);
- psql_assert(quote);
+ Assert(source != NULL);
+ Assert(quote != '\0');
src = dst = source;
@@ -299,8 +299,8 @@ quote_if_needed(const char *source, const
Re: [HACKERS] Use gcc built-in atomic inc/dec in lock.c
On Wed, Dec 12, 2012 at 5:19 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: On 12 December 2012 22:11, Mikko Tiihonen mikko.tiiho...@nitorcreations.com wrote: noticed a XXX: It might be worth considering using an atomic fetch-and-add instruction here, on architectures where that is supported. in lock.c Here is my first try at using it. That's interesting, but I have to wonder if there is any evidence that this *is* actually helpful to performance. Ditto. I've had at least one bad experience with an attempted change to this sort of thing backfiring. And it's possible that's because my implementation sucked, but the only concrete evidence of that which I was able to discern was bad performance. So I've learned to be skeptical of these kinds of things unless there are clear benchmark results. -- 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] Re: bulk_multi_insert infinite loops with large rows and small fill factors
On Wed, Dec 12, 2012 at 8:29 AM, David Gould da...@sonic.net wrote: We lose noticable performance when we raise fill-factor above 10. Even 20 is slower. Whoa. During busy times these hosts sometimes fall into a stable state with very high cpu use mostly in s_lock() and LWLockAcquire() and I think PinBuffer plus very high system cpu in the scheduler (I don't have the perf trace in front of me so take this with a grain of salt). In this mode they fall from the normal 7000 queries per second to below 3000. I have seen signs of something similar to this when running pgbench -S tests at high concurrency. I've never been able to track down where the problem is happening. My belief is that once a spinlock starts to be contended, there's some kind of death spiral that can't be arrested until the workload eases up. But I haven't had much luck identifying exactly which spinlock is the problem or if it even is just one... -- 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] PageIsAllVisible()'s trustworthiness in Hot Standby
On Wed, Dec 12, 2012 at 12:31 PM, Pavan Deolasee pavan.deola...@gmail.com wrote: On Wed, Dec 12, 2012 at 1:35 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 4, 2012 at 12:10 PM, Pavan Deolasee pavan.deola...@gmail.com wrote: Hmm. Yeah, I do not have guts to prove that either. I'll probably write up a comment for your consideration to explain why we don't trust PD_ALL_VISIBLE in Hot standby for seq scans, but still trust VM for index-only scans. Sure. Here is a small patch that adds comments to heap scan code explaining why we don't trust the all-visible flag in the page, still continue to support index-only scans on hot standby. Committed, with a few modifications to the last part. -- 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 of systable_beginscan_ordered in event trigger patch
Robert Haas robertmh...@gmail.com writes: Robert, does that ring a bell to you? I'm going to crawl the archives tomorrow if not… Yeah, I'm pretty sure you can't set event triggers of any kind on event triggers. You proposed to allow some stuff that would affect every command, but I yelled and screamed about that until we got rid of it, 'cuz it just seemed way too dangerous. I meant about the way the documentation is phrased to introduce the example, which is somewhat wrong because not all commands are concerned, quite a bunch of them will not be disabled by such a command trigger. Tom Lane t...@sss.pgh.pa.us writes: Sooner or later there will be one somewhere in the code path involved in doing a heap_delete on pg_event_trigger, or one of the subsidiary catalogs such as pg_depend, or maybe one that just manages to fire someplace during backend startup, or who knows what. You're right that we need to be careful here, in ways that I didn't foresee. The first thing I can think of is to disable such low level events on system catalogs, of course. Hence, I want a kill switch for all event triggers that will most certainly work, and the just-committed patch provides one. We absolutely need that, and running Event Triggers in standalone mode seems counter productive to me anyway. That said maybe we need to be able to have a per-session leave me alone mode of operation. What do you think of ALTER EVENT TRIGGER DISABLE ALL; -- just tried, no conflict I don't think we need the ENABLE ALL variant, and it would not be symetric anyway as you would want to be able to only enable those event triggers that were already enabled before, and it seems to me that cancelling a statement is best done with ROLLBACK; or ROLLBACK TO SAVEPOINT …;. 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] Assert for frontend programs?
Andrew Dunstan and...@dunslane.net writes: I noticed, BTW, that there are one or two places in backend code that seem to call plain assert unconditionally, notably src/port/open.c, src/backend/utils/adt/inet_net_pton.c and some contrib modules. That seems undesirable. Should we need to look at turning these into Assert calls? Yeah, possibly. The inet_net_pton.c case is surely because it was that way in the BIND code we borrowed; perhaps the others are the same story. I don't object to changing them, since we don't seem to be actively adopting any new upstream versions; but again I can't get too excited. - psql_assert(!*text); + Assert(*text != '\0'); I think you got that one backwards. #include c.h +#ifdef USE_ASSERT_CHECKING +#include assert.h +#define Assert(p) assert(p) +#else +#define Assert(p) +#endif Perhaps a comment would be in order here? Specifically something about providing Assert() so that it can be used in both backend and frontend code? 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] Use of systable_beginscan_ordered in event trigger patch
On Fri, Dec 14, 2012 at 3:46 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: Robert, does that ring a bell to you? I'm going to crawl the archives tomorrow if not… Yeah, I'm pretty sure you can't set event triggers of any kind on event triggers. You proposed to allow some stuff that would affect every command, but I yelled and screamed about that until we got rid of it, 'cuz it just seemed way too dangerous. I meant about the way the documentation is phrased to introduce the example, which is somewhat wrong because not all commands are concerned, quite a bunch of them will not be disabled by such a command trigger. Tom Lane t...@sss.pgh.pa.us writes: Sooner or later there will be one somewhere in the code path involved in doing a heap_delete on pg_event_trigger, or one of the subsidiary catalogs such as pg_depend, or maybe one that just manages to fire someplace during backend startup, or who knows what. You're right that we need to be careful here, in ways that I didn't foresee. The first thing I can think of is to disable such low level events on system catalogs, of course. Hence, I want a kill switch for all event triggers that will most certainly work, and the just-committed patch provides one. We absolutely need that, and running Event Triggers in standalone mode seems counter productive to me anyway. That said maybe we need to be able to have a per-session leave me alone mode of operation. What do you think of ALTER EVENT TRIGGER DISABLE ALL; -- just tried, no conflict I don't think we need the ENABLE ALL variant, and it would not be symetric anyway as you would want to be able to only enable those event triggers that were already enabled before, and it seems to me that cancelling a statement is best done with ROLLBACK; or ROLLBACK TO SAVEPOINT …;. ISTM that a PGC_SUSER GUC, as I proposed previously, would serve this need adequately, without the cost of more cruft in gram.y. Am I wrong? -- 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 gcc built-in atomic inc/dec in lock.c
- Цитат от Mikko Tiihonen (mikko.tiiho...@nitorcreations.com), на 14.12.2012 в 17:33 - On 12/13/2012 12:19 AM, Peter Geoghegan wrote: On 12 December 2012 22:11, Mikko Tiihonen mikko.tiiho...@nitorcreations.com wrote: noticed a XXX: It might be worth considering using an atomic fetch-and-add instruction here, on architectures where that is supported. in lock.c Here is my first try at using it. That's interesting, but I have to wonder if there is any evidence that this *is* actually helpful to performance. One of my open questions listed in the original email was request for help on creating a test case that exercise the code path enough so that it any improvements can be measured. Running pgbench on 16+ cores/threads could stress locking primitives. From my experience even benchmarks run on 8 core systems should tell the difference. -- Luben Karavelov
[HACKERS] Parser Cruft in gram.y
Robert Haas robertmh...@gmail.com writes: ISTM that a PGC_SUSER GUC, as I proposed previously, would serve this need adequately, without the cost of more cruft in gram.y. I can't help but think about the experiments you did some time ago about splitting the grammar into differents sub-grammars (for lack of a better term). If I remember correctly, your result showed no performance gain from separating away Queries and DML on the one side from the rest, DDL and DCL and such like. IIRC, you didn't have a regression either. Now, what about splitting those grammars in order to freely add any new production rules with or without new keywords for administrative commands, without a blink of though about the main parser grammar. I guess that the traffic cop would need to have a decent fast path to very quickly get to use the right parser, and I suppose you did already implement that in your previous experiment. If that's sensible as a way forward, that can also be the basis for allowing extensions to implement their own command set too. The trick would be how to implement extensible grammar routing. That would come way after we have the initial split, though. 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] Adjusting elog behavior in bootstrap/standalone mode
In 3688.1355509...@sss.pgh.pa.us I complained PS: one odd thing here is that the ereport(LOG) in InstallXLogFileSegment isn't doing anything; otherwise we'd have gotten a much more helpful error report about could not link file. I don't think we run the bootstrap mode with log_min_messages set high enough to disable LOG messages, so why isn't it printing? I looked into this and found that the reason the useful message didn't appear is that elog.c has different rules for what to print in bootstrap (and standalone-backend) mode: /* Determine whether message is enabled for server log output */ if (IsPostmasterEnvironment) output_to_server = is_log_level_output(elevel, log_min_messages); else /* In bootstrap/standalone case, do not sort LOG out-of-order */ output_to_server = (elevel = log_min_messages); In view of the confusion this caused just now, I wondered if we shouldn't get rid of the special case and always follow the is_log_level_output rule. I tried modifying the code that way, and soon found that it made initdb rather noisy: creating configuration files ... ok creating template1 database in /home/postgres/data/base/1 ... LOG: bogus data in postmaster.pid LOG: database system was shut down at 2012-12-14 15:55:35 EST LOG: shutting down LOG: database system is shut down ok initializing pg_authid ... LOG: bogus data in postmaster.pid LOG: database system was shut down at 2012-12-14 15:55:54 EST LOG: shutting down LOG: database system is shut down ok initializing dependencies ... LOG: bogus data in postmaster.pid LOG: database system was shut down at 2012-12-14 15:55:55 EST LOG: shutting down LOG: database system is shut down ok Unsurprisingly, the same four messages appear in a manual standalone-backend run: $ postgres --single LOG: bogus data in postmaster.pid LOG: database system was shut down at 2012-12-14 15:56:27 EST PostgreSQL stand-alone backend 9.3devel backend LOG: shutting down LOG: database system is shut down $ Now, the bogus data message is actually indicative of a bug. I've not tracked it down yet, but it evidently must mean that AddToDataDirLockFile() is being called with out-of-sequence target_line numbers in standalone mode. This is pretty bad because it means that a standalone backend isn't setting up the lock file the way it ought to. We hadn't realized that because elog.c's behavior was hiding the message that a backend code author would normally expect to appear. So that reinforces my feeling that this special case in elog.c is a bad idea that needs to die. However, to do that without trashing initdb's normal display, we have to do something to quiet the other three messages. One possibility is to tweak the elog call sites for these specific messages so that they are, say, NOTICE not LOG level when not IsPostmasterEnvironment. That seems like a bit of a hack, but I don't see another answer that doesn't involve behind-the-scenes decisions in elog.c ... which is exactly what I want to get rid of. Thoughts? 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] Assert for frontend programs?
On 12/14/12 11:33 AM, Tom Lane wrote: Works for me. So just rename that to Assert() and move it into postgres-fe.h? Or just call assert() and don't invent our own layer? -- 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] Assert for frontend programs?
On 12/14/2012 04:23 PM, Peter Eisentraut wrote: On 12/14/12 11:33 AM, Tom Lane wrote: Works for me. So just rename that to Assert() and move it into postgres-fe.h? Or just call assert() and don't invent our own layer? Well, part of the point is that it lets you use Assert() in code that might be run in both the frontend and the backend. 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] Parser Cruft in gram.y
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Now, what about splitting those grammars in order to freely add any new production rules with or without new keywords for administrative commands, without a blink of though about the main parser grammar. Let me explain to you why there will never be a situation where we can consider new keywords to be zero-cost. $ size src/backend/parser/gram.o textdata bss dec hex filename 952864 104 0 952968 e8a88 src/backend/parser/gram.o $ size src/backend/postgres textdata bss dec hex filename 6815102 123416 239356 7177874 6d8692 src/backend/postgres That is, the grammar tables already amount to 14% of the total bulk of the server executable. (The above numbers exclude debug symbols BTW.) That bloat is not free; for one thing, it's competing for L1 cache with all the actual code in the backend. And the main cause of it is that we have lots-and-lots of keywords, because the parser tables are basically number-of-tokens wide by number-of-states high. (In HEAD there are 433 tokens known to the grammar, all but 30 of which are keywords, and 4367 states.) Splitting the grammar into multiple grammars is unlikely to do much to improve this --- in fact, it could easily make matters worse due to duplication. Rather, we just have to be careful about adding new keywords. In this connection, I quite like the fact that recent syntax extensions such as EXPLAIN (...options...) have managed to avoid making the option names into grammar keywords at all. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Makefiles don't seem to remember to rebuild everything anymore
In a fully-built source tree: $ cd pgsql/src/backend/parser $ make make: Nothing to be done for `all'. ... okay so far ... $ rm gram.o rm: remove regular file `gram.o'? y $ make make: Nothing to be done for `all'. WTF? If I also remove objfiles.txt then make wakes up and remembers it's supposed to do something. I can reproduce this with both make 3.81 and 3.82, so I think it's a bug in our makefiles not make. I don't immediately see where the problem is though. 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] Assert for frontend programs?
Peter Eisentraut pete...@gmx.net writes: On 12/14/12 11:33 AM, Tom Lane wrote: Works for me. So just rename that to Assert() and move it into postgres-fe.h? Or just call assert() and don't invent our own layer? Having the layer is a good thing, eg so that USE_ASSERT_CHECKING can control it, or so that somebody can inject a different behavior if they want. 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] Switching timeline over streaming replication
Heikki, Tested this on yesterday's snapshot. Worked great. Test: 4 Ubuntu 10.04 LTS Cloud Servers (GoGrid) Configuration: Compiled 9.3(12-12-12) with: pg_stat_statements, citext, ISN, btree_gist, pl/perl Setup Test: Master-Master Replicated to: master-replica using pg_basebackup -x. No archiving. Master-Replica replicated to Replica-Replica1 and Replica-Replica2 using pg_basebackup -x All came up on first try, with no issues. Ran customized pgbench (with waits); lag time to cascading replicas was 1 second. Failover Test: 1. started customized pgbench on master-master. 2. shut down master-master (-fast) 3. promoted master-replica to new master 4. restarted custom pgbench, at master-replica Result: Replication to replica-replica1,2 working fine, no interruptions in existing connections to replica-replicas. Now I wanna test a chain of cascading replicas ... how far can we chain these? -- Josh Berkus PostgreSQL Experts Inc. http://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] Parser Cruft in gram.y
Tom Lane t...@sss.pgh.pa.us writes: Let me explain to you why there will never be a situation where we can consider new keywords to be zero-cost. Thanks for taking the time to do this. Splitting the grammar into multiple grammars is unlikely to do much to improve this --- in fact, it could easily make matters worse due to duplication. Rather, we just have to be careful about adding new keywords. In this connection, I quite like the fact that recent syntax extensions such as EXPLAIN (...options...) have managed to avoid making the option names into grammar keywords at all. I'm still pretty unhappy about this state of affairs. I would like to have a fast path and a you can go crazy here path. Apparently the solutions to reduce the footprint involve hand made recursive decent parsers which are harder to maintain. I've read a little about the packrat parsing techniques, but far from enough to understand how much they could help us solve the binary bloat problem we have here while not making it harder to maintain our grammar. Maybe some other techniques are available… Ideas? Or should I just bite the bullet? -- 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] Re: bulk_multi_insert infinite loops with large rows and small fill factors
On Fri, 14 Dec 2012 15:39:44 -0500 Robert Haas robertmh...@gmail.com wrote: On Wed, Dec 12, 2012 at 8:29 AM, David Gould da...@sonic.net wrote: We lose noticable performance when we raise fill-factor above 10. Even 20 is slower. Whoa. Any interest in a fill-factor patch to place exactly one row per page? That would be the least contended. There are applications where it might help. During busy times these hosts sometimes fall into a stable state with very high cpu use mostly in s_lock() and LWLockAcquire() and I think PinBuffer plus very high system cpu in the scheduler (I don't have the perf trace in front of me so take this with a grain of salt). In this mode they fall from the normal 7000 queries per second to below 3000. I have seen signs of something similar to this when running pgbench -S tests at high concurrency. I've never been able to track down where I think I may have seen that with pgbench -S too. I did not have time to investigate more, but out of a sequence of three minute runs I got most runs at 300k+ qps and but a couple were around 200k qps. the problem is happening. My belief is that once a spinlock starts to be contended, there's some kind of death spiral that can't be arrested until the workload eases up. But I haven't had much luck identifying exactly which spinlock is the problem or if it even is just one... I agree about the death spiral. I think what happens is all the backends get synchcronized by waiting and they are more likely to contend again. -dg -- David Gould 510 282 0869 da...@sonic.net If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parser Cruft in gram.y
Tom Lane wrote: the parser tables are basically number-of-tokens wide by number-of-states high. (In HEAD there are 433 tokens known to the grammar, all but 30 of which are keywords, and 4367 states.) Splitting the grammar into multiple grammars is unlikely to do much to improve this --- in fact, it could easily make matters worse due to duplication. I agree that without knowing what percentage would be used by each parser in a split, it could go either way. Consider a hypothetical situation where one parser has 80% and the other 50% of the current combined parser -- 30% overlap on both tokens and grammer constructs. (Picking numbers out of the air, for purposes of demonstration.) Combined = 433 * 4,367 = 1,890,911 80% = 346 * 3,493 = 1,208,578 50% = 216 * 2,183 = 471,528 Total for split = 1,680,106 Of course if they were both at 80% it would be a higher total than combined, but unless you have a handle on the percentages, it doesn't seem like a foregone conclusion. Do you have any feel for what the split would be? -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] logical decoding - GetOldestXmin
On 2012-12-14 14:01:30 -0500, Robert Haas wrote: On Fri, Dec 14, 2012 at 6:46 AM, Andres Freund and...@2ndquadrant.com wrote: Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. I'm not sure that would be so bad, but I guess I question the need to do it this way at all. Most of the time, if you need to advertise your global xmin, you use GetSnapshotData(), not GetOldestXmin(), and I guess I'm not seeing why that wouldn't also work here. Am I dumb? I wondered upthread whether that would be better: On 2012-12-13 21:03:44 +0100, Andres Freund wrote: Another alternative to this would be to get a snapshot with GetSnapshotData(), copy the xmin to the logical slot, then call ProcArrayEndTransaction(). But that doesn't really seem to be nicer to me. Not sure why I considered it ugly anymore, but it actually has a noticeable disadvantage. GetOldestXmin is nicer is than GetSnapshotData as the latter set a fairly new xid as xmin whereas GetOldestXmin returns the actual current xmin horizon. Thats preferrable because it allows us to start up more quickly. snapbuild.c can only start building a snapshot once it has seen a xl_running_xact with oldestRunningXid = own_xmin. Otherwise we cannot be sure that no relevant catalog tuples have been removed. This also made me notice that my changes to GetSnapshotData were quite pessimal... I do set the xmin of the new snapshot to the logical xmin instead of doing it only to globalxmin/RecentGlobalXmin. Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] Switching timeline over streaming replication
On Sat, Dec 8, 2012 at 12:51 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 06.12.2012 15:39, Amit Kapila wrote: On Thursday, December 06, 2012 12:53 AM Heikki Linnakangas wrote: On 05.12.2012 14:32, Amit Kapila wrote: On Tuesday, December 04, 2012 10:01 PM Heikki Linnakangas wrote: After some diversions to fix bugs and refactor existing code, I've committed a couple of small parts of this patch, which just add some sanity checks to notice incorrect PITR scenarios. Here's a new version of the main patch based on current HEAD. After testing with the new patch, the following problems are observed. Defect - 1: 1. start primary A 2. start standby B following A 3. start cascade standby C following B. 4. start another standby D following C. 5. Promote standby B. 6. After successful time line switch in cascade standby C D, stop D. 7. Restart D, Startup is successful and connecting to standby C. 8. Stop C. 9. Restart C, startup is failing. Ok, the error I get in that scenario is: C 2012-12-05 19:55:43.840 EET 9283 FATAL: requested timeline 2 does not contain minimum recovery point 0/3023F08 on timeline 1 C 2012-12-05 19:55:43.841 EET 9282 LOG: startup process (PID 9283) exited with exit code 1 C 2012-12-05 19:55:43.841 EET 9282 LOG: aborting startup due to startup process failure That mismatch causes the error. I'd like to fix this by always treating the checkpoint record to be part of the new timeline. That feels more correct. The most straightforward way to implement that would be to peek at the xlog record before updating replayEndRecPtr and replayEndTLI. If it's a checkpoint record that changes TLI, set replayEndTLI to the new timeline before calling the redo-function. But it's a bit of a modularity violation to peek into the record like that. Or we could just revert the sanity check at beginning of recovery that throws the requested timeline 2 does not contain minimum recovery point 0/3023F08 on timeline 1 error. The error I added to redo of checkpoint record that says unexpected timeline ID %u in checkpoint record, before reaching minimum recovery point %X/%X on timeline %u checks basically the same thing, but at a later stage. However, the way minRecoveryPointTLI is updated still seems wrong to me, so I'd like to fix that. I'm thinking of something like the attached (with some more comments before committing). Thoughts? This has fixed the problem reported. However, I am not able to think will there be any problem if we remove check requested timeline 2 does not contain minimum recovery point 0/3023F08 on timeline 1 at beginning of recovery and just update replayEndTLI with ThisTimeLineID? Well, it seems wrong for the control file to contain a situation like this: pg_control version number:932 Catalog version number: 201211281 Database system identifier: 5819228770976387006 Database cluster state: shut down in recovery pg_control last modified: pe 7. joulukuuta 2012 17.39.57 Latest checkpoint location: 0/3023EA8 Prior checkpoint location:0/260 Latest checkpoint's REDO location:0/3023EA8 Latest checkpoint's REDO WAL file:00020003 Latest checkpoint's TimeLineID: 2 ... Time of latest checkpoint:pe 7. joulukuuta 2012 17.39.49 Min recovery ending location: 0/3023F08 Min recovery ending loc's timeline: 1 Note the latest checkpoint location and its TimelineID, and compare them with the min recovery ending location. The min recovery ending location is ahead of latest checkpoint's location; the min recovery ending location actually points to the end of the checkpoint record. But how come the min recovery ending location's timeline is 1, while the checkpoint record's timeline is 2. Now maybe that would happen to work if remove the sanity check, but it still seems horribly confusing. I'm afraid that discrepancy will come back to haunt us later if we leave it like that. So I'd like to fix that. Mulling over this for some more, I propose the attached patch. With the patch, we peek into the checkpoint record, and actually perform the timeline switch (by changing ThisTimeLineID) before replaying it. That way the checkpoint record is really considered to be on the new timeline for all purposes. At the moment, the only difference that makes in practice is that we set replayEndTLI, and thus minRecoveryPointTLI, to the new TLI, but it feels logically more correct to do it that way. This patch has already been included in HEAD. Right? I found another requested timeline does not contain minimum recovery point error scenario in HEAD: 1. Set up the master 'M', one standby 'S1', and one cascade standby 'S2'. 2. Shutdown the master 'M' and promote the standby 'S1', and wait for 'S2' to reconnect to 'S1'. 3. Set
Re: [HACKERS] Doc patch to note which system catalogs have oids
On Fri, 2012-12-14 at 00:04 -0800, Jeff Davis wrote: On Tue, 2012-10-02 at 10:46 -0500, Karl O. Pinc wrote: I am now submitting patches to the commitfest for review. (I'm not sure how I missed this.) I prefer this version of the patch. I also attached an alternative version that may address Tom's concern by noting that the OIDs are hidden in the description. Committed this version. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] is allow_nonpic_in_shlib still useful?
In the plperl and plpython makefiles we look for a shared library of libperl or libpython, and if it's not found, we check for allow_nonpic_in_shlib, and if that's yes, then we proceed anyway. Apparently, and IIRC, this was set up in a time when those shared libraries were not available through standard builds, but I think that hasn't been the case for quite a while. The only platforms where we set allow_nonpick_in_shlib is linux and freebsd/i386 (presumably an obsolescent combination). Are there any Linux builds that don't supply the required shared libraries? I suspend this hack isn't useful anymore and ought to be removed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
