Re: [HACKERS] Speed up Clog Access by increasing CLOG buffers

2016-09-17 Thread Amit Kapila 
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

2016-09-17 Thread Amit Kapila 
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

2016-09-17 Thread Peter Eisentraut 
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

2016-09-17 Thread Michael Paquier 
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

2016-09-17 Thread MauMau 
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

2016-09-17 Thread Peter Geoghegan 
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

2016-09-17 Thread Tomas Vondra 

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

2016-09-17 Thread Yury Zhuravlev 

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

2016-09-17 Thread Heikki Linnakangas 

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

2016-09-17 Thread Peter Geoghegan 
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

2016-09-17 Thread Andreas Karlsson 

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

Re: [HACKERS] Rename max_parallel_degree?

2016-09-17 Thread Amit Kapila 
On Fri, Sep 16, 2016 at 11:54 PM, Robert Haas  wrote:
>
> +   /*
> +* We need a write barrier to make sure the update of
> +* parallel_terminate_count is done before the store to in_use
> +*/
>
> Does the order actually matter here?
>

I think so.  If slot->in_use is reordered before the check of
is_parallel_worker, then it is possible that concurrent registration
of worker can mark the is_parallel_worker as false before we check the
flag here.  See explanation in previous e-mail [1].


[1] - 
https://www.postgresql.org/message-id/CAA4eK1%2BQ_DdJ28qXYSHZiBKNf2MY1QFrv5XAOAw4ZXHw4TPMxA%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