Re: [HACKERS] Speed up Clog Access by increasing CLOG buffers
On Sat, Sep 17, 2016 at 11:25 PM, Tomas Vondra wrote: > On 09/17/2016 07:05 AM, Amit Kapila wrote: >> >> On Sat, Sep 17, 2016 at 9:17 AM, Tomas Vondra >> wrote: >>> >>> On 09/14/2016 05:29 PM, Robert Haas wrote: > > ... Sure, but you're testing at *really* high client counts here. Almost nobody is going to benefit from a 5% improvement at 256 clients. You need to test 64 clients and 32 clients and 16 clients and 8 clients and see what happens there. Those cases are a lot more likely than these stratospheric client counts. >>> >>> Right. My impression from the discussion so far is that the patches >>> only improve performance with very many concurrent clients - but as >>> Robert points out, almost no one is running with 256 active >>> clients, unless they have 128 cores or so. At least not if they >>> value latency more than throughput. >>> >> >> See, I am also not in favor of going with any of these patches, if >> they doesn't help in reduction of contention. However, I think it is >> important to understand, under what kind of workload and which >> environment it can show the benefit or regression whichever is >> applicable. > > > Sure. Which is why I initially asked what type of workload should I be > testing, and then done the testing with multiple savepoints as that's what > you suggested. But apparently that's not a workload that could benefit from > this patch, so I'm a bit confused. > >> Just FYI, couple of days back one of EDB's partner who was doing the >> performance tests by using HammerDB (which is again OLTP focussed >> workload) on 9.5 based code has found that CLogControlLock has the >> significantly high contention. They were using synchronous_commit=off >> in their settings. Now, it is quite possible that with improvements >> done in 9.6, the contention they are seeing will be eliminated, but >> we have yet to figure that out. I just shared this information to you >> with the intention that this seems to be a real problem and we should >> try to work on it unless we are able to convince ourselves that this >> is not a problem. >> > > So, can we approach the problem from this direction instead? That is, > instead of looking for workloads that might benefit from the patches, look > at real-world examples of CLOG lock contention and then evaluate the impact > on those? > Sure, we can go that way as well, but I thought instead of testing with a new benchmark kit (HammerDB), it is better to first get with some simple statements. > Extracting the workload from benchmarks probably is not ideal, but it's > still better than constructing the workload on our own to fit the patch. > > FWIW I'll do a simple pgbench test - first with synchronous_commit=on and > then with synchronous_commit=off. Probably the workloads we should have > started with anyway, I guess. > Here, synchronous_commit = off case could be interesting. Do you see any problem with first trying a workload where Dilip is seeing benefit? I am not suggesting we should not do any other testing, but just first lets try to reproduce the performance gain which is seen in Dilip's tests. -- With Regards, Amit Kapila. 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] Parallel sec scan in plpgsql
On Fri, Sep 16, 2016 at 8:48 PM, Alex Ignatov wrote: > > On 16.09.2016 16:50, Amit Kapila wrote: >> >> >> Can you try by setting force_parallel_mode = off;? I think it is >> sending the whole function execution to worker due to >> force_parallel_mode. >> >> > > No changes: > Okay, it just skipped from my mind that we don't support parallel queries for SQL statement execution (or statements executed via exec_stmt_execsql) from plpgsql. For detailed explanation of why that is not feasible you can refer one of my earlier e-mails [1] on similar topic. I think if we can somehow get the results via Perform statement, then it could be possible to use parallelism via plpgsql. However, you can use it via SQL functions, an example is below: set min_parallel_relation_size =0; set parallel_tuple_cost=0; set parallel_setup_cost=0; Load 'auto_explain'; set auto_explain.log_min_duration = 0; set auto_explain.log_analyze = true; set auto_explain.log_nested_statements = true; create table test_plpgsql(c1 int, c2 char(1000)); insert into test_plpgsql values(generate_series(1,10),'aaa'); create or replace function parallel_test_set_sql() returns setof bigint as $$ select count(*) from test_plpgsql; $$language sql PARALLEL SAFE STRICT STABLE; Then execute function as: select * from parallel_test_set_sql(); You can see below plan if auto_explain module is loaded. Finalize Aggregate (cost=14806.85..14806.86 rows=1 width=8) (actual tim e=1094.966..1094.967 rows=1 loops=1) -> Gather (cost=14806.83..14806.84 rows=2 width=8) (actual time=472. 216..1094.943 rows=3 loops=1) Workers Planned: 2 Workers Launched: 2 -> Partial Aggregate (cost=14806.83..14806.84 rows=1 width=8) (actual time=177.867..177.868 rows=1 loops=3) -> Parallel Seq Scan on test_plpgsql (cost=0.00..14702.6 7 rows=41667 width=0) (actual time=0.384..142.565 rows=3 loops=3) CONTEXT: SQL function "parallel_test_set_sql" statement 1 LOG: duration: 2965.040 ms plan: Query Text: select * from parallel_test_set_sql(); Function Scan on parallel_test_set_sql (cost=0.25..10.25 rows=1000 widt h=8) (actual time=2538.620..2776.955 rows=1 loops=1) In general, I think we should support the cases as required (or written) by you from plpgsql or sql functions. We need more work to support such cases. There are probably two ways of supporting such cases, we can build some intelligence in plpgsql execution such that it can recognise such queries and allow to use parallelism or we need to think of enabling parallelism for cases where we don't run the plan to completion. Most of the use cases from plpgsql or sql function fall into later category as they don't generally run the plan to completion. [1] - https://www.postgresql.org/message-id/CAA4eK1K8kaO_jRk42-o2rmhSRbKV-3mR%2BiNVcONLdbcSXW5TfQ%40mail.gmail.com -- With Regards, Amit Kapila. 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] WIP: About CMake v2
On 9/16/16 9:33 AM, Michael Paquier wrote: > I am no cmake guru yet, but VPATH builds are supported out of the box, > which is cool. > > Your patch recommends to build with cmake after creating build/, now I > would expect most users to run cmake from the root folder. My understanding is that cmake strongly recommends building in a separate directory, which is usually a subdirectory named something like "build". So the above is likely going to be the standard workflow. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] [bug fix] pg_recvlogical is missing in the Windows installation
On Sun, Sep 18, 2016 at 7:01 AM, MauMau wrote: > pg_recvlogical is not included in the Windows client installation, > which is performed by running "install client". The > attached patch based on HEAD fixes this. I confirmed nothing else is > missing in the client installation. Good cacth. This has been missed for a couple of years. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [bug fix] pg_recvlogical is missing in the Windows installation
Hello, pg_recvlogical is not included in the Windows client installation, which is performed by running "install client". The attached patch based on HEAD fixes this. I confirmed nothing else is missing in the client installation. Regards MauMau (= Takayuki Tsunakawa) missing_win_modules.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] Tuplesort merge pre-reading
On Sat, Sep 17, 2016 at 9:41 AM, Heikki Linnakangas wrote: > Ok. I'll probably read through it myself once more some time next week, and > also have a first look at your actual parallel sorting patch. Have a good > trip! Thanks! It will be good to get away for a while. I'd be delighted to recruit you to work on the parallel CREATE INDEX patch. I've already explained how I think this preread patch of yours works well with parallel tuplesort (as proposed) in particular. To reiterate: while what you've come up with here is technically an independent improvement to merging, it's much more valuable in the overall context of parallel sort, where disproportionate wall clock time is spent merging, and where multiple passes are the norm (one pass in each worker, plus one big final pass in the leader process alone -- logtape.c fragmentation becomes a real issue). The situation is actually similar to the original batch memory preloading patch that went into 9.6 (which your new patch supersedes), and the subsequently committed quicksort for external sort patch (which my new patch extends to work in parallel). Because I think of your preload patch as a part of the overall parallel CREATE INDEX effort, if that was the limit of your involvement then I'd still think it fair to credit you as my co-author. I hope it isn't the limit of your involvement, though, because it seems likely that the final result will be better still if you get involved with the big patch that formally introduces parallel CREATE INDEX. -- Peter Geoghegan -- 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 up Clog Access by increasing CLOG buffers
On 09/17/2016 07:05 AM, Amit Kapila wrote: On Sat, Sep 17, 2016 at 9:17 AM, Tomas Vondra wrote: On 09/14/2016 05:29 PM, Robert Haas wrote: ... Sure, but you're testing at *really* high client counts here. Almost nobody is going to benefit from a 5% improvement at 256 clients. You need to test 64 clients and 32 clients and 16 clients and 8 clients and see what happens there. Those cases are a lot more likely than these stratospheric client counts. Right. My impression from the discussion so far is that the patches only improve performance with very many concurrent clients - but as Robert points out, almost no one is running with 256 active clients, unless they have 128 cores or so. At least not if they value latency more than throughput. See, I am also not in favor of going with any of these patches, if they doesn't help in reduction of contention. However, I think it is important to understand, under what kind of workload and which environment it can show the benefit or regression whichever is applicable. Sure. Which is why I initially asked what type of workload should I be testing, and then done the testing with multiple savepoints as that's what you suggested. But apparently that's not a workload that could benefit from this patch, so I'm a bit confused. Just FYI, couple of days back one of EDB's partner who was doing the performance tests by using HammerDB (which is again OLTP focussed workload) on 9.5 based code has found that CLogControlLock has the significantly high contention. They were using synchronous_commit=off in their settings. Now, it is quite possible that with improvements done in 9.6, the contention they are seeing will be eliminated, but we have yet to figure that out. I just shared this information to you with the intention that this seems to be a real problem and we should try to work on it unless we are able to convince ourselves that this is not a problem. So, can we approach the problem from this direction instead? That is, instead of looking for workloads that might benefit from the patches, look at real-world examples of CLOG lock contention and then evaluate the impact on those? Extracting the workload from benchmarks probably is not ideal, but it's still better than constructing the workload on our own to fit the patch. FWIW I'll do a simple pgbench test - first with synchronous_commit=on and then with synchronous_commit=off. Probably the workloads we should have started with anyway, I guess. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, 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] WIP: About CMake v2
Andres Freund wrote: It's way too likely that we end up releasing with multiple buildsystems that way. If we fail in the CMake integration we can easily remove all CMake files before code freeze. I hope step by step CMake integration better way for current situation. But I do not insist, just I trying to find the easiest way. -- Yury Zhuravlev Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] Tuplesort merge pre-reading
On 09/17/2016 07:27 PM, Peter Geoghegan wrote: On Wed, Sep 14, 2016 at 10:43 AM, Heikki Linnakangas wrote: Addressed all your comments one way or another, new patch attached. So, it's clear that this isn't ready today. As I mentioned, I'm going away for a week now. I ask that you hold off on committing this until I return, and have a better opportunity to review the performance characteristics of this latest revision, for one thing. Ok. I'll probably read through it myself once more some time next week, and also have a first look at your actual parallel sorting patch. Have a good trip! - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Tuplesort merge pre-reading
On Wed, Sep 14, 2016 at 10:43 AM, Heikki Linnakangas wrote: > Addressed all your comments one way or another, new patch attached. So, it's clear that this isn't ready today. As I mentioned, I'm going away for a week now. I ask that you hold off on committing this until I return, and have a better opportunity to review the performance characteristics of this latest revision, for one thing. Thanks -- Peter Geoghegan -- 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] OpenSSL 1.1 breaks configure and more
On 09/16/2016 04:11 PM, Christoph Berg wrote: Thanks for the patch! I just tried to apply it to 9.2. There was a conflict in configure.in which was trivial to resolve. Another conflict in contrib/pgcrypto/pgcrypto.c was not applicable because the code doesn't seem to exist (didn't try very hard though). Ignoring the contrib conflict, it still didn't compile: /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c: In function ‘secure_write’: /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c:342:17: error: dereferencing pointer to incomplete type ‘SSL {aka struct ssl_st}’ if (port->ssl->state != SSL_ST_OK) ^~ /home/cbe/projects/postgresql/debian/9.2/build/../src/backend/libpq/be-secure.c:342:28: error: ‘SSL_ST_OK’ undeclared (first use in this function) if (port->ssl->state != SSL_ST_OK) ^ This is related to the renegotiation which was first fixed and later removed in the 9.4 cycle, but intentionally not backported. It seems like OpenSSL refactored the state machine in 1.1 which is why the code above breaks. I am not entirely sure I follow what the old code in 9.3 and 9.2 is strying to do and why it messes directly with the state of the statemachine. Andreas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers