Re: [HACKERS] Question regarding SSL code in backend and frontend
Those code fragment judges the return value from SSL_read(). secure_read() does retrying when SSL_ERROR_WANT_READ *and* SSL_ERROR_WANT_WRITE returned. However, pqsecure_read() does not retry when SSL_ERROR_WANT_READ. It seems they are not consistent. Comments? There's no particular reason why they should be consistent, I think. The assumptions for nonblocking operation are different. Ok. Thanks. BTW, usage of SSL_CTX_new() is different among frontend and backend as well. fe-secure.c:SSL_context = SSL_CTX_new(TLSv1_method()); be-secure.c:SSL_context = SSL_CTX_new(SSLv23_method()); In my understanding by using SSLV23_method, it is compatible with SSLv2, SSLv3, and TLSv1 protocol. So it seems there's no particular reason to use TLSv1_method(). Am I missing something? -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp -- 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] [BUGS] BUG #6572: The example of SPI_execute is bogus
On Wed, Apr 4, 2012 at 8:00 AM, Tom Lane t...@sss.pgh.pa.us wrote: umi.tan...@gmail.com writes: http://www.postgresql.org/docs/9.1/static/spi-spi-execute.html === SPI_execute(INSERT INTO foo SELECT * FROM bar, false, 5); will allow at most 5 rows to be inserted into the table. === This seems not true unless I'm missing something. Hmm ... that did work as described, until we broke it :-(. This is an oversight in the 9.0 changes that added separate ModifyTuple nodes to plan trees. ModifyTuple doesn't return after each updated row, unless there's a RETURNING clause; which means that the current_tuple_count check logic in ExecutePlan() no longer stops execution as intended. Given the lack of complaints since 9.0, maybe we should not fix this but just redefine the new behavior as being correct? But it seems mighty inconsistent that the tuple limit would apply if you have RETURNING but not when you don't. In any case, the ramifications are wider than one example in the SPI docs. Thoughts? To be honest, I was surprised when I found tcount parameter is said to be applied to even INSERT. I believe people think that parameter is to limit memory consumption when returning tuples thus it'd be applied for only SELECT or DML with RETURNING. So I'm +1 for non-fix but redefine the behavior. Who wants to limit the number of rows processed inside the backend, from SPI? Thanks, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] bugfix for cursor arguments in named notation
On 2012-04-04 17:10, Tom Lane wrote:
Yeb Havingayebhavi...@gmail.com writes:
Using a cursor argument name equal to another plpgsql variable results
in the error:
cursor .. has no argument named
I think a better way would be to temporarily set
plpgsql_IdentifierLookup to IDENTIFIER_LOOKUP_DECLARE, so as to suppress
variable name lookup in the first place. The patch as you have it seems
to me to make too many assumptions about what name lookup might produce.
Thank you for looking at it. Attached is a patch that implements your
suggestion.
regards,
Yeb
From b762f447615795c65136d02495cb72f4a1293af3 Mon Sep 17 00:00:00 2001
From: Willem Yeb w...@mgrid.net
Date: Thu, 5 Apr 2012 09:53:38 +0200
Subject: [PATCH] Fix cursor has no argument named error.
---
src/pl/plpgsql/src/gram.y |3 +++
src/test/regress/expected/plpgsql.out | 18 ++
src/test/regress/sql/plpgsql.sql | 15 +++
3 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 5a555af..0f37c4f 100644
--- a/src/pl/plpgsql/src/gram.y
+++ b/src/pl/plpgsql/src/gram.y
@@ -3445,9 +3445,12 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
if (tok1 == IDENT tok2 == COLON_EQUALS)
{
char *argname;
+ IdentifierLookup oldlookup = plpgsql_IdentifierLookup;
/* Read the argument name, and find its position */
+ plpgsql_IdentifierLookup = IDENTIFIER_LOOKUP_DECLARE;
yylex();
+ plpgsql_IdentifierLookup = oldlookup;
argname = yylval.str;
for (argpos = 0; argpos row-nfields; argpos++)
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 5455ade..56cfa57 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -2420,6 +2420,24 @@ select namedparmcursor_test8();
0
(1 row)
+-- cursor parameter named the same as other plpgsql variables
+create or replace function namedparmcursor_test9(p1 int) returns int4 as $$
+declare
+ c1 cursor (p1 int, p2 int) for
+select count(*) from tenk1 where thousand = p1 and tenthous = p2;
+ n int4;
+ p2 int4 := 1006;
+begin
+ open c1 (p1 := p1, p2 := p2);
+ fetch c1 into n;
+ return n;
+end $$ language plpgsql;
+select namedparmcursor_test9(6);
+ namedparmcursor_test9
+---
+ 1
+(1 row)
+
--
-- tests for raise processing
--
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index f577dc3..6b9795d 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2053,6 +2053,21 @@ begin
end $$ language plpgsql;
select namedparmcursor_test8();
+-- cursor parameter named the same as other plpgsql variables
+create or replace function namedparmcursor_test9(p1 int) returns int4 as $$
+declare
+ c1 cursor (p1 int, p2 int) for
+select count(*) from tenk1 where thousand = p1 and tenthous = p2;
+ n int4;
+ p2 int4 := 1006;
+begin
+ open c1 (p1 := p1, p2 := p2);
+ fetch c1 into n;
+ return n;
+end $$ language plpgsql;
+
+select namedparmcursor_test9(6);
+
--
-- tests for raise processing
--
--
1.7.1
--
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] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 1:23 AM, Robert Haas robertmh...@gmail.com wrote: I don't think we're micro-optimizing, either. I don't consider avoiding a 10-second cessation of all database activity to be a micro-optimization even on a somewhat artificial benchmark. Robert is not skewing the SLRU mechanism towards this case, he is simply fixing something so gross that bug is the only word for it, and that shows up clearly on this test. I'm happy that the fix proposed has general utility without negative impact on other workloads. In general terms, I agree we shouldn't rely on pgbench as a general workload, nor should we solely tune Postgres for throughput without regard to response time. But that point is not relevant to this specific issue. -- Simon Riggs 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] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 12:25 AM, Robert Haas robertmh...@gmail.com wrote: That seems much smarter. I'm thinking this should be back patched because it appears to be fairly major, so I'm asking for some more certainty that every thing you say here is valid. No doubt much of it is valid, but that's not enough. Yeah, I was thinking about that. What we're doing right now seems pretty stupid, so maybe it's worth considering a back-patch. OTOH, I'm usually loathe to tinker with performance in stable releases. I'll defer to the opinions of others on this point. I'm also loathe to back patch. But its not very often we find a problem that causes all backends to wait behind a single I/O. The wait-behind-I/O aspect is OK because that is what is designed to happen. The unexpected bit is the point that the system *quickly* switches around so that *all* backends choose to wait behind that same I/O, which is mad. There is no doubt that your I/Os are slow here and that the specific test accentuates that, but neither of those things are rare. If it was an optimiser bug that made something run in 10sec that should have run in 10ms we fix it. So we fix this also. -- Simon Riggs 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] poll: CHECK TRIGGER?
Hello 2012/4/4 Heikki Linnakangas heikki.linnakan...@enterprisedb.com: On 04.04.2012 19:32, Tom Lane wrote: Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes: I don't think I'm getting my point across by explaining, so here's a modified version of the patch that does what I was trying to say. Minor side point: some of the diff noise in this patch comes from s/copy_plpgsql_datum/plpgsql_copy_plpgsql_datum/, which seems entirely useless. The name already contains plpgsql, and even if it didn't, there is no particular reason for plpgsql to worry about polluting global symbol namespace. Nothing else resolves against its symbols anyway, at least not on any platform we claim to support. I would therefore also argue against the other renamings like s/exec_move_row/plpgsql_exec_move_row/. Agreed. Looking closer, I'm not sure we even need to expose exec_move_row() to pl_check.c. It's only used to initialize row-type function arguments to NULL. But variables that are not explicitly initialized are NULL anyway, and the checker shouldn't use the values stored in variables for anything, so I believe that initialization in function_check() can be replaced with something much simpler or removed altogether. I returned back original names and removed exec_move_row from pl_check.c This is little bit cleaned Heikki version Regards Pavel -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com plpgsql_check_function-2012-04-05-1.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] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 5:41 AM, Simon Riggs si...@2ndquadrant.com wrote: I'm also loathe to back patch. But its not very often we find a problem that causes all backends to wait behind a single I/O. You have a point. Meanwhile, here are the benchmark results you requested. I did half hour runs with -l. Here are the 90th-100th percentile latencies, without patch and then with patch. 90 1668 1620 91 1747 1690 92 1845 1785 93 1953 1907 94 2064 2035 95 2176 2160 96 2300 2291 97 2461 2451 98 2739 2710 99 3542 3495 100 12955473 19072385 Overall tps, first without and then with patch: tps = 14546.644712 (including connections establishing) tps = 14550.515173 (including connections establishing) TPS graphs by second attached. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company attachment: tps-master.pngattachment: tps-slru-replacement-fix.png -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 12:56 PM, Robert Haas robertmh...@gmail.com wrote: Overall tps, first without and then with patch: tps = 14546.644712 (including connections establishing) tps = 14550.515173 (including connections establishing) TPS graphs by second attached. Again, I'm not that fussed about throughput because it just hides the detail. I am concerned about the distribution of response times, so I'd like to see the numbers about that if you don't mind. i.e. does the patch remove long waits. -- Simon Riggs 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] pgsql_fdw, FDW for PostgreSQL server
At 22:11 12/03/28 +0900, Shigeru HANADA wrote: ANALYZE support for foreign tables is proposed by Fujita-san in current CF, so I'd like to push it. I updated the patch to the latest HEAD. Please find attached a patch. Best regards, Etsuro Fujita postgresql-analyze-v7.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pgsql_fdw, FDW for PostgreSQL server
Sorry, I sent this email without noticing Hanada-san' earlier email. So, please look at Hanada-san's post. Best regards, Etsuro Fujita -Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Etsuro Fujita Sent: Thursday, April 05, 2012 9:36 PM To: Shigeru HANADA; Albe Laurenz Cc: Tom Lane; Kevin Grittner; Robert Haas; PostgreSQL-development; Kohei KaiGai; Martijn van Oosterhout; Hitoshi Harada Subject: Re: [HACKERS] pgsql_fdw, FDW for PostgreSQL server At 22:11 12/03/28 +0900, Shigeru HANADA wrote: ANALYZE support for foreign tables is proposed by Fujita-san in current CF, so I'd like to push it. I updated the patch to the latest HEAD. Please find attached a patch. Best regards, Etsuro Fujita -- 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] Publish checkpoint timing and sync files summary data to pg_stat_bgwriter
On 3 April 2012 12:11, Robert Haas robertmh...@gmail.com wrote: Well, for 9.2, I am asking that you rip all that stuff out of the patch altogether so we can focus on the stuff that was in the original patch. Given how we're pushed for time, I'm inclined to agree that that is the best course of action. Attached revision does not attempt to do anything with strategy writes/allocations. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services bgwriter_stats_2012_04_05.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 8:30 AM, Simon Riggs si...@2ndquadrant.com wrote: On Thu, Apr 5, 2012 at 12:56 PM, Robert Haas robertmh...@gmail.com wrote: Overall tps, first without and then with patch: tps = 14546.644712 (including connections establishing) tps = 14550.515173 (including connections establishing) TPS graphs by second attached. Again, I'm not that fussed about throughput because it just hides the detail. I am concerned about the distribution of response times, so I'd like to see the numbers about that if you don't mind. i.e. does the patch remove long waits. Sorry, I don't understand specifically what you're looking for. I provided latency percentiles in the last email; what else do you want? -- 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] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 2:24 PM, Robert Haas robertmh...@gmail.com wrote: Sorry, I don't understand specifically what you're looking for. I provided latency percentiles in the last email; what else do you want? I think he wants how many waits were there that were between 0 and 1s how many between 1s and 2s, etc. Mathematically it's equivalent but I also have trouble visualizing just how much improvement is represented by 90th percentile dropping from 1688 to 1620 (ms?) -- greg -- 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 improvements
On Wed, Apr 4, 2012 at 11:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: Stephen Frost sfr...@snowman.net writes: The single-user option *sounds* viable, but, iirc, it actually isn't due to the limitations on what can be done in that mode. Yeah. IMO the right long-term fix is to be able to run pg_dump and psql talking to a standalone backend, but nobody's gotten round to making that possible. Are you thinking about something like postgres --single --port=PORT_NUMBER_OR_SOCKET_DIRECTORY? -- 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 improvements
Robert Haas robertmh...@gmail.com writes: On Wed, Apr 4, 2012 at 11:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: Yeah. IMO the right long-term fix is to be able to run pg_dump and psql talking to a standalone backend, but nobody's gotten round to making that possible. Are you thinking about something like postgres --single --port=PORT_NUMBER_OR_SOCKET_DIRECTORY? No, opening up a port is exactly what we *don't* want it to do. Otherwise you're right back to worrying about how to make sure that unwanted connections don't get in. Notions like private socket directories don't solve this because we don't have that option available on Windows. The vague idea I had was for libpq to have a connection option that would cause it to spawn a standalone backend as a child process and communicate with that over two pipes, just like any popen'd process. The backend would see this exactly like standalone mode now, except for speaking FE/BE protocol over its stdin/stdout rather than the existing ad-hoc user interface for standalone mode. 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] bugfix for cursor arguments in named notation
Yeb Havinga yebhavi...@gmail.com writes: On 2012-04-04 17:10, Tom Lane wrote: I think a better way would be to temporarily set plpgsql_IdentifierLookup to IDENTIFIER_LOOKUP_DECLARE, Thank you for looking at it. Attached is a patch that implements your suggestion. Oh, sorry, I assumed you'd seen that I committed a fix like that last night. 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] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 9:29 AM, Greg Stark st...@mit.edu wrote: On Thu, Apr 5, 2012 at 2:24 PM, Robert Haas robertmh...@gmail.com wrote: Sorry, I don't understand specifically what you're looking for. I provided latency percentiles in the last email; what else do you want? I think he wants how many waits were there that were between 0 and 1s how many between 1s and 2s, etc. Mathematically it's equivalent but I also have trouble visualizing just how much improvement is represented by 90th percentile dropping from 1688 to 1620 (ms?) Yes, milliseconds. Sorry for leaving out that detail. I've run these scripts so many times that my eyes are crossing. Here are the latencies, bucketized by seconds, first for master and then for the patch, on the same test run as before: 0 26179411 1 3642 2 660 3 374 4 166 5 356 6 41 7 8 8 56 9 0 10 0 11 21 12 11 0 26199130 1 4840 2 267 3 290 4 40 5 77 6 36 7 3 8 2 9 33 10 37 11 2 12 1 13 4 14 5 15 3 16 0 17 1 18 1 19 1 I'm not sure I find those numbers all that helpful, but there they are. There are a couple of outliers beyond 12 s on the patched run, but I wouldn't read anything into that; the absolute worst values bounce around a lot from test to test. However, note that every bucket between 2s and 8s improves, sometimes dramatically. It's worth keeping in mind here that the system is under extreme I/O strain on this test, and the kernel responds by forcing user processes to sleep when they try to do I/O. So the long stalls that this patch eliminates are bound are actually allowing the I/O queues to drain out a little, and without that rest time, you're bound to see more I/O stalls elsewhere. It's also worth keeping in mind that this is an extremely write-intensive benchmark, and that Linux 3.2 changed behavior in this area quite a bit. It's not impossible that on an older kernel, the type of I/O thrashing that happens when too many backends are blocked by dirty_ratio or dirty_bytes might actually make this change a regression compared to letting those backends block on a semaphore, which may be a reason to NOT back-patch this change. I think that the patch is fixing a fairly obvious defect in our algorithm, but that doesn't mean that there's no set of circumstances under which it could backfire, especially when deployed onto three or four year old kernels. -- 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 improvements
On Thursday, April 05, 2012 03:46:54 PM Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Apr 4, 2012 at 11:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: Yeah. IMO the right long-term fix is to be able to run pg_dump and psql talking to a standalone backend, but nobody's gotten round to making that possible. Are you thinking about something like postgres --single --port=PORT_NUMBER_OR_SOCKET_DIRECTORY? No, opening up a port is exactly what we *don't* want it to do. Otherwise you're right back to worrying about how to make sure that unwanted connections don't get in. Notions like private socket directories don't solve this because we don't have that option available on Windows. I wonder if it wouldn't be better to pass a named pipe under windows and use a AF_UNIX socket everwhere else. Both should be pretty easily usable with the existing code. PG already seems to use named pipes under windows, so... Andres -- 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 improvements
* Andres Freund (and...@anarazel.de) wrote: I wonder if it wouldn't be better to pass a named pipe under windows and use a AF_UNIX socket everwhere else. Both should be pretty easily usable with the existing code. PG already seems to use named pipes under windows, so... I didn't think Tom's suggestion was really all that difficult to implement and sounds like a more-generally-useful change anyway (which you might want to use outside of this specific use case). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_upgrade improvements
On Thursday, April 05, 2012 04:44:11 PM Stephen Frost wrote: * Andres Freund (and...@anarazel.de) wrote: I wonder if it wouldn't be better to pass a named pipe under windows and use a AF_UNIX socket everwhere else. Both should be pretty easily usable with the existing code. PG already seems to use named pipes under windows, so... I didn't think Tom's suggestion was really all that difficult to implement and sounds like a more-generally-useful change anyway (which you might want to use outside of this specific use case). Hm. Changing libpq to use two pipes at the same time sounds considerably more invasive than basically just changing the socket creation and some minor details. Why would pipes be more useful? Its not like you could build useful pipelines with them. Also, it might open a window for implementing AF_UNIX like connections on windows... Andres -- 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 improvements
* Andres Freund (and...@anarazel.de) wrote: Hm. Changing libpq to use two pipes at the same time sounds considerably more invasive than basically just changing the socket creation and some minor details. It's not something we'd back-patch, but I don't believe it'd be very difficult to implement.. Why would pipes be more useful? Its not like you could build useful pipelines with them. The point is to avoid the risk that someone else could connect to the database at the same time you're doing work on it. Also, it might open a window for implementing AF_UNIX like connections on windows... That's an unrelated discussion, imv.. Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_upgrade improvements
Hi, Not sure if were just missing each others point? On Thursday, April 05, 2012 05:20:04 PM Stephen Frost wrote: Why would pipes be more useful? Its not like you could build useful pipelines with them. The point is to avoid the risk that someone else could connect to the database at the same time you're doing work on it. I got that. I just fail to see what the advantage of using two pipes instead of one socket as every other plain connection would be? Using named pipes solves that tidbit from Tom: Notions like private socket directories don't solve this because we don't have that option available on Windows. If you have named pipes or AF_UNIX sockets you can solve that by either just passing the fd to your child and not allowing any access to it (no problem on either platform) or by using a private directory. Andres -- 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 improvements
Stephen Frost sfr...@snowman.net writes: * Andres Freund (and...@anarazel.de) wrote: Why would pipes be more useful? Its not like you could build useful pipelines with them. The point is to avoid the risk that someone else could connect to the database at the same time you're doing work on it. Right. Unless I misunderstand the semantics of named pipes on Windows, we don't want that because in principle some unrelated process could connect to it as soon as you set it up. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade improvements
Andres Freund and...@anarazel.de writes: The point is to avoid the risk that someone else could connect to the database at the same time you're doing work on it. I got that. I just fail to see what the advantage of using two pipes instead of one socket as every other plain connection would be? Yeah, that would be a small pain in the neck, but it eliminates a huge pile of practical difficulties, like your blithe assumption that you can find a private directory somewhere (wrong) or disallow access to other people (also wrong, if they are using the same account as you). The short answer is that sockets and named pipes are *meant* to be publicly accessible. Guaranteeing that they are not is a difficult task full of possibilities for security holes. 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] Patch: add timing of buffer I/O requests
On Tue, Mar 27, 2012 at 8:30 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Mar 27, 2012 at 3:22 PM, Ants Aasma a...@cybertec.at wrote: On Tue, Mar 27, 2012 at 9:58 PM, Robert Haas robertmh...@gmail.com wrote: I've committed the core of this. I left out the stats collector stuff, because it's still per-table and I think perhaps we should back off to just per-database. I changed it so that it does not conflate wait time with I/O time. Maybe there should be a separate method of measuring wait time, but I don't think it's a good idea for the per-backend stats to measure a different thing than what gets reported up to the stats collector - we should have ONE definition of each counter. I also tweaked the EXPLAIN output format a bit, and the docs. Thank you for working on this. Taking a fresh look at it, I agree with you that conflating waiting for backend local IO, and IO performed by some other backend might paint us into a corner. For most workloads the wait for IO performed by others should be the minority anyway. I won't really miss the per table stats. But if the pg_stat_statements normalisation patch gets commited, it would be really neat to also have IO waits there. It would be much easier to quickly diagnose what is causing all this IO issues. So, the pg_stat_statements part of this is committed now. Do you want to prepare a revised patch to add per-database counters to the statistics collector? I think that might be a good idea... Hearing nothing further on this point, I went and did it myself. In the process I noticed a couple of things that I think we need to fix. Currently, the statistics views that include timing information are displayed in milliseconds (see pg_stat_user_functions), while the underlying SQL-callable functions return the data in microseconds. Whether or not this was a good design decision, we're stuck with it now; the documentation implies that the views and functions use the same units. I'll go fix that next. Meanwhile, pg_stat_statements converts the same data to seconds but makes it a double rather than a bigint. I think that was a bad idea and we should make it consistent use a bigint and milliseconds while we still can. -- 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] Publish checkpoint timing and sync files summary data to pg_stat_bgwriter
On Wed, Mar 28, 2012 at 7:23 AM, Robert Haas robertmh...@gmail.com wrote: Although I liked the idea of separating this out, I wasn't thinking we should do it as part of this patch. It seems like an independent project. At any rate, I strongly agree that counting the number of strategy allocations is not really a viable proxy for counting the number of backend writes. You can't know how many of those actually got dirtied. I was thinking that it would be part of this patch, but only because Greg mentioned the view format stabilizing. If it is going to be frozen, I wanted this in it. But I guess stabilize and fossilize are different things... Since any backend write is necessarily the result of that backend trying to allocate a buffer, I think maybe we should just count whether the number of times it was trying to allocate a buffer *using a BAS* vs. the number of times it was trying to allocate a buffer *not using a BAS*. That is, decide whether or not it's a strategy write not based on whether the buffer came in via a strategy allocation, but rather based on whether it's going out as a result of a strategy allocation. Yes, exactly. Cheers, Jeff -- 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 improvements
On Thursday, April 05, 2012 05:39:19 PM Tom Lane wrote: Andres Freund and...@anarazel.de writes: The point is to avoid the risk that someone else could connect to the database at the same time you're doing work on it. I got that. I just fail to see what the advantage of using two pipes instead of one socket as every other plain connection would be? Yeah, that would be a small pain in the neck, but it eliminates a huge pile of practical difficulties, like your blithe assumption that you can find a private directory somewhere (wrong) or disallow access to other people (also wrong, if they are using the same account as you). I don't think this needs to protect against malicious intent of a user running with the *same* privileges as the postmaster. That one can simply delete the whole cluster anyway. For everybody else you can just create a directory in PGDATA and revoke all permissions on it for everybody but the owner. For named pipes you could just create a random name with permissions only for the current user (thats possible in the same call) and be done with it. Andres -- 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 improvements
Andres Freund and...@anarazel.de writes: On Thursday, April 05, 2012 05:39:19 PM Tom Lane wrote: Yeah, that would be a small pain in the neck, but it eliminates a huge pile of practical difficulties, like your blithe assumption that you can find a private directory somewhere (wrong) or disallow access to other people (also wrong, if they are using the same account as you). I don't think this needs to protect against malicious intent of a user running with the *same* privileges as the postmaster. Who said anything about malicious intent? Please re-read the original gripe in this thread. There's nothing un-legitimate about, eg, clients trying to connect to the database during your maintenance window. What we want is to be sure that nobody can connect to the database except the person running the standalone instance. To my mind sure means sure; it does not include qualifiers like unless some other process tries to do the same thing at about the same time. 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] Finer Extension dependencies
On Tue, Apr 3, 2012 at 4:15 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Tom Lane t...@sss.pgh.pa.us writes: On Apr 2, 2012, at 11:24 AM, Peter Eisentraut wrote: Or an extension could specify itself which version numbering scheme it uses. This just has to be a reference to a type, which in turn could be semver, debversion, or even just numeric or text (well, maybe name). Then you'd just need to use the comparison operators of that type to figure things out. That's exactly what I'm trying to avoid :) Well, the primary argument for avoiding version comparison semantics to begin with was exactly that we didn't want to mandate a particular version-numbering scheme. However, if we're going to decide that we have to have version comparisons, I think we should just bite the bullet and specify one version numbering scheme. More than one is going to add complexity, sow confusion, and not really buy anything. I still believe we don't *need* any numbering scheme for extension versions. Now, maybe we as a community want one. I'm voting against. Examining this thread, I think there is insufficient consensus to push this patch into 9.2. It's not entirely clear that this patch isn't what we want, but it's not entirely clear that it is what we want either, and I think it's too late in the release cycle to push anything into the release that we're not fairly sure we actually want, because there won't be time to revisit that decision before this ends up out in the wild. It's also not the sort of thing we can just whack around in the next release if we get it wrong, because APIs for coping with versioning are exactly the kind of thing that you can't go change at the drop of a hat. I'm therefore marking this Returned with Feedback. -- 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 improvements
On Thursday, April 05, 2012 06:12:48 PM Tom Lane wrote: Andres Freund and...@anarazel.de writes: On Thursday, April 05, 2012 05:39:19 PM Tom Lane wrote: Yeah, that would be a small pain in the neck, but it eliminates a huge pile of practical difficulties, like your blithe assumption that you can find a private directory somewhere (wrong) or disallow access to other people (also wrong, if they are using the same account as you). I don't think this needs to protect against malicious intent of a user running with the *same* privileges as the postmaster. Who said anything about malicious intent? Please re-read the original gripe in this thread. There's nothing un-legitimate about, eg, clients trying to connect to the database during your maintenance window. Yes, there is not. But those clients won't connect to a socket in some directory thats created extra for this purpose which they don't even know the name of. Scanning the directory tree for that would require malicious intent. What we want is to be sure that nobody can connect to the database except the person running the standalone instance. To my mind sure means sure; it does not include qualifiers like unless some other process tries to do the same thing at about the same time. Its not like creating a file/directory/pipename with a random name and retrying if it already exists is an especially hard thing. That does make *sure* it does not happen by accident. Beside the fact that single run backend should already make sure were not running concurrently. So even a *fixed* atomically created filename name should be enough if its not the regular name and in a place thats not accessible from the outside. Anyway, I don't think those reasons are sensible, but I don't care enough to argue further. Andres -- 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] pgsql_fdw, FDW for PostgreSQL server
Albe Laurenz laurenz.a...@wien.gv.at writes: I think that the price of a remote table scan is something we should be willing to pay for good local statistics. And there is always the option not to analyze the foreign table if you are not willing to pay that price. Maybe the FDW API could be extended so that foreign data wrappers can provide a random sample to avoid a full table scan. The one thing that seems pretty clear from this discussion is that one size doesn't fit all. I think we really ought to provide a hook so that the FDW can determine whether ANALYZE applies to its foreign tables at all, and how to obtain the sample rows if it does. Since we've already whacked the FDW API around in incompatible ways for 9.2, now is probably a good time to add that. I'm inclined to say this should happen whether or not we accept any of the currently proposed patches for 9.2, because if the hook is there it will provide a way for people to experiment with foreign-table ANALYZE operations outside of core. 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] Speed dblink using alternate libpq tuple storage
On Wed, Apr 04, 2012 at 06:41:00PM -0400, Tom Lane wrote:
Marko Kreen mark...@gmail.com writes:
On Wed, Apr 4, 2012 at 10:17 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Given the lack of consensus around the suspension API, maybe the best
way to get the underlying libpq patch to a committable state is to take
it out --- that is, remove the return zero option for row processors.
Agreed.
Done that way.
Minor cleanups:
* Change callback return value to be 'bool': 0 is error.
Currently the accepted return codes are 1 and -1,
which is weird.
If we happen to have the 'early-exit' logic in the future,
it should not work via callback return code. So keeping the 0
in reserve is unnecessary.
* Support exceptions in multi-statement PQexec() by storing
finished result under PGconn temporarily. Without doing it,
the result can leak if callback longjmps while processing
next result.
* Add caution to docs for permanently keeping custom callback.
This API fragility is also reason why early-exit (if it appears)
should not work via callback - instead it should give safer API.
--
marko
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0ec501e..1e7678c 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -5608,6 +5608,16 @@ defaultNoticeProcessor(void *arg, const char *message)
caution
para
+It's dangerous to leave custom row processor attached to connection
+permanently as there might be occasional queries that expect
+regular PGresult behaviour. So unless it is certain all code
+is familiar with custom callback, it is better to restore default
+row processor after query is finished.
+ /para
+ /caution
+
+ caution
+ para
The row processor function sees the rows before it is known whether the
query will succeed overall, since the server might return some rows before
encountering an error. For proper transactional behavior, it must be
@@ -5674,8 +5684,8 @@ typedef struct pgDataValue
parametererrmsgp/parameter is an output parameter used only for error
reporting. If the row processor needs to report an error, it can set
literal*/parametererrmsgp/parameter to point to a suitable message
- string (and then return literal-1/). As a special case, returning
- literal-1/ without changing literal*/parametererrmsgp/parameter
+ string (and then return literal0/). As a special case, returning
+ literal0/ without changing literal*/parametererrmsgp/parameter
from its initial value of NULL is taken to mean quoteout of memory/.
/para
@@ -5702,10 +5712,10 @@ typedef struct pgDataValue
para
The row processor function must return either literal1/ or
- literal-1/.
+ literal0/.
literal1/ is the normal, successful result value; applicationlibpq/
will continue with receiving row values from the server and passing them to
- the row processor. literal-1/ indicates that the row processor has
+ the row processor. literal0/ indicates that the row processor has
encountered an error. In that case,
applicationlibpq/ will discard all remaining rows in the result set
and then return a literalPGRES_FATAL_ERROR/ typePGresult/type to
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 03fd6e4..90f6d6a 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2897,6 +2897,11 @@ closePGconn(PGconn *conn)
* absent */
conn-asyncStatus = PGASYNC_IDLE;
pqClearAsyncResult(conn); /* deallocate result */
+ if (conn-tempResult)
+ {
+ PQclear(conn-tempResult);
+ conn-tempResult = NULL;
+ }
pg_freeaddrinfo_all(conn-addrlist_family, conn-addrlist);
conn-addrlist = NULL;
conn-addr_cur = NULL;
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 86f157c..554df94 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1028,7 +1028,7 @@ PQgetRowProcessor(const PGconn *conn, void **param)
/*
* pqStdRowProcessor
* Add the received row to the PGresult structure
- * Returns 1 if OK, -1 if error occurred.
+ * Returns 1 if OK, 0 if error occurred.
*
* Note: param should point to the PGconn, but we don't actually need that
* as of the current coding.
@@ -1059,7 +1059,7 @@ pqStdRowProcessor(PGresult *res, const PGdataValue *columns,
tup = (PGresAttValue *)
pqResultAlloc(res, nfields * sizeof(PGresAttValue), TRUE);
if (tup == NULL)
- return -1;
+ return 0;
for (i = 0; i nfields; i++)
{
@@ -1078,7 +1078,7 @@ pqStdRowProcessor(PGresult *res, const PGdataValue *columns,
val = (char *) pqResultAlloc(res, clen + 1, isbinary);
if (val == NULL)
-return -1;
+return 0;
/* copy and zero-terminate the data (even if it's binary) */
memcpy(val, columns[i].value, clen);
@@ -1091,7 +1091,7 @@ pqStdRowProcessor(PGresult *res, const PGdataValue *columns,
/* And add the tuple to the
Re: [HACKERS] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 7:05 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 5, 2012 at 9:29 AM, Greg Stark st...@mit.edu wrote: On Thu, Apr 5, 2012 at 2:24 PM, Robert Haas robertmh...@gmail.com wrote: Sorry, I don't understand specifically what you're looking for. I provided latency percentiles in the last email; what else do you want? I think he wants how many waits were there that were between 0 and 1s how many between 1s and 2s, etc. Mathematically it's equivalent but I also have trouble visualizing just how much improvement is represented by 90th percentile dropping from 1688 to 1620 (ms?) Yes, milliseconds. Sorry for leaving out that detail. I've run these scripts so many times that my eyes are crossing. Here are the latencies, bucketized by seconds, first for master and then for the patch, on the same test run as before: 0 26179411 1 3642 2 660 3 374 4 166 5 356 6 41 7 8 8 56 9 0 10 0 11 21 12 11 0 26199130 1 4840 2 267 3 290 4 40 5 77 6 36 7 3 8 2 9 33 10 37 11 2 12 1 13 4 14 5 15 3 16 0 17 1 18 1 19 1 I'm not sure I find those numbers all that helpful, but there they are. There are a couple of outliers beyond 12 s on the patched run, but I wouldn't read anything into that; the absolute worst values bounce around a lot from test to test. However, note that every bucket between 2s and 8s improves, sometimes dramatically. However, if it improved a bucket by pushing the things out of it into a higher bucket, that is not really an improvement. At 8 seconds *or higher*, for example, it goes from 88 things in master to 90 things in the patch. Maybe something like a Kaplan-Meier survival curve analysis would be the way to go (where a long transaction survival is bad). But probably overkill. What were full_page_writes and wal_buffers set to for these runs? It's worth keeping in mind here that the system is under extreme I/O strain on this test, and the kernel responds by forcing user processes to sleep when they try to do I/O. Should the tests be dialed back a bit so that the I/O strain is less extreme? Analysis is probably best done right after where the scalability knee is, not long after that point where the server has already collapsed to a quivering mass. Cheers, Jeff -- 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] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 3:05 PM, Robert Haas robertmh...@gmail.com wrote: I'm not sure I find those numbers all that helpful, but there they are. There are a couple of outliers beyond 12 s on the patched run, but I wouldn't read anything into that; the absolute worst values bounce around a lot from test to test. However, note that every bucket between 2s and 8s improves, sometimes dramatically. The numbers seem pretty compelling to me. They seem to indicate that you've killed one of the big source of stalls but that there are more lurking including at least one which causes small number of small stalls. The only fear I have is that I'm still wondering what happens to your code when *all* the buffers become blocked on I/O. Can you catch whether this ever occurred in your test and explain what should happen in that case? -- greg -- 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] Speed dblink using alternate libpq tuple storage
Marko Kreen mark...@gmail.com writes: Minor cleanups: * Change callback return value to be 'bool': 0 is error. Currently the accepted return codes are 1 and -1, which is weird. No, I did it that way intentionally, with the thought that we might add back return code zero (or other return codes) in the future. If we go with bool then sensible expansion is impossible, or at least ugly. (I think it was you that objected to 0/1/2 in the first place, no?) If we happen to have the 'early-exit' logic in the future, it should not work via callback return code. This assumption seems unproven to me, and even if it were, it doesn't mean we will never have any other exit codes. * Support exceptions in multi-statement PQexec() by storing finished result under PGconn temporarily. Without doing it, the result can leak if callback longjmps while processing next result. I'm unconvinced this is a good thing either. 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] Another review of URI for libpq, v7 submission
On ons, 2012-03-28 at 01:49 +0300, Alex wrote: Attached is a gzipped v10, complete with the above fixes and fixes to bugs found by Heikki. Documentation and code comments are also finally updated. The compiler warning is still there: fe-connect.c: In function ‘conninfo_parse’: fe-connect.c:4122:20: error: unused variable ‘option’ [-Werror=unused-variable] -- 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 improvements
On Thu, Apr 5, 2012 at 17:34, Tom Lane t...@sss.pgh.pa.us wrote: Stephen Frost sfr...@snowman.net writes: * Andres Freund (and...@anarazel.de) wrote: Why would pipes be more useful? Its not like you could build useful pipelines with them. The point is to avoid the risk that someone else could connect to the database at the same time you're doing work on it. Right. Unless I misunderstand the semantics of named pipes on Windows, we don't want that because in principle some unrelated process could connect to it as soon as you set it up. By default it does, and that's what we're using it for (pg_ctl kill). That can be handled with permissions and counters, I think. But more importantly in this case, you cannot communicate across a named pipe on windows with the normal socket calls send() and recv(). You have to use the Windows API file calls like ReadFile() and WriteFile(). So it would require some rather major surgery to use that. This is why the pgpipe implementation we had used a socketpair instead of named pipes. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Finer Extension dependencies
Robert Haas robertmh...@gmail.com writes: Examining this thread, I think there is insufficient consensus to push this patch into 9.2. It's not entirely clear that this patch isn't what we want, but it's not entirely clear that it is what we want either, and I think it's too late in the release cycle to push anything into the release that we're not fairly sure we actually want, because there won't be time to revisit that decision before this ends up out in the wild. It's also not the sort of thing we can just whack around in the next release if we get it wrong, because APIs for coping with versioning are exactly the kind of thing that you can't go change at the drop of a hat. I'm therefore marking this Returned with Feedback. Fair enough I guess. The first extension patch took me more than 2 years∫of brewing before we get to a consensus on what we wanted in core, I shouldn't have assumed it would ease the path for next improvements too much. 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] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 12:30 PM, Jeff Janes jeff.ja...@gmail.com wrote: I'm not sure I find those numbers all that helpful, but there they are. There are a couple of outliers beyond 12 s on the patched run, but I wouldn't read anything into that; the absolute worst values bounce around a lot from test to test. However, note that every bucket between 2s and 8s improves, sometimes dramatically. However, if it improved a bucket by pushing the things out of it into a higher bucket, that is not really an improvement. At 8 seconds *or higher*, for example, it goes from 88 things in master to 90 things in the patch. Maybe something like a Kaplan-Meier survival curve analysis would be the way to go (where a long transaction survival is bad). But probably overkill. I think we are well into the realm of overkill here. At ten thousand feet, this patch is preventing us from having every backend wait on an I/O that only one of those backends actually cares about. While it possible that, in some set of circumstances, a bizarre behavior of that type could happen to work out well, we are not in the business of coding stupid algorithms and hoping that a meteor hits us and makes them work out to be wins. We should not be too afraid of changing behavior that is obviously wrong, and this is an example of that. I've presented extensive data showing that, in fact, this patch eliminates exactly the behavior that it is intended to eliminate without causing regressions elsewhere. The fact that there is some criteria that you can apply to the results of those tests where the patch doesn't win is unsurprising. If you slice data that contains a large amount of random noise in enough different ways, one of them will almost certainly show a regression; that's what noise is. Yes, it's true that if you look at the number of transactions that stall for = 8s, the patch appears to be a very slight loss. But first, this is only one trial. A second run would produce different numbers that might be better or worse. And second, if you picked = 7s, then the run with the patch would win by about as much as it loses in the = 8 s test. If you pick = 11 s, it wins by a lot. What were full_page_writes and wal_buffers set to for these runs? on, 16MB. It's worth keeping in mind here that the system is under extreme I/O strain on this test, and the kernel responds by forcing user processes to sleep when they try to do I/O. Should the tests be dialed back a bit so that the I/O strain is less extreme? Analysis is probably best done right after where the scalability knee is, not long after that point where the server has already collapsed to a quivering mass. I think that would be missing the point, which IMV is to avoid having a server collapse into a quivering mass just because we apply a heavy write workload. Actually, I think we're doing pretty well with that up to about 16-24 concurrent writers, and even going somewhat beyond that we're not really slowing down so much as just not scaling. For many people that's not a problem because they don't have that much write workload, but if someone does have that much write workload, we can hardly tell them: well, you know, if you don't apply so much load to the database, it will perform better. No kidding. Anybody can write a database that performs well when it's totally unloaded. The trick is to write one that scales as far as possible and degrades as gracefully as possible when it can no longer scale. -- 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] Speed dblink using alternate libpq tuple storage
On Thu, Apr 05, 2012 at 12:49:37PM -0400, Tom Lane wrote: Marko Kreen mark...@gmail.com writes: Minor cleanups: * Change callback return value to be 'bool': 0 is error. Currently the accepted return codes are 1 and -1, which is weird. No, I did it that way intentionally, with the thought that we might add back return code zero (or other return codes) in the future. If we go with bool then sensible expansion is impossible, or at least ugly. (I think it was you that objected to 0/1/2 in the first place, no?) Well, I was the one who added the 0/1/2 in the first place, then I noticed that -1/0/1 works better as a triple. But the early-exit from callback creates unnecessary fragility so now I'm convinced we don't want to do it that way. If we happen to have the 'early-exit' logic in the future, it should not work via callback return code. This assumption seems unproven to me, and even if it were, it doesn't mean we will never have any other exit codes. I cannot even imagine why we would want new codes, so it seems like unnecessary mess in API. But it's a minor issue, so if you intend to keep it, I won't push it further. * Support exceptions in multi-statement PQexec() by storing finished result under PGconn temporarily. Without doing it, the result can leak if callback longjmps while processing next result. I'm unconvinced this is a good thing either. This is less minor issue. Do we support longjmp() or not? Why are you convinced that it's not needed? -- marko -- 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] patch: improve SLRU replacement algorithm
On Thu, Apr 5, 2012 at 12:44 PM, Greg Stark st...@mit.edu wrote: On Thu, Apr 5, 2012 at 3:05 PM, Robert Haas robertmh...@gmail.com wrote: I'm not sure I find those numbers all that helpful, but there they are. There are a couple of outliers beyond 12 s on the patched run, but I wouldn't read anything into that; the absolute worst values bounce around a lot from test to test. However, note that every bucket between 2s and 8s improves, sometimes dramatically. The numbers seem pretty compelling to me. Thanks. They seem to indicate that you've killed one of the big source of stalls but that there are more lurking including at least one which causes small number of small stalls. The data in my OP identifies the other things that can cause stalls = 100 ms with considerable specificity. The only fear I have is that I'm still wondering what happens to your code when *all* the buffers become blocked on I/O. Can you catch whether this ever occurred in your test and explain what should happen in that case? If all the buffers are I/O-busy, it just falls back to picking the least-recently-used buffer, which is a reasonable heuristic, since that I/O is likely to be done first. However, when I ran this with all the debugging instrumentation enabled, it reported no waits in slru.c consistent with that situation ever having occurred. So if something like that did happen during the test run, it produced a wait of less than 100 ms, but I think it's more likely that it never happened at all. I think part of the confusion here may relate to a previous discussion about increasing the number of CLOG buffers. During that discussion, I postulated that increasing the number of CLOG buffers improved performance because we could encounter a situation where every buffer is I/O-busy, causing new backends that wanted to perform an I/O to have to wait until some backend that had been doing an I/O finished it. It's now clear that I was totally wrong, because you don't need to have every buffer busy before the next backend that needs a CLOG page blocks on an I/O. As soon as ONE backend blocks on a CLOG buffer I/O, every other backend that needs to evict a page will pile up on the same I/O. I just assumed that we couldn't possibly be doing anything that silly, but we are. So here's my new theory: the real reason why increasing the number of CLOG pages improved performance is because it caused dirty pages to reach the tail of the LRU list less frequently. It's particularly bad if a page gets written and fsync'd but then someone still needs to WRITE that page, so it gets pulled back in and written and fsync'd a second time. Such events are less likely with more buffers. Of course, increasing the number of buffers also decreases cache pressure in general. What's clear from these numbers as well is that there is a tremendous amount of CLOG cache churn, and therefore we can infer that most of those I/Os complete almost immediately - if they did not, it would be impossible to replace 5000 CLOG buffers in a second no matter how many backends you have. It's the occasional I/Os that don't completely almost immediately that are at issue here. -- 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] Fix PL/Python metadata when there is no result
On lör, 2012-03-24 at 16:24 -0400, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 17:14 -0500, Tom Lane wrote: I said it was a reasonable alternative, not that it was the only one we should consider. The behavior of .nrows() might be accidental, but perhaps it is a preferable model to adopt. After pondering this for several days now I still think the best approach is to change .nrows() to return None for utility commands and have the other metadata functions throw exceptions. OK, I don't have strong feelings about it. Well, scratch that. This whole area is sloppily documented. resultset.nrows() returns SPI_processed, which is the number of rows, er, processed by the statement, which may or may not be the number of rows returned. So a statement that returns no result set could very well have nrows() 0. The number of rows returned can be obtained by using len(resultset) (not documented, but one could perhaps guess it). But len() cannot return None, so we cannot use that as a marker. The alternatives are now to introduce a new function like has_rows() that returns True iff result rows exist and therefore result metadata can be fetched, or go back to having coltypes() et al. return None when no metadata exists. I'm in favor of the latter, because the former would add somewhat needless complications and doesn't really add any robustness or the like. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Last gasp
These patches aren't marked with a committer FK arrays ECPG fetch foreign stats command triggers check function parallel pg_dump Does that mean myself or others should be claiming them for commit/reject? -- Simon Riggs 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] Fix PL/Python metadata when there is no result
2012/4/5 Peter Eisentraut pete...@gmx.net On lör, 2012-03-24 at 16:24 -0400, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On ons, 2012-03-07 at 17:14 -0500, Tom Lane wrote: I said it was a reasonable alternative, not that it was the only one we should consider. The behavior of .nrows() might be accidental, but perhaps it is a preferable model to adopt. After pondering this for several days now I still think the best approach is to change .nrows() to return None for utility commands and have the other metadata functions throw exceptions. OK, I don't have strong feelings about it. Well, scratch that. This whole area is sloppily documented. resultset.nrows() returns SPI_processed, which is the number of rows, er, processed by the statement, which may or may not be the number of rows returned. So a statement that returns no result set could very well have nrows() 0. The number of rows returned can be obtained by using len(resultset) (not documented, but one could perhaps guess it). But len() cannot return None, so we cannot use that as a marker. The alternatives are now to introduce a new function like has_rows() that returns True iff result rows exist and therefore result metadata can be fetched, or go back to having coltypes() et al. return None when no metadata exists. I'm in favor of the latter, because the former would add somewhat needless complications and doesn't really add any robustness or the like. I consider that this is an error to request metadata when the query does not return some. For example: UPDATE mytable SET value = 1 does not return column metadata, so user is not supposed to col coltypes(). That's why I propose to return an error. coltypes() is supposed to return a sequence, not None. Checking for None is a bad coding practise IMO, especially when dealing with lists. But anyway, returning None or raising an error is still much better than crashing :-) Cheers, -- Jean-Baptiste Quenot
Re: [HACKERS] Fix PL/Python metadata when there is no result
Peter Eisentraut pete...@gmx.net writes: The alternatives are now to introduce a new function like has_rows() that returns True iff result rows exist and therefore result metadata can be fetched, or go back to having coltypes() et al. return None when no metadata exists. I'm in favor of the latter, because the former would add somewhat needless complications and doesn't really add any robustness or the like. Seems sensible to me. We had better also document what nrows() really does. Should we also introduce a new function that is number of rows in the resultset, rather than depending on len()? I think it might be useful, or at least consistent, to have a function defined as number of rows, or None if the resultset does not contain rows. In particular, I think it is important to be able to distinguish between a command result (which cannot possibly contain rows) and a query result that happens to contain zero rows. 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] Last gasp
On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs si...@2ndquadrant.com wrote: These patches aren't marked with a committer FK arrays ECPG fetch foreign stats command triggers check function parallel pg_dump Does that mean myself or others should be claiming them for commit/reject? I've been right in the middle of the command triggers stuff, so I suppose I would pick that up for commit if it were ready, but there hasn't been a new version this week unless I've missed it, and even if the new version arrived right this minute, I don't think I or anyone else can do a good job committing a patch of that size in the time remaining. So I think it's time to formally put that one out of its misery. I think the ECPG fetch patch is about ready to go. Normally Michael Meskes handles all ECPG patches, but I'm not sure what his schedule is like. I'm not sure what the politics are of someone else touching that code. Heikki recently produced a revision of the check function patch, but I'm not sure whether he's planning to commit that or whether it's a demonstration of a broader rework he wants Pavel (or someone else) to do. At any rate, I think it's up to him whether or not to commit that. I think Andrew is working on parallel pg_dump, so I suspect the rest of us should stay out of the way. I've also looked at it extensively and will work on it if Andrew isn't, but I don't think it's ready to commit. A few preliminary pieces could go in, perhaps. I am not sure what the state of the foreign stats patch is, or FK arrays. -- 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] Publish checkpoint timing and sync files summary data to pg_stat_bgwriter
On Thu, Apr 5, 2012 at 9:05 AM, Peter Geoghegan pe...@2ndquadrant.com wrote: On 3 April 2012 12:11, Robert Haas robertmh...@gmail.com wrote: Well, for 9.2, I am asking that you rip all that stuff out of the patch altogether so we can focus on the stuff that was in the original patch. Given how we're pushed for time, I'm inclined to agree that that is the best course of action. Attached revision does not attempt to do anything with strategy writes/allocations. I have committed this after extensive revisions. I removed the sync_files count, since no one ever explained what that was good for. I renamed a bunch of things so that it was clear that these stats were referring to checkpoints rather than anything else. I moved the documentation to what I believe to be the correct place. I fixed a few other assorted things, too, and reverted a couple hunks that didn't seem to me to be adding anything. -- 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] Last gasp
On 05.04.2012 21:00, Robert Haas wrote: Heikki recently produced a revision of the check function patch, but I'm not sure whether he's planning to commit that or whether it's a demonstration of a broader rework he wants Pavel (or someone else) to do. At any rate, I think it's up to him whether or not to commit that. Yeah, I guess it's up to me, now that I've touched that code. I don't feel ready to commit it yet. It needs some more cleanup, which I could do, but frankly even with the refactoring I did I'm still not totally happy with the way it works. I feel that there ought to be a less duplicative approach, but I don't have any more concrete proposals. Unless someone more motivated picks up the patch now, I think it needs to be pushed to 9.3. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review of pg_archivecleanup -x option patch
On Wed, Mar 28, 2012 at 12:13 PM, Jaime Casanova ja...@2ndquadrant.com wrote: On Wed, Mar 28, 2012 at 8:47 AM, Robert Haas robertmh...@gmail.com wrote: $ ./pg_archivecleanup -x bz2 /tmp 000100010058 Hmm, but I thought that the idea was that the extension was optional. Perhaps I'm missing something but I don't think the previous patch will complain about that either; or at least I don't see why the behavior should be any different. Can someone enlighten me on this point? mmm! you're right... it's not complaining either... i was sure it was... and i'm not sure i want to contor things for that... so, just forget my last mail about that... your refactor is just fine for me OK, committed that way. -- 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] Last gasp
Robert Haas robertmh...@gmail.com writes: On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs si...@2ndquadrant.com wrote: These patches aren't marked with a committer I think the ECPG fetch patch is about ready to go. Normally Michael Meskes handles all ECPG patches, but I'm not sure what his schedule is like. I'm not sure what the politics are of someone else touching that code. I think we should leave that one for Michael. Frankly, none of the rest of us pay enough attention to ecpg to be candidates to take responsibility for nontrivial patches there. In fact, for *most* of these patches the fact that they're still here should give you pause about just committing them. I am not sure what the state of the foreign stats patch is, or FK arrays. I'm taking the foreign stats one; as I mentioned a bit ago, I think we need to push an FDW ANALYZE hook into 9.2. I don't like the details of what's in the current submission but I think it's fixable with not much effort. The FK arrays one I'm kind of queasy about. It's a cool-sounding idea but I'm not convinced that all the corner-case details have been adequately thought through, and I'm scared of being unable to fix any such bugs in later versions because of backwards compatibility worries. It'd be a lot better to be pushing this in at the start of a devel cycle than the end. Most of the rest of this stuff I'm about ready to put off to 9.3. The risk/reward ratio for committing it on the last day of the last 9.2 fest doesn't look good. 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] checkpoint patches
On 4/3/12 11:30 PM, Greg Smith wrote: On 03/25/2012 04:29 PM, Jim Nasby wrote: Another $0.02: I don't recall the community using pg_bench much at all to measure latency... I believe it's something fairly new. I point this out because I believe there are differences in analysis that you need to do for TPS vs latency. I think Robert's graphs support my argument; the numeric X-percentile data might not look terribly good, but reducing peak latency from 100ms to 60ms could be a really big deal on a lot of systems. My intuition is that one or both of these patches actually would be valuable in the real world; it would be a shame to throw them out because we're not sure how to performance test them... One of these patches is already valuable in the real world. There it will stay, while we continue mining it for nuggets of deeper insight into the problem that can lead into a better test case. Starting at pgbench latency worked out fairly well for some things. Last year around this time I published some results I summarized at http://blog.2ndquadrant.com/en/gregs-planetpostgresql/2011/02/ , which included things like worst-case latency going from =34 seconds on ext3 to =5 seconds on xfs. The problem I keep hitting now is that 2 to 5 second latencies on Linux are extremely hard to get rid of if you overwhelm storage--any storage. That's where the wall is, where if you try to drive them lower than that you pay some hard trade-off penalties, if it works at all. Take a look at the graph I've attached. That's a slow drive not able to keep up with lots of random writes stalling, right? No. It's a Fusion-io card that will do 600MB/s of random I/O. But clog it up with an endless stream of pgbench writes, never with any pause to catch up, and I can get Linux to clog it for many seconds whenever I set it loose. If there's a fundamental flaw in how linux deals with heavy writes that means you can't rely on certain latency windows, perhaps we should be looking at using a different OS to test those cases... -- Jim C. Nasby, Database Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- 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] Last gasp
On Thu, Apr 5, 2012 at 7:00 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs si...@2ndquadrant.com wrote: These patches aren't marked with a committer FK arrays ECPG fetch foreign stats command triggers check function parallel pg_dump Does that mean myself or others should be claiming them for commit/reject? I've been right in the middle of the command triggers stuff, so I suppose I would pick that up for commit if it were ready, but there hasn't been a new version this week unless I've missed it, and even if the new version arrived right this minute, I don't think I or anyone else can do a good job committing a patch of that size in the time remaining. So I think it's time to formally put that one out of its misery. I think doing so will cause substantial misery for many users. I find it hard to believe that such a simple concept hasn't managed to produce some workable subset after months of work. -- Simon Riggs 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] Last gasp
On Thu, Apr 5, 2012 at 2:17 PM, Simon Riggs si...@2ndquadrant.com wrote: On Thu, Apr 5, 2012 at 7:00 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs si...@2ndquadrant.com wrote: These patches aren't marked with a committer FK arrays ECPG fetch foreign stats command triggers check function parallel pg_dump Does that mean myself or others should be claiming them for commit/reject? I've been right in the middle of the command triggers stuff, so I suppose I would pick that up for commit if it were ready, but there hasn't been a new version this week unless I've missed it, and even if the new version arrived right this minute, I don't think I or anyone else can do a good job committing a patch of that size in the time remaining. So I think it's time to formally put that one out of its misery. I think doing so will cause substantial misery for many users. I find it hard to believe that such a simple concept hasn't managed to produce some workable subset after months of work. I am not interested in relitigating on this thread what has already been extensively discussed nearby. Dimitri and I agreed on numerous changes to try to make the behavior sane, and those changes were suggested and agreed to for good reason. We didn't agree on every point, of course, but we did agree on most of it, and there is no patch that implements what was agreed. Even if there were, there is not time to review and commit a heavily revised version of a 1000 line patch before tomorrow, and any suggestion to the contrary is just plain 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] Last gasp
2012/4/5 Heikki Linnakangas heikki.linnakan...@enterprisedb.com: On 05.04.2012 21:00, Robert Haas wrote: Heikki recently produced a revision of the check function patch, but I'm not sure whether he's planning to commit that or whether it's a demonstration of a broader rework he wants Pavel (or someone else) to do. At any rate, I think it's up to him whether or not to commit that. Yeah, I guess it's up to me, now that I've touched that code. I don't feel ready to commit it yet. It needs some more cleanup, which I could do, but frankly even with the refactoring I did I'm still not totally happy with the way it works. I feel that there ought to be a less duplicative approach, but I don't have any more concrete proposals. Unless someone more motivated picks up the patch now, I think it needs to be pushed to 9.3. I played with more general plpgpsm walker, but I am sure, so it is wrong way - request for checking and dumping, and releasing plans are too different. So there are not too much possibilities :( The main issue is in design of exec nodes. I have no idea how to move checking to there without increasing complexity in pl_exec.c. Some are relative simply, but case, if, block are not. Other idea is moving plpgsql_check_function to contrib. Regards Pavel -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com t -- 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] Last gasp
On Thu, Apr 5, 2012 at 2:23 PM, Tom Lane t...@sss.pgh.pa.us wrote: In fact, for *most* of these patches the fact that they're still here should give you pause about just committing them. Um, yeah. I am not sure what the state of the foreign stats patch is, or FK arrays. I'm taking the foreign stats one; as I mentioned a bit ago, I think we need to push an FDW ANALYZE hook into 9.2. I don't like the details of what's in the current submission but I think it's fixable with not much effort. Cool. The FK arrays one I'm kind of queasy about. It's a cool-sounding idea but I'm not convinced that all the corner-case details have been adequately thought through, and I'm scared of being unable to fix any such bugs in later versions because of backwards compatibility worries. It'd be a lot better to be pushing this in at the start of a devel cycle than the end. I've been feeling that that patch has been suffering from a lack of reviewer attention, which is a real shame, because I think the functionality is indeed really cool. But I haven't looked at it enough to know what kind of shape it's in. Most of the rest of this stuff I'm about ready to put off to 9.3. The risk/reward ratio for committing it on the last day of the last 9.2 fest doesn't look good. +1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Last gasp
Excerpts from Simon Riggs's message of jue abr 05 14:28:54 -0300 2012: These patches aren't marked with a committer FK arrays ECPG fetch foreign stats command triggers check function parallel pg_dump Does that mean myself or others should be claiming them for commit/reject? The FK locking patch isn't on this list; however, I'm hijacking this thread to say that some benchmarking runs we tried weren't all that great, showing 9% performance degradation on stock pgbench -- i.e. a large hit that will harm everybody even if they are not using FKs at all. I'm thus setting the patch returned with feedback, which is sure to make several hackers happy and tons of users unhappy. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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] Last gasp
On Thu, Apr 5, 2012 at 7:23 PM, Tom Lane t...@sss.pgh.pa.us wrote: The FK arrays one I'm kind of queasy about. It's a cool-sounding idea but I'm not convinced that all the corner-case details have been adequately thought through, and I'm scared of being unable to fix any such bugs in later versions because of backwards compatibility worries. It'd be a lot better to be pushing this in at the start of a devel cycle than the end. OK, that's clear. I would have taken it, but not now. -- Simon Riggs 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] Last gasp
On Thu, Apr 5, 2012 at 2:37 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: The FK locking patch isn't on this list; however, I'm hijacking this thread to say that some benchmarking runs we tried weren't all that great, showing 9% performance degradation on stock pgbench -- i.e. a large hit that will harm everybody even if they are not using FKs at all. I'm thus setting the patch returned with feedback, which is sure to make several hackers happy and tons of users unhappy. Ouch! That's a real bummer. It makes me glad that you tested it, but I can't say I'm happy about the outcome. Did you get in any insight into where the regression is coming from? -- 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] Last gasp
On Thu, Apr 5, 2012 at 7:37 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: The FK locking patch isn't on this list; We'd agreed you were the assigned committer, hence not on list. however, I'm hijacking this thread to say that some benchmarking runs we tried weren't all that great, showing 9% performance degradation on stock pgbench -- i.e. a large hit that will harm everybody even if they are not using FKs at all. I'm thus setting the patch returned with feedback, which is sure to make several hackers happy and tons of users unhappy. I've done what I can to alter that, but I think its the right decision at this point. I would say its been the largest and most subtle patch submitted, so please don't be down by that. -- Simon Riggs 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] Last gasp
On Thu, 2012-04-05 at 14:27 -0400, Robert Haas wrote: On Thu, Apr 5, 2012 at 2:17 PM, Simon Riggs si...@2ndquadrant.com wrote: On Thu, Apr 5, 2012 at 7:00 PM, Robert Haas robertmh...@gmail.com wrote: On Thu, Apr 5, 2012 at 1:28 PM, Simon Riggs si...@2ndquadrant.com wrote: These patches aren't marked with a committer FK arrays ECPG fetch foreign stats command triggers check function parallel pg_dump Does that mean myself or others should be claiming them for commit/reject? I've been right in the middle of the command triggers stuff, so I suppose I would pick that up for commit if it were ready, but there hasn't been a new version this week unless I've missed it, and even if the new version arrived right this minute, I don't think I or anyone else can do a good job committing a patch of that size in the time remaining. So I think it's time to formally put that one out of its misery. I think doing so will cause substantial misery for many users. I find it hard to believe that such a simple concept hasn't managed to produce some workable subset after months of work. I am not interested in relitigating on this thread what has already been extensively discussed nearby. Dimitri and I agreed on numerous changes to try to make the behavior sane, To me it looked like the scope of the patch started to suddenly expand exponentially a few days ago from a simple COMMAND TRIGGERS, which would have finally enabled trigger-based or logical replication systems to do full replication to something recursive which would attempt to cover all weird combinations of commands triggering other commands for which there is no real use-case in view, except a suggestion don't do it :) The latest patch (v18) seemed quite ok for its original intended purpose. and those changes were suggested and agreed to for good reason. We didn't agree on every point, of course, but we did agree on most of it, and there is no patch that implements what was agreed. Even if there were, there is not time to review and commit a heavily revised version of a 1000 line patch before tomorrow, and any suggestion to the contrary is just plain 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] foreign key locks, 2nd attempt
On 25 March 2012 09:17, Simon Riggs si...@2ndquadrant.com wrote: The main thing we're waiting on are the performance tests to confirm the lack of regression. I have extensively benchmarked the latest revision of the patch (tpc-b.sql), which I pulled from Alvaro's github account. The benchmark was of the feature branch's then-and-current HEAD, Don't follow update chains unless caller requests it. I've had to split these numbers out into two separate reports. Incidentally, at some future point I hope that pgbench-tools can handling testing across feature branches, initdb'ing and suchlike automatically and as needed. That's something that's likely to happen sooner rather than later. The server used was kindly supplied by the University of Oregon open source lab. Server (It's virtualised, but apparently this is purely for sandboxing purposes and the virtualisation technology is rather good): IBM,8231-E2B POWER7 processor (8 cores). Fedora 16 8GB Ram Dedicated RAID1 disks. Exact configuration unknown. postgresql.conf (this information is available when you drill down into each test too, fwiw): max_connections = 200 shared_buffers = 2GB checkpoint_segments = 30 checkpoint_completion_target = 0.8 effective_cache_size = 6GB Reports: http://results_fklocks.staticloud.com/ http://results_master_for_fks.staticloud.com/ Executive summary: There is a clear regression of less than 10%. There also appears to be a new source of contention at higher client counts. I realise that the likely upshot of this, and other concerns that are generally held at this late stage is that this patch will not make it into 9.2 . For what it's worth, that comes as a big disappointment to me. I would like to thank both Alvaro and Noah for their hard work here. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Last gasp
On Thu, Apr 05, 2012 at 02:23:03PM -0400, Tom Lane wrote: I think the ECPG fetch patch is about ready to go. Normally Michael Meskes handles all ECPG patches, but I'm not sure what his schedule is like. I'm not sure what the politics are of someone else touching that code. I think we should leave that one for Michael. Frankly, none of the rest of us pay enough attention to ecpg to be candidates to take responsibility for nontrivial patches there. I will take care of this over the next couple days. Is the patch that Zoltan send last Friday the latest version? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at googlemail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] Last gasp
On Thu, 2012-04-05 at 20:46 +0200, Hannu Krosing wrote: On Thu, 2012-04-05 at 14:27 -0400, Robert Haas wrote: On Thu, Apr 5, 2012 at 2:17 PM, Simon Riggs si...@2ndquadrant.com wrote: ... I think doing so will cause substantial misery for many users. I find it hard to believe that such a simple concept hasn't managed to produce some workable subset after months of work. I am not interested in relitigating on this thread what has already been extensively discussed nearby. Dimitri and I agreed on numerous changes to try to make the behavior sane, To me it looked like the scope of the patch started to suddenly expand exponentially a few days ago from a simple COMMAND TRIGGERS, which would have finally enabled trigger-based or logical replication systems to do full replication to something recursive which would attempt to cover all weird combinations of commands triggering other commands for which there is no real use-case in view, except a suggestion don't do it :) The latest patch (v18) seemed quite ok for its original intended purpose. Sorry, i hit send! too early. Would it be possible to put some command trigger hooks in a few strategic places so that some trigger-like functionality could be loaded at run time, mainly with a view of writing DDL replication 'non-triggers' , mostly based on current v18 code, but of course without all the nice CREATE TRIGGER syntax ? perhaps created with a pg_create_command_trigger(...) that is something in the line of how Full Text Indexing was done for a long time. and those changes were suggested and agreed to for good reason. We didn't agree on every point, of course, but we did agree on most of it, and there is no patch that implements what was agreed. Even if there were, there is not time to review and commit a heavily revised version of a 1000 line patch before tomorrow, and any suggestion to the contrary is just plain 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] Last gasp
On 04/05/2012 02:00 PM, Robert Haas wrote: I think Andrew is working on parallel pg_dump, so I suspect the rest of us should stay out of the way. I've also looked at it extensively and will work on it if Andrew isn't, but I don't think it's ready to commit. A few preliminary pieces could go in, perhaps. I am working on it when I get time, but I am slammed. It would probably take me a couple of full days to do a thorough code review, and finding that is hard. 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] Last gasp
On Thu, Apr 5, 2012 at 2:46 PM, Hannu Krosing ha...@krosing.net wrote: To me it looked like the scope of the patch started to suddenly expand exponentially a few days ago from a simple COMMAND TRIGGERS, which would have finally enabled trigger-based or logical replication systems to do full replication to something recursive which would attempt to cover all weird combinations of commands triggering other commands for which there is no real use-case in view, except a suggestion don't do it :) The latest patch (v18) seemed quite ok for its original intended purpose. OK, so here we go, rehashing the discussion we already had on thread A on thread B. The particular issue you are mentioning there was not the reason that patch isn't going to end up in 9.2. If the only thing the patch had needed was a little renaming and syntax cleanup, I would have done it myself (or Dimitri would have) and I would have committed it. That is not the problem, or at least it's not the only problem. There are at least two other major issues: - The patch as posted fires triggers at unpredictable times depending on which command you're executing. Some things that are really sub-commands fire triggers anyway as if they were toplevel commands; others don't; whether or not it happens in a particular case is determined by implementation details rather than by any consistent principle of operation. In the cases where triggers do fire, they don't always fire at the same place in the execution sequence. - The patch isn't safe if the triggers try to execute DDL on the object being modified. It's not exactly clear what misbehavior will result in every case, but it is clear that that it hasn't really been thought about. Now, if anyone who was actually following the conversation thought these things were not problems, they could have written back to the relevant thread and said, hey, I don't mind if the trigger firing behavior changes every time someone does any refactoring of our badly-written DDL code and if the server blows up in random ways when someone does something unexpected in the trigger that's OK with me too. Maybe not surprisingly, no one said that. Two people wrote into that thread after my latest round of reviewing and both of them disagreed with only minor points of my review, and we had a technical discussion about those issues. But showing up after the fact and acting as if there were no serious issues found during that review is either disingenuous or a sign that you didn't really read the thread. -- 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] Last gasp
On Thu, Apr 5, 2012 at 2:58 PM, Hannu Krosing ha...@krosing.net wrote: On Thu, 2012-04-05 at 20:46 +0200, Hannu Krosing wrote: On Thu, 2012-04-05 at 14:27 -0400, Robert Haas wrote: On Thu, Apr 5, 2012 at 2:17 PM, Simon Riggs si...@2ndquadrant.com wrote: ... I think doing so will cause substantial misery for many users. I find it hard to believe that such a simple concept hasn't managed to produce some workable subset after months of work. I am not interested in relitigating on this thread what has already been extensively discussed nearby. Dimitri and I agreed on numerous changes to try to make the behavior sane, To me it looked like the scope of the patch started to suddenly expand exponentially a few days ago from a simple COMMAND TRIGGERS, which would have finally enabled trigger-based or logical replication systems to do full replication to something recursive which would attempt to cover all weird combinations of commands triggering other commands for which there is no real use-case in view, except a suggestion don't do it :) The latest patch (v18) seemed quite ok for its original intended purpose. Sorry, i hit send! too early. Would it be possible to put some command trigger hooks in a few strategic places so that some trigger-like functionality could be loaded at run time, mainly with a view of writing DDL replication 'non-triggers' , mostly based on current v18 code, but of course without all the nice CREATE TRIGGER syntax ? I certainly think that would be a possible way forward, but I don't think we should try to engineer that in the next 24 hours. Had the original goals of the patch been somewhat more modest, I think we could have gotten it into 9.2, but there's no time to rethink the scope of the patch now. With all respect for Dimitri and his *very* hard work on this subject, submitting a brand new major feature to the last CommitFest is not really a great way to get it committed, especially given that we didn't have consensus on the design before he started coding. There is every reason to think that we can get this feature into 9.3 with some more work, but it's not ready yet, and wishing won't make it so. -- 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] Last gasp
Excerpts from Robert Haas's message of jue abr 05 15:40:17 -0300 2012: On Thu, Apr 5, 2012 at 2:37 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: The FK locking patch isn't on this list; however, I'm hijacking this thread to say that some benchmarking runs we tried weren't all that great, showing 9% performance degradation on stock pgbench -- i.e. a large hit that will harm everybody even if they are not using FKs at all. I'm thus setting the patch returned with feedback, which is sure to make several hackers happy and tons of users unhappy. Ouch! That's a real bummer. It makes me glad that you tested it, but I can't say I'm happy about the outcome. Did you get in any insight into where the regression is coming from? Not really -- after reaching that conclusion I dropped immediate work on the patch to do other stuff (like checking whether there's any other patch I can help with in commitfest). I will resume work later, for a (hopefully early) 9.3 submission. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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] Last gasp
On Thu, 2012-04-05 at 15:02 -0400, Robert Haas wrote: On Thu, Apr 5, 2012 at 2:46 PM, Hannu Krosing ha...@krosing.net wrote: To me it looked like the scope of the patch started to suddenly expand exponentially a few days ago from a simple COMMAND TRIGGERS, which would have finally enabled trigger-based or logical replication systems to do full replication to something recursive which would attempt to cover all weird combinations of commands triggering other commands for which there is no real use-case in view, except a suggestion don't do it :) The latest patch (v18) seemed quite ok for its original intended purpose. OK, so here we go, rehashing the discussion we already had on thread A on thread B. The particular issue you are mentioning there was not the reason that patch isn't going to end up in 9.2. If the only thing the patch had needed was a little renaming and syntax cleanup, I would have done it myself (or Dimitri would have) and I would have committed it. That is not the problem, or at least it's not the only problem. There are at least two other major issues: - The patch as posted fires triggers at unpredictable times depending on which command you're executing. Some things that are really sub-commands fire triggers anyway as if they were toplevel commands; others don't; whether or not it happens in a particular case is determined by implementation details rather than by any consistent principle of operation. In the cases where triggers do fire, they don't always fire at the same place in the execution sequence. For me it would be enough to know if the trigger is fired by top-level command or not. In worst case I could probably detect it myself, just give me something to hang the detection code to . - The patch isn't safe if the triggers try to execute DDL on the object being modified. It's not exactly clear what misbehavior will result in every case, but it is clear that that it hasn't really been thought about. It never seemed important for me, as the only thing I was ever expecting to do in a command trigger was inserting rows in one unrelated table. To me DDL-triggered-by-DDL seemed like a very bad idea anyway. But as you seemed to be envisioning some use-cases for that I did not object to you working it out. Now, if anyone who was actually following the conversation thought these things were not problems, they could have written back to the relevant thread and said, hey, I don't mind if the trigger firing behavior changes every time someone does any refactoring of our badly-written DDL code and if the server blows up in random ways when someone does something unexpected in the trigger that's OK with me too. I don't mind if the trigger firing behavior changes every time someone does any refactoring of our badly-written DDL code Here :) Maybe not surprisingly, no one said that. Two people wrote into that thread after my latest round of reviewing and both of them disagreed with only minor points of my review, and we had a technical discussion about those issues. But showing up after the fact and acting as if there were no serious issues found during that review is either disingenuous or a sign that you didn't really read the thread. As there are other ways to blow up the backend, i did not object to you either working out a better solution or leaving it as it is. I am speaking up now as this is the first time I am told I have to wait another year for this feature to arrive. -- 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] Refactoring simplify_function (was: Caching constant stable expressions)
On Fri, Mar 23, 2012 at 7:41 PM, Marti Raudsepp ma...@juffo.org wrote: Yeah, I'm still working on addressing the comments from your last email. Haven't had much time to work on it for the last 2 weeks, but I hope to finish most of it this weekend. Since the updated patch seems never to have been posted, I think this one is dead for 9.2. I'll mark it Returned with Feedback. -- 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] Last gasp
On Thu, 2012-04-05 at 15:07 -0400, Robert Haas wrote: On Thu, Apr 5, 2012 at 2:58 PM, Hannu Krosing ha...@krosing.net wrote: Would it be possible to put some command trigger hooks in a few strategic places so that some trigger-like functionality could be loaded at run time, mainly with a view of writing DDL replication 'non-triggers' , mostly based on current v18 code, but of course without all the nice CREATE TRIGGER syntax ? I certainly think that would be a possible way forward, but I don't think we should try to engineer that in the next 24 hours. Why not ? If they are in wrong places, they just wont work, when they are in right places, we ca get (kind of) command triggers. We have had hooks for DTrace for some time and nobody has complained . If somebody misuses the command trigger hooks, it is his fault not core developers Had the original goals of the patch been somewhat more modest, I think we could have gotten it into 9.2, but there's no time to rethink the scope of the patch now. With all respect for Dimitri and his *very* hard work on this subject, submitting a brand new major feature to the last CommitFest is not really a great way to get it committed, especially given that we didn't have consensus on the design before he started coding. I guess it grew into a mayor feature because the few earlier attempts at DDL triggers (or begin/commit/abort triggers etc.) were shot down because they did not address the other issues of the bigger scope ;) There is every reason to think that we can get this feature into 9.3 with some more work, but it's not ready yet, and wishing won't make it so. Sad but true. -- 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] Last gasp
On Thu, Apr 5, 2012 at 3:20 PM, Hannu Krosing ha...@krosing.net wrote: For me it would be enough to know if the trigger is fired by top-level command or not. Well, you would have been out of luck, then. - The patch isn't safe if the triggers try to execute DDL on the object being modified. It's not exactly clear what misbehavior will result in every case, but it is clear that that it hasn't really been thought about. It never seemed important for me, as the only thing I was ever expecting to do in a command trigger was inserting rows in one unrelated table. To me DDL-triggered-by-DDL seemed like a very bad idea anyway. But as you seemed to be envisioning some use-cases for that I did not object to you working it out. Whether or not it works is one thing. I think it's acceptable for it to not do anything useful, although actually I think that given a week to work on it I could make it to completely safe. I don't think it's acceptable for it to, say, corrupt the system catalog contents. Now, if anyone who was actually following the conversation thought these things were not problems, they could have written back to the relevant thread and said, hey, I don't mind if the trigger firing behavior changes every time someone does any refactoring of our badly-written DDL code and if the server blows up in random ways when someone does something unexpected in the trigger that's OK with me too. I don't mind if the trigger firing behavior changes every time someone does any refactoring of our badly-written DDL code Here :) Duly noted, but color me unconvinced. Maybe not surprisingly, no one said that. Two people wrote into that thread after my latest round of reviewing and both of them disagreed with only minor points of my review, and we had a technical discussion about those issues. But showing up after the fact and acting as if there were no serious issues found during that review is either disingenuous or a sign that you didn't really read the thread. As there are other ways to blow up the backend, i did not object to you either working out a better solution or leaving it as it is. I am speaking up now as this is the first time I am told I have to wait another year for this feature to arrive. Really? You've never seen a patch punted to the next release before because it wasn't robust enough? Considering that I see your name mentioned in the 8.2 release notes, I find that a bit hard to believe. -- 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] Last gasp
On Thu, 2012-04-05 at 15:30 -0400, Robert Haas wrote: On Thu, Apr 5, 2012 at 3:20 PM, Hannu Krosing ha...@krosing.net wrote: For me it would be enough to know if the trigger is fired by top-level command or not. Well, you would have been out of luck, then. It seemed to me you were coming along nicely with fixing that by adding the toplevel, except that the disussion drifted away to solving some bigger problems of DDL triggering DDL. - The patch isn't safe if the triggers try to execute DDL on the object being modified. It's not exactly clear what misbehavior will result in every case, but it is clear that that it hasn't really been thought about. It never seemed important for me, as the only thing I was ever expecting to do in a command trigger was inserting rows in one unrelated table. To me DDL-triggered-by-DDL seemed like a very bad idea anyway. But as you seemed to be envisioning some use-cases for that I did not object to you working it out. Whether or not it works is one thing. I think it's acceptable for it to not do anything useful, although actually I think that given a week to work on it I could make it to completely safe. Due to fractal nature of programming problems I doubt the completely safe part ;) You (or someone else) would have found a next obscure set of conditions where specially crafted function and/or schema could wreak havoc somewhere. I don't think it's acceptable for it to, say, corrupt the system catalog contents. Sure, but any C function can do that already, and we have had C UDF-s from the start. Now, if anyone who was actually following the conversation thought these things were not problems, they could have written back to the relevant thread and said, hey, I don't mind if the trigger firing behavior changes every time someone does any refactoring of our badly-written DDL code and if the server blows up in random ways when someone does something unexpected in the trigger that's OK with me too. I don't mind if the trigger firing behavior changes every time someone does any refactoring of our badly-written DDL code Here :) Duly noted, but color me unconvinced. I can say it again :) Maybe not surprisingly, no one said that. Two people wrote into that thread after my latest round of reviewing and both of them disagreed with only minor points of my review, and we had a technical discussion about those issues. But showing up after the fact and acting as if there were no serious issues found during that review is either disingenuous or a sign that you didn't really read the thread. As there are other ways to blow up the backend, i did not object to you either working out a better solution or leaving it as it is. I am speaking up now as this is the first time I am told I have to wait another year for this feature to arrive. Really? You've never seen a patch punted to the next release before because it wasn't robust enough? Considering that I see your name mentioned in the 8.2 release notes, I find that a bit hard to believe. I must admit I did check it only occasionally and to me it seemed to come along nicely. I really should have started panicking earlier. -- 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] Last gasp
On Thu, Apr 5, 2012 at 8:30 PM, Robert Haas robertmh...@gmail.com wrote: I am speaking up now as this is the first time I am told I have to wait another year for this feature to arrive. Really? You've never seen a patch punted to the next release before because it wasn't robust enough? Considering that I see your name mentioned in the 8.2 release notes, I find that a bit hard to believe. Hannu didn't say he hadn't seen a patch punted. He said this was the first time he knew he would have to wait another year for the feature to arrive. It's shocking since after months of work and an especially extended edition CF, we expect people to deliver something, not just shunt the whole thing off as rejected with 1 days's notice to alter that outcome. Of course nobody can alter the outcome with such short notice. I don't care to hear the reasons, cos such excuses aren't good enough, whoever they come from. -- Simon Riggs 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] Last gasp
On 4/5/12 12:56 PM, Hannu Krosing wrote: Really? You've never seen a patch punted to the next release before because it wasn't robust enough? Considering that I see your name mentioned in the 8.2 release notes, I find that a bit hard to believe. I must admit I did check it only occasionally and to me it seemed to come along nicely. I have to agree ... for anyone just following the email thread, the patch did not seem to be in trouble. I'm not clear that Dimitri knew that it was, either. -- 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] Last gasp
On Thu, Apr 5, 2012 at 3:56 PM, Hannu Krosing ha...@krosing.net wrote: On Thu, 2012-04-05 at 15:30 -0400, Robert Haas wrote: On Thu, Apr 5, 2012 at 3:20 PM, Hannu Krosing ha...@krosing.net wrote: For me it would be enough to know if the trigger is fired by top-level command or not. Well, you would have been out of luck, then. It seemed to me you were coming along nicely with fixing that by adding the toplevel, except that the disussion drifted away to solving some bigger problems of DDL triggering DDL. Well, here's the basic issue. If you want to trigger at the very beginning or very end of a DDL command, that's really not that hard. It can be done just by modifying ProcessUtility. And just to be clear, by not very hard what I really mean is you could get it done in 2 CommitFests if you are a talented and experienced hacker. The problem is that what Dimitri really wants, at least in part, is command triggers that fire in the *middle* of commands - say, just after locking the object we're performing surgery on. That turns out to be a lot harder than it looks for reasons that are NOT Dimitri's fault. We tend to assume that PostgreSQL's code is basically good code, and in general that's true, but some of the code that is at issue here is actually really bad code. It uses ugly, nasty hacks to get the different things that have to happen as part of a DDL operation to happen in the right order, and it's not very careful about doing locking, name lookups, and permissions checking in one and only one place. This means that if you stick a hook in the right place to fire triggers for command A, you actually end up firing the triggers as part of commands B, C, and D when they reach under the hood and trick the function that runs command A into thinking that it's executing the toplevel command A is supposed to process when, in reality, we're doing something completely different and just using that function as a subroutine. It is probably not a tremendous amount of work to refactor ourselves out of this problem, but that work hasn't been done yet. There's a second problem, too, which is that not every place in the backend is a safe place to execute arbitrary user-defined code. Now, the approach you are proposing to that problem is to say, hey, if it crashes the backend, then don't do it that way. And obviously, with functions written in C, we have no choice about that; native code can crash the backend whether we like it or not. However, I think that we rightly have a higher standard for features that are exposed at the SQL level. It is likely true that if you or I or Dimitri were using this feature, we could tiptoe our way around doing anything that was dangerous, and we probably wouldn't be very confused by the strange trigger-firing behavior either, because we understand how the code is written. I don't think our users will be so forgiving. They right expect that the behavior will be logical and documented, that it won't eat their data or crash without a good reason, and that when things do go wrong they'll get an intelligible error message. The current approach doesn't guarantee any of those things. Whether or not it works is one thing. I think it's acceptable for it to not do anything useful, although actually I think that given a week to work on it I could make it to completely safe. Due to fractal nature of programming problems I doubt the completely safe part ;) You (or someone else) would have found a next obscure set of conditions where specially crafted function and/or schema could wreak havoc somewhere. Well, it's not possible to write completely perfect code for anything other than a completely trivial program, and I think there is certainly a time to say, hey, you know, this patch may not be perfect in every way, but it is good enough that we should start with this as a base, and then build on it. But I think that forseeable catalog or data corruption hazards and anything that might pop out an elog() are generally accepted as things that we want to avoid even in initial commits. I really should have started panicking earlier. Yep. I think Tom and I and others were all pretty clear that there were not enough people reviewing this CommitFest, and that we were spread pretty thin trying to make sure every patch got at least some attention. There were many patches that I could have done more with if I hadn't had 30 of them to do something with, and this is one of them. I could have looked at it sooner; I could have spent time revising it. Your position is apparently that since I noted problems with the patch, it was my job to fix it. I don't know how to square that with the seemingly-universal assumption that every patch is entitled to get looked at by a committer. I cannot both look at every patch and fix every problem I find with each one. If the lesson we take from this CommitFest is that the people who put the most time into making it successful are
Re: [HACKERS] Last gasp
On Thu, Apr 5, 2012 at 4:38 PM, Josh Berkus j...@agliodbs.com wrote: On 4/5/12 12:56 PM, Hannu Krosing wrote: Really? You've never seen a patch punted to the next release before because it wasn't robust enough? Considering that I see your name mentioned in the 8.2 release notes, I find that a bit hard to believe. I must admit I did check it only occasionally and to me it seemed to come along nicely. I have to agree ... for anyone just following the email thread, the patch did not seem to be in trouble. I'm not clear that Dimitri knew that it was, either. I said repeatedly that there wasn't enough time to get this done for 9.2, starting many weeks ago. It would have been more clear if I'd had time to hone in on the specific problems sooner, but see the other note I just sent for why that didn't happen. -- 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] Last gasp
On Thu, Apr 5, 2012 at 1:40 PM, Robert Haas robertmh...@gmail.com wrote: Yep. I think Tom and I and others were all pretty clear that there were not enough people reviewing this CommitFest, Sorry to derail the thread just a little, but I've thought a little about this and I wonder if part of the problem is just that some of the patches are really large: doing a really thorough job reviewing is quite demanding, and the amount and kind of lore everyone has at their disposal is different. To get to the point, I wonder if it makes sense for someone who has a better sense a-priori what they're looking for in a committable patch (i.e. a committer, or someone with a telepathic link to one or more) to delegate specific pieces of patches for thorough review, sketching some thoughts about pitfalls or hazards to look for. Think of it as a patch checklist that is informed by the experience of a committer skimming its contents. -- fdr -- 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] Last gasp
Robert Haas robertmh...@gmail.com writes: I am not interested in relitigating on this thread what has already been extensively discussed nearby. Dimitri and I agreed on numerous changes to try to make the behavior sane, and those changes were suggested and agreed to for good reason. We didn't agree on every Those changes are about done now. I didn't have as much time as I would have wanted to spend on that this week, so if you want to see my command trigger patch re-painted into an event trigger system, please go see the github repository: https://github.com/dimitri/postgres/tree/event_triggers https://github.com/dimitri/postgres/compare/command_triggers...event_triggers https://github.com/dimitri/postgres/compare/master...event_triggers dim=# create event trigger snitch before command_start execute procedure snitch(); create event trigger snitch before command_start execute procedure snitch(); CREATE EVENT TRIGGER dim=# create event trigger another_snitch before command_start when tag in (create table, create view) execute procedure snitch(); create event trigger another_snitch before command_start when tag in (create table, create view) execute procedure snitch(); CREATE EVENT TRIGGER point, of course, but we did agree on most of it, and there is no patch that implements what was agreed. Even if there were, there is not time to review and commit a heavily revised version of a 1000 line patch before tomorrow, and any suggestion to the contrary is just plain wrong. The main problems were, as you say it in another mail, the hook placement and the ddl-in-ddl. The hook placement is being solved thanks to naming several events where the trigger are fired. We could have done that in a much simpler way by deciding on when a BEFORE trigger really ought to be called, instead of doing that we're generalizing the system so that you can register your user function before events such as command_start, or create_command or security_check. Coming next, calling user functions *instead of* those steps (not done in the 9.2 version). The ideas about toplevel commands and calling user functions on CASCADEd objects drops is now possible to implement but not yet in the patch, it's simple enough to do. Calling DDL from a DDL trigger in a way that could change the conditions in which the main command is expecting to run (drop the object being altered in the trigger) is certainly a shoot yourself in the foot case and the feature is superuser only: we're talking about a doc patch here. My conclusion here is very simple: if we *want* that feature, we can easily enough have it in 9.2 in some form. That will include the possibility to code a C-extension to implementing command rewriting for use in our 3 major trigger based replication systems, and custom applications too. It's been about a year since we started talking about that topic, and it only changed from being command triggers to being event triggers last week during review, at version 18 of the patch, each previous version having been both public and reviewed and tested. So, as a community, do we want the feature and are we able to cook something up for 9.2? 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] Last gasp
On Thu, Apr 5, 2012 at 4:27 PM, Simon Riggs si...@2ndquadrant.com wrote: On Thu, Apr 5, 2012 at 8:30 PM, Robert Haas robertmh...@gmail.com wrote: I am speaking up now as this is the first time I am told I have to wait another year for this feature to arrive. Really? You've never seen a patch punted to the next release before because it wasn't robust enough? Considering that I see your name mentioned in the 8.2 release notes, I find that a bit hard to believe. Hannu didn't say he hadn't seen a patch punted. He said this was the first time he knew he would have to wait another year for the feature to arrive. Oh. I misunderstood what he was saying, then. When ANY patch is still pending after a month of CommitFest, everyone should assume it's in trouble. Triply so if it's a big patch. We are almost three months into this one, and the patch in question is very large. -- 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] Question regarding SSL code in backend and frontend
On Thu, Apr 5, 2012 at 08:00, Tatsuo Ishii is...@postgresql.org wrote: Those code fragment judges the return value from SSL_read(). secure_read() does retrying when SSL_ERROR_WANT_READ *and* SSL_ERROR_WANT_WRITE returned. However, pqsecure_read() does not retry when SSL_ERROR_WANT_READ. It seems they are not consistent. Comments? There's no particular reason why they should be consistent, I think. The assumptions for nonblocking operation are different. Ok. Thanks. BTW, usage of SSL_CTX_new() is different among frontend and backend as well. fe-secure.c: SSL_context = SSL_CTX_new(TLSv1_method()); be-secure.c: SSL_context = SSL_CTX_new(SSLv23_method()); In my understanding by using SSLV23_method, it is compatible with SSLv2, SSLv3, and TLSv1 protocol. So it seems there's no particular reason to use TLSv1_method(). Am I missing something? The reason is that TLS is more secure, I believe. It was changed in commit 750a0e676e1f8f71bf1c6aba05d3264a7c57218b for backwards compatibility. If anything, we should be changing it to TLSv1 in both client and server, since every client out there now should be using that anyway, given that the client has been specifying it for a long time. Unless I am also missing something? :-) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Last gasp
On Thu, Apr 5, 2012 at 5:04 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Those changes are about done now. Huh? AFAICS, there are vestiges of the old terminology all over the patch, although some very incomplete renaming has been done. I'm pretty sure you haven't made the firing points consistent either, although I'd have to spend more time reviewing to be sure of that. I think it's fair to say that there's probably another month of work here to get this committed. It is, of course, up to the community as a whole whether they want to delay 9.2 another month for this and perhaps other patches that are not yet done. But the idea that we're going to pull it together into committable form in the next 24 hours is not realistic. -- 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] Last gasp
Robert Haas robertmh...@gmail.com writes: Huh? AFAICS, there are vestiges of the old terminology all over the patch, although some very incomplete renaming has been done. I'm pretty sure you haven't made the firing points consistent either, although I'd have to spend more time reviewing to be sure of that. I've choosen to internally have command triggers built in an event trigger framework, because clearly the only kind of events we have an idea how to manage are commands here. Don't generalize an API with only one use case, etc. So internally it's very much command oriented, while the usage is all about events. The firing point didn't move yet, I finished the mechanism. It's now about picking the right event name for each of them, I've begin to explore about that in aggregatecmds.c already. So I think that's only a policy issue now when it was just impossible some days ago. I think it's fair to say that there's probably another month of work here to get this committed. It is, of course, up to the community as a whole whether they want to delay 9.2 another month for this and perhaps other patches that are not yet done. But the idea that we're going to pull it together into committable form in the next 24 hours is not realistic. All of that depends on what you want to see commit, I'm still under the illusion that parts of it are going to be salvaged. For example we could decide to implement command_start and commend_end in utility.c and remove all special variables support apart from the parse tree for user functions written in C. That would already be something. Of course as I wasn't running for that outcome that's not what you currently see in the branch. 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] Another review of URI for libpq, v7 submission
Excerpts from Peter Eisentraut's message of jue abr 05 13:52:13 -0300 2012: On ons, 2012-03-28 at 01:49 +0300, Alex wrote: Attached is a gzipped v10, complete with the above fixes and fixes to bugs found by Heikki. Documentation and code comments are also finally updated. The compiler warning is still there: fe-connect.c: In function ‘conninfo_parse’: fe-connect.c:4122:20: error: unused variable ‘option’ [-Werror=unused-variable] Here's an updated patch, including this fix and others. (Most notable the fact that I got rid of conninfo_store_uri_encoded_value, instead expanding conninfo_storeval a bit). I also fixed the test script to run in VPATH. I intend to commit this shortly, barring objection from Peter who is listed as committer in the CF app. I think the only thing I'm not really sure about is the usage of the synopsis tag to mark example URIs in the docs. It looks to me that they should mostly be literal instead, but I'm not really sure about that either -- I'm suspecting the PDF output would look rather horrible. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support libpq-uri-v11.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
[HACKERS] System catalog typos
Hi, I attach a patch to correct various system catalog/view definitions in the docs. Regards Thom catalog_doc_fixes.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Last gasp
On Fri, 2012-04-06 at 00:02 +0200, Dimitri Fontaine wrote: Robert Haas robertmh...@gmail.com writes: Huh? AFAICS, there are vestiges of the old terminology all over the patch, although some very incomplete renaming has been done. I'm pretty sure you haven't made the firing points consistent either, although I'd have to spend more time reviewing to be sure of that. I've choosen to internally have command triggers built in an event trigger framework, because clearly the only kind of events we have an idea how to manage are commands here. Don't generalize an API with only one use case, etc. So internally it's very much command oriented, while the usage is all about events. The firing point didn't move yet, I finished the mechanism. It's now about picking the right event name for each of them, I've begin to explore about that in aggregatecmds.c already. So I think that's only a policy issue now when it was just impossible some days ago. I think it's fair to say that there's probably another month of work here to get this committed. It is, of course, up to the community as a whole whether they want to delay 9.2 another month for this and perhaps other patches that are not yet done. But the idea that we're going to pull it together into committable form in the next 24 hours is not realistic. All of that depends on what you want to see commit, I'm still under the illusion that parts of it are going to be salvaged. I would really like the controversial parts of the recent feature creep to get un-creeped :) At least until 9.3 . For example we could decide to implement command_start and commend_end in utility.c and remove all special variables support apart from the parse tree for user functions written in C. That would already be something. I would need only one of those - just something to record the DDL command that was done. I'd prefer to get command_end triggers if command_start would mean it could fail later. CREATE TRIGGER AFTER ANY DDL EXECUTE record_ddl(); My next wish after the above is available would be triggers on BEGIN/ROLLBACK/COMMIT to implement streaming and possibly also synchronous logical replication - the current logical replication systems rely on trigger-gathered events only being available after commit and so don't need to get notified about COMMIT/ROLLBACK. CREATE TRIGGER AFTER BEGIN EXECUTE remote_transaction_begin_(); CREATE TRIGGER AFTER ROLLBACK EXECUTE remote_transaction_rollback(); CREATE TRIGGER AFTER COMMIT EXECUTE remote_transaction_commit(); Of course as I wasn't running for that outcome that's not what you currently see in the branch. I would very much like to see the stripped down, after command only version to get into 9.2 I would not mind at all waiting a week or two for this to happen. And i promise I will keep a very keen eye on further development and also will start testing right away for it being sufficient for DDL replication as soon as the stripping is completed :) Also, I suspect that the rest of the event trigger functionality could better be served as simple hooks anyway, at least until we have some good real life use-case to test against before working on a more involved implementation. 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] Question regarding SSL code in backend and frontend
Magnus Hagander mag...@hagander.net writes: If anything, we should be changing it to TLSv1 in both client and server, since every client out there now should be using that anyway, given that the client has been specifying it for a long time. Huh? libpq isn't every client. 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] pg_receivexlog stops upon server restart
Hi, I've tried out pg_receivexlog and have noticed that when restarting the cluster, pg_receivexlog gets cut off... it doesn't keep waiting. This is surprising as the DBA would have to remember to start pg_receivexlog up again. -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] parallel dump/restore code on WIndows
Why was this code: #ifdef WIN32 if (parallel_init_done GetCurrentThreadId() != mainThreadId) ExitThread(code); #endif removed from dumputils.c by commit 5e86c61a7eec0fdc6961493a150159fa8fc63b1c? The commit message doesn't refer to it at all. I don't understand it at all, and there is nothing in Joachim's latest patch that would call ExitThread() from an exit handler, so the effect seems likely to me to break parallel windows processing in nasty ways if called from a worker thread, although I haven't tested it. 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] Last gasp
On Thu, Apr 05, 2012 at 02:34:30PM -0400, Robert Haas wrote: On Thu, Apr 5, 2012 at 2:23 PM, Tom Lane t...@sss.pgh.pa.us wrote: The FK arrays one I'm kind of queasy about. ?It's a cool-sounding idea but I'm not convinced that all the corner-case details have been adequately thought through, and I'm scared of being unable to fix any such bugs in later versions because of backwards compatibility worries. It'd be a lot better to be pushing this in at the start of a devel cycle than the end. I've been feeling that that patch has been suffering from a lack of reviewer attention, which is a real shame, because I think the functionality is indeed really cool. But I haven't looked at it enough to know what kind of shape it's in. As the reviewer, I'm not aware of any unexplored corner cases or problems that ought to preclude commit. That said, it is a large patch; I doubt anyone could pick it up from scratch and commit it with less than a solid day's effort, and 2-3 days might be more likely. In retrospect, I should have suggested splitting the new ON DELETE/ON UPDATE actions into their own patch. That would have nicely slimmed the base patch and also isolated it from the ON DELETE EACH CASCADE judgement call. -- 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] System catalog typos
On 5 April 2012 23:22, Thom Brown t...@linux.com wrote: Hi, I attach a patch to correct various system catalog/view definitions in the docs. I also found a couple typos in completely different sections. (patch attached) -- Thom doc_typo_fixes.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] System catalog typos
On 6 April 2012 01:22, Thom Brown t...@linux.com wrote: On 5 April 2012 23:22, Thom Brown t...@linux.com wrote: Hi, I attach a patch to correct various system catalog/view definitions in the docs. I also found a couple typos in completely different sections. (patch attached) Apologies, that last patch had one correction in the wrong place. Reattached. -- Thom doc_typo_fixes.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] About the behavior of array_length() for empty array
postgres=# select '{}'::integer[] = array[[]]::integer[][];
?column?
--
t
postgres=# select '{}'::integer[] = array[[[]]]::integer[][][];
?column?
--
t
From this point of view, seems N dimensions of empty array all are equivalent.
But the result of array_length of them always are null.
postgres=# select array_length('{}'::integer[],0);
array_length
--
postgres=# select array_length(array[[[]]]::integer[][][],0);
array_length
--
postgres=# select array_length(array[[[]]]::integer[][][],3) is null;
?column?
--
t
I just think any empty array length should return 0, but not null. It's not
a NULL array.
Is there standard definition of this behavior?
--
--
iihero(Xiong He) http://www.sql9.com
--
Re: [HACKERS] parallel dump/restore code on WIndows
On Thu, Apr 5, 2012 at 8:12 PM, Andrew Dunstan and...@dunslane.net wrote: Why was this code: #ifdef WIN32 if (parallel_init_done GetCurrentThreadId() != mainThreadId) ExitThread(code); #endif removed from dumputils.c by commit 5e86c61a7eec0fdc6961493a150159fa8fc63b1c? The commit message doesn't refer to it at all. I don't understand it at all, and there is nothing in Joachim's latest patch that would call ExitThread() from an exit handler, so the effect seems likely to me to break parallel windows processing in nasty ways if called from a worker thread, although I haven't tested it. Ah, snap. That's a mistake on my part. Sorry. -- 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] measuring lwlock-related latency spikes
On Tue, Apr 3, 2012 at 8:28 AM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Might as well jump in with both feet: autovacuum_naptime = 1s autovacuum_vacuum_threshold = 1 autovacuum_vacuum_scale_factor = 0.0 If that smooths the latency peaks and doesn't hurt performance too much, it's decent evidence that the more refined technique could be a win. It seems this isn't good for either throughput or latency. Here are latency percentiles for a recent run against master with my usual settings: 90 1668 91 1747 92 1845 93 1953 94 2064 95 2176 96 2300 97 2461 98 2739 99 3542 100 12955473 And here's how it came out with these settings: 90 1818 91 1904 92 1998 93 2096 94 2200 95 2316 96 2459 97 2660 98 3032 99 3868 100 10842354 tps came out tps = 13658.330709 (including connections establishing), vs 14546.644712 on the other run. I have a (possibly incorrect) feeling that even with these ridiculously aggressive settings, nearly all of the cleanup work is getting done by HOT prunes rather than by vacuum, so we're still not testing what we really want to be testing, but we're doing a lot of extra work along the way. -- 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] WIP: Collecting statistics on CSV file data
Thanks, Hanada-san! Best regards, Etsuro Fujita -Original Message- From: Shigeru HANADA [mailto:shigeru.han...@gmail.com] Sent: Friday, April 06, 2012 11:41 AM To: Etsuro Fujita Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] WIP: Collecting statistics on CSV file data (2012/04/05 21:10), Shigeru HANADA wrote: file_fdw This patch contains a use case of new handler function in contrib/file_fdw. Since file_fdw reads data from a flat file, fileAnalyzeForeignTable uses similar algorithm to ordinary tables; it samples first N rows first, and replaces them randomly with subsequent rows. Also file_fdw updates pg_class.relpages by calculating number of pages from size of the data file. To allow FDWs to implement sampling argorighm like this, several functions are exported from analyze.c, e.g. random_fract, init_selection_state, and get_next_S. Just after my post, Fujita-san posted another v7 patch[1], so I merged v7 patches into v8 patch. [1] http://archives.postgresql.org/pgsql-hackers/2012-04/msg00212.php Changes taken from Fujita-san's patch = * Remove reporting validrows and deadrows at the end of acquire_sample_rows of file_fdw. Thus, it doesn't validate NOT NULL constraints any more. * Improve get_rel_size of file_fdw, which is used in GetForeignRelSize, to estimate current # of tuples of the foreign table from these values. - # of pages/tuples which are updated by last ANALYZE - current file size Additional Changes == * Fix memory leak in acquire_sample_rows which caused by calling NextCopyFrom repeatedly in one long-span memory context. I add per-record temporary context and it's used during processing a record. Main context is used to create heap tuples from sampled records, because sample tuples must be valid after the function ends. * Some cosmetic changes for document, e.g. remove line-break inside tagged elements. * Some cosmetic changes to make patch more readable by minimizing difference from master branch. Changes did *not* merged * Fujita-san moved document of AnalyzeForeignTable to the section Foreign Data Wrapper Helper Functions from Foreign Data Wrapper Callback Routines. But I think analyze handler is one of callback functions, though it's optional. Please find attached a patch. Regards, -- Shigeru HANADA -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
