Built-in connection pooler

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
50 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Built-in connection pooler

konstantin knizhnik
Hi hacker,

I am working for some time under built-in connection pooler for Postgres:
https://www.postgresql.org/message-id/flat/a866346d-5582-e8e8-2492-fd32732b0783%40postgrespro.ru#b1c354d5cf937893a75954e1e77f81f5

Unlike existed external pooler, this solution supports session semantic for pooled connections: you can use temporary tables, prepared statements, GUCs,...
But to make it possible I need to store/restore session context.
It is not so difficult, but it requires significant number of changes in Postgres code.
It will be committed in PgProEE-12 version of Postgres Professional version of Postgres,
but I realize that there are few changes to commit it to mainstream version of Postgres.

Dimitri Fontaine proposed to develop much simpler version of pooler which can be community:

The main idea I want to pursue is the following:

  - only solve the “idle connection” problem, nothing else, making idle connection basically free
  - implement a layer in between a connection and a session, managing a “client backend” pool
  - use the ability to give a socket to another process, as you did, so that the pool is not a proxy
  - allow re-using of a backend for a new session only when it is safe to do so
Unfortunately, we have not found a way to support SSL connections with socket redirection.
So I have implemented solution with traditional proxy approach. 
If client changes session context (creates temporary tables, set GUC values, prepare statements,...) then its backend becomes "tainted"
and is not more participate in pooling. It is now dedicated to this backend. But it still receives data through proxy.
Once this client is disconnected, tainted backend is terminated.

Built-in proxy accepts connection on special port (by default 6543).
If you connect to standard port, then normal Postgres backends will be launched and there is no difference with vanilla Postgres .
And if you connect to proxy port, then connection is redirected to one of proxy workers which then perform scheduling of all sessions, assigned to it.
There is currently on migration of sessions between proxies. There are three policies of assigning session to proxy:
random, round-robin and load-balancing.


The main differences with pgbouncer&K are that:

1. It is embedded and requires no extra steps for installation and configurations.
2. It is not single threaded (no bottleneck)
3. It supports all clients (if client needs session semantic, then it will be implicitly given dedicated backend)


Some performance results (pgbench -S -n):

#Connections
Proxy
Proxy/SSL
Direct
Direct/SSL
1
13752
12396
17443
15762
10
53415
59615
68334
85885
1000
60152
20445
60003
24047


Proxy configuration is the following:

session_pool_size = 4
connection_proxies = 2

postgres=# select * from pg_pooler_state();
 pid  | n_clients | n_ssl_clients | n_pools | n_backends | n_dedicated_backends | tx_bytes | rx_bytes | n_transactions
------+-----------+---------------+---------+------------+----------------------+----------+----------+----------------
 1310 |         1 |             0 |       1 |          4 |                    0 | 10324739 |  9834981 |         156388
 1311 |         0 |             0 |       1 |          4 |                    0 | 10430566 |  9936634 |         158007
(2 rows)


This implementation contains much less changes to Postgres core (it is more like invoking pgbouncer as Postgres worker).
The main things I have added are:
1. Mechanism for sending socket to a process (needed to redirect connection to proxy)
2. Support of edge pooling mode for epoll (needed to multiplex reads and writes)
3. Library libpqconn for establishing libpq connection from core


Proxy version of built-in connection pool is in conn_proxy branch in the following GIT repository:
    https://github.com/postgrespro/postgresql.builtin_pool.git


Also I attach patch to the master to this mail.
Will be please to receive your comments.

-- 
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company 

builtin_connection_proxy-1.patch (100K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

Bruce Momjian
On Thu, Jan 24, 2019 at 08:14:41PM +0300, Konstantin Knizhnik wrote:

> The main differences with pgbouncer&K are that:
>
> 1. It is embedded and requires no extra steps for installation and
> configurations.
> 2. It is not single threaded (no bottleneck)
> 3. It supports all clients (if client needs session semantic, then it will be
> implicitly given dedicated backend)
>
>
> Some performance results (pgbench -S -n):
>
> ┌────────────────┬────────┬─────────────┬─────────┬─────────────────────────┐
> │ #Connections   │ Proxy  │ Proxy/SSL   │ Direct  │ Direct/SSL   │
> ├────────────────┼────────┼─────────────┼─────────┼──────────────┤
> │ 1              │  13752 │       12396 │   17443 │        15762 │
> ├────────────────┼────────┼─────────────┼─────────┼──────────────┤
> │ 10             │  53415 │       59615 │   68334 │        85885 │
> ├────────────────┼────────┼─────────────┼─────────┼──────────────┤
> │ 1000           │  60152 │       20445 │   60003 │        24047 │
> └────────────────┴────────┴─────────────┴─────────┴──────────────┘

It is nice it is a smaller patch.  Please remind me of the performance
advantages of this patch.

--
  Bruce Momjian  <[hidden email]>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

Dimitri Fontaine-5
Hi,

Bruce Momjian <[hidden email]> writes:
> It is nice it is a smaller patch.  Please remind me of the performance
> advantages of this patch.

The patch as it stands is mostly helpful in those situations:

  - application server(s) start e.g. 2000 connections at start-up and
    then use them depending on user traffic

    It's then easy to see that if we would only fork as many backends as
    we need, while having accepted the 2000 connections without doing
    anything about them, we would be in a much better position than when
    we fork 2000 unused backends.

  - application is partially compatible with pgbouncer transaction
    pooling mode

    Then in that case, you would need to run with pgbouncer in session
    mode. This happens when the application code is using session level
    SQL commands/objects, such as prepared statements, temporary tables,
    or session-level GUCs settings.

    With the attached patch, if the application sessions profiles are
    mixed, then you dynamically get the benefits of transaction pooling
    mode for those sessions which are not “tainting” the backend, and
    session pooling mode for the others.

    It means that it's then possible to find the most often used session
    and fix that one for immediate benefits, leaving the rest of the
    code alone. If it turns out that 80% of your application sessions
    are the same code-path and you can make this one “transaction
    pooling” compatible, then you most probably are fixing (up to) 80%
    of your connection-related problems in production.

  - applications that use a very high number of concurrent sessions

    In that case, you can either set your connection pooling the same as
    max_connection and see no benefits (and hopefully no regressions
    either), or set a lower number of backends serving a very high
    number of connections, and have sessions waiting their turn at the
    “proxy” stage.

    This is a kind of naive Admission Control implementation where it's
    better to have active clients in the system wait in line consuming
    as few resources as possible. Here, in the proxy. It could be done
    with pgbouncer already, this patch gives a stop-gap in PostgreSQL
    itself for those use-cases.

    It would be mostly useful to do that when you have queries that are
    benefiting of parallel workers. In that case, controling the number
    of active backend forked at any time to serve user queries allows to
    have better use of the parallel workers available.

In other cases, it's important to measure and accept the possible
performance cost of running a proxy server between the client connection
and the PostgreSQL backend process. I believe the numbers shown in the
previous email by Konstantin are about showing the kind of impact you
can see when using the patch in a use-case where it's not meant to be
helping much, if at all.

Regards,
--
dim

Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

Michael Paquier-2
On Mon, Jan 28, 2019 at 10:33:06PM +0100, Dimitri Fontaine wrote:
> In other cases, it's important to measure and accept the possible
> performance cost of running a proxy server between the client connection
> and the PostgreSQL backend process. I believe the numbers shown in the
> previous email by Konstantin are about showing the kind of impact you
> can see when using the patch in a use-case where it's not meant to be
> helping much, if at all.

Have you looked at the possibility of having the proxy worker be
spawned as a background worker?  I think that we should avoid spawning
any new processes on the backend from the ground as we have a lot more
infrastructures since 9.3.  The patch should actually be bigger, the
code is very raw and lacks comments in a lot of areas where the logic
is not so obvious, except perhaps to the patch author.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik
In reply to this post by Bruce Momjian


On 29.01.2019 0:10, Bruce Momjian wrote:

> On Thu, Jan 24, 2019 at 08:14:41PM +0300, Konstantin Knizhnik wrote:
>> The main differences with pgbouncer&K are that:
>>
>> 1. It is embedded and requires no extra steps for installation and
>> configurations.
>> 2. It is not single threaded (no bottleneck)
>> 3. It supports all clients (if client needs session semantic, then it will be
>> implicitly given dedicated backend)
>>
>>
>> Some performance results (pgbench -S -n):
>>
>> ┌────────────────┬────────┬─────────────┬─────────┬─────────────────────────┐
>> │ #Connections   │ Proxy  │ Proxy/SSL   │ Direct  │ Direct/SSL   │
>> ├────────────────┼────────┼─────────────┼─────────┼──────────────┤
>> │ 1              │  13752 │       12396 │   17443 │        15762 │
>> ├────────────────┼────────┼─────────────┼─────────┼──────────────┤
>> │ 10             │  53415 │       59615 │   68334 │        85885 │
>> ├────────────────┼────────┼─────────────┼─────────┼──────────────┤
>> │ 1000           │  60152 │       20445 │   60003 │        24047 │
>> └────────────────┴────────┴─────────────┴─────────┴──────────────┘
> It is nice it is a smaller patch.  Please remind me of the performance
> advantages of this patch.
>
The primary purpose of pooler is efficient support of large number of
connections and minimizing system resource usage.
But as far as Postgres is not scaling well at SMP system with larger
number of CPU cores (due to many reasons discussed in hackers)
reducing number of concurrently working backends can also significantly
increase performance.

I have not done such testing yet but I am planing to do it as well as
comparison with pgbouncer and Odyssey.
But please notice that this proxy approach is by design slower than my
previous implementation used in PgPRO-EE (based on socket redirection).
At some workloads connections throughout proxy cause up to two times
decrease of performance comparing with dedicated backends.
There is no such problem with old connection pooler implementation which
was always not worser than vanilla. But it doesn't support SSL connections
and requires much more changes in Postgres core.







--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik
In reply to this post by Michael Paquier-2


On 29.01.2019 8:14, Michael Paquier wrote:
> On Mon, Jan 28, 2019 at 10:33:06PM +0100, Dimitri Fontaine wrote:
>> In other cases, it's important to measure and accept the possible
>> performance cost of running a proxy server between the client connection
>> and the PostgreSQL backend process. I believe the numbers shown in the
>> previous email by Konstantin are about showing the kind of impact you
>> can see when using the patch in a use-case where it's not meant to be
>> helping much, if at all.
> Have you looked at the possibility of having the proxy worker be
> spawned as a background worker?

Yes, it was my first attempt.
The main reason why I have implemented it in old ways are:
1. I need to know PID of spawned worker. Strange - it is possible to get
PID of dynamic bgworker, but no of static one.
Certainly it is possible  to find a way of passing this PID to
postmaster but it complicates start of worker.
2. I need to pass socket to spawner proxy.  Once again: it can be
implemented also with bgworker but requires more coding (especially
taken in account support of Win32 and FORKEXEC mode).

>   I think that we should avoid spawning
> any new processes on the backend from the ground as we have a lot more
> infrastructures since 9.3.  The patch should actually be bigger, the
> code is very raw and lacks comments in a lot of areas where the logic
> is not so obvious, except perhaps to the patch author.

The main reason for publishing this patch was to receive feedbacks and
find places which should be rewritten.
I will add more comments but I will be pleased if you point me which
places are obscure now and require better explanation.


--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik
In reply to this post by Bruce Momjian
Attached please find results of benchmarking of different connection
poolers.

Hardware configuration:
    Intel(R) Xeon(R) CPU           X5675  @ 3.07GHz
    24 cores (12 physical)
    50 GB RAM

Tests:
      pgbench read-write (scale 1): performance is mostly limited by
disk throughput
      pgbench select-only (scale 1): performance is mostly limited by
efficient utilization of CPU by all workers
      pgbench with YCSB-like workload with Zipf distribution:
performance is mostly limited by lock contention

Participants:
     1. pgbouncer (16 and 32 pool size, transaction level pooling)
     2. Postgres Pro-EE connection poller: redirection of client
connection to poll workers and maintaining of session contexts.
         16 and 32 connection pool size (number of worker backend).
     3. Built-in proxy connection pooler: implementation proposed in
this thread.
         16/1 and 16/2 specifies number of worker backends per proxy and
number of proxies, total number of backends is multiplication of this
values.
     4. Yandex Odyssey (32/2 and 64/4 configurations specifies number of
backends and Odyssey threads).
     5. Vanilla Postgres (marked at diagram as "12devel-master/2fadf24
POOL=none")

In all cases except 2) master branch of Postgres is used.
Client (pgbench), pooler and postgres are laucnhed at the same host.
Communication is though loopback interface (host=localhost).
We have tried to find the optimal parameters for each pooler.
Three graphics attached to the mail illustrate three different test cases.

Few comments about this results:
- PgPro EE pooler shows the best results in all cases except tpc-b like.
In this case proxy approach is more efficient because more flexible job
schedule between workers
   (in EE sessions are scattered between worker backends at connect
time, while proxy chooses least loaded backend for each transaction).
- pgbouncer is not able to scale well because of its single-threaded
architecture. Certainly it is possible to spawn several instances of
pgbouncer and scatter
   workload between them. But we have not did it.
- Vanilla Postgres demonstrates significant degradation of performance
for large number of active connections on all workloads except read-only.
- Despite to the fact that Odyssey is new player (or may be because of
it), Yandex pooler doesn't demonstrate good results. It is the only
pooler which also cause degrade of performance with increasing number of
connections. May be it is caused by memory leaks, because it memory
footprint is also actively increased during test.

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


tpc-b.png (175K) Download Attachment
select-only.png (161K) Download Attachment
zipf.png (174K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik
New version of the patch (rebased + bug fixes) is attached to this mail.

On 20.03.2019 18:32, Konstantin Knizhnik wrote:

> Attached please find results of benchmarking of different connection
> poolers.
>
> Hardware configuration:
>    Intel(R) Xeon(R) CPU           X5675  @ 3.07GHz
>    24 cores (12 physical)
>    50 GB RAM
>
> Tests:
>      pgbench read-write (scale 1): performance is mostly limited by
> disk throughput
>      pgbench select-only (scale 1): performance is mostly limited by
> efficient utilization of CPU by all workers
>      pgbench with YCSB-like workload with Zipf distribution:
> performance is mostly limited by lock contention
>
> Participants:
>     1. pgbouncer (16 and 32 pool size, transaction level pooling)
>     2. Postgres Pro-EE connection poller: redirection of client
> connection to poll workers and maintaining of session contexts.
>         16 and 32 connection pool size (number of worker backend).
>     3. Built-in proxy connection pooler: implementation proposed in
> this thread.
>         16/1 and 16/2 specifies number of worker backends per proxy
> and number of proxies, total number of backends is multiplication of
> this values.
>     4. Yandex Odyssey (32/2 and 64/4 configurations specifies number
> of backends and Odyssey threads).
>     5. Vanilla Postgres (marked at diagram as "12devel-master/2fadf24
> POOL=none")
>
> In all cases except 2) master branch of Postgres is used.
> Client (pgbench), pooler and postgres are laucnhed at the same host.
> Communication is though loopback interface (host=localhost).
> We have tried to find the optimal parameters for each pooler.
> Three graphics attached to the mail illustrate three different test
> cases.
>
> Few comments about this results:
> - PgPro EE pooler shows the best results in all cases except tpc-b
> like. In this case proxy approach is more efficient because more
> flexible job schedule between workers
>   (in EE sessions are scattered between worker backends at connect
> time, while proxy chooses least loaded backend for each transaction).
> - pgbouncer is not able to scale well because of its single-threaded
> architecture. Certainly it is possible to spawn several instances of
> pgbouncer and scatter
>   workload between them. But we have not did it.
> - Vanilla Postgres demonstrates significant degradation of performance
> for large number of active connections on all workloads except read-only.
> - Despite to the fact that Odyssey is new player (or may be because of
> it), Yandex pooler doesn't demonstrate good results. It is the only
> pooler which also cause degrade of performance with increasing number
> of connections. May be it is caused by memory leaks, because it memory
> footprint is also actively increased during test.
>
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


builtin_connection_proxy-3.patch (104K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

Thomas Munro-5
On Thu, Mar 21, 2019 at 4:33 AM Konstantin Knizhnik
<[hidden email]> wrote:
> New version of the patch (rebased + bug fixes) is attached to this mail.

Hi again Konstantin,

Interesting work.  No longer applies -- please rebase.

--
Thomas Munro
https://enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik


On 01.07.2019 12:57, Thomas Munro wrote:
> On Thu, Mar 21, 2019 at 4:33 AM Konstantin Knizhnik
> <[hidden email]> wrote:
>> New version of the patch (rebased + bug fixes) is attached to this mail.
> Hi again Konstantin,
>
> Interesting work.  No longer applies -- please rebase.
>
Rebased version of the patch is attached.
Also all this version of built-ni proxy can be found in conn_proxy
branch of https://github.com/postgrespro/postgresql.builtin_pool.git

builtin_connection_proxy-4.patch (104K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

Thomas Munro-5
On Tue, Jul 2, 2019 at 3:11 AM Konstantin Knizhnik
<[hidden email]> wrote:
> On 01.07.2019 12:57, Thomas Munro wrote:
> > Interesting work.  No longer applies -- please rebase.
> >
> Rebased version of the patch is attached.
> Also all this version of built-ni proxy can be found in conn_proxy
> branch of https://github.com/postgrespro/postgresql.builtin_pool.git

Thanks Konstantin.  I haven't looked at the code, but I can't help
noticing that this CF entry and the autoprepare one are both features
that come up again an again on feature request lists I've seen.
That's very cool.  They also both need architectural-level review.
With my Commitfest manager hat on: reviewing other stuff would help
with that; if you're looking for something of similar complexity and
also the same level of
everyone-knows-we-need-to-fix-this-!@#$-we-just-don't-know-exactly-how-yet
factor, I hope you get time to provide some more feedback on Takeshi
Ideriha's work on shared caches, which doesn't seem a million miles
from some of the things you're working on.

Could you please fix these compiler warnings so we can see this
running check-world on CI?

https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.46324
https://travis-ci.org/postgresql-cfbot/postgresql/builds/555180678

--
Thomas Munro
https://enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik


On 08.07.2019 3:37, Thomas Munro wrote:

> On Tue, Jul 2, 2019 at 3:11 AM Konstantin Knizhnik
> <[hidden email]> wrote:
>> On 01.07.2019 12:57, Thomas Munro wrote:
>>> Interesting work.  No longer applies -- please rebase.
>>>
>> Rebased version of the patch is attached.
>> Also all this version of built-ni proxy can be found in conn_proxy
>> branch of https://github.com/postgrespro/postgresql.builtin_pool.git
> Thanks Konstantin.  I haven't looked at the code, but I can't help
> noticing that this CF entry and the autoprepare one are both features
> that come up again an again on feature request lists I've seen.
> That's very cool.  They also both need architectural-level review.
> With my Commitfest manager hat on: reviewing other stuff would help
> with that; if you're looking for something of similar complexity and
> also the same level of
> everyone-knows-we-need-to-fix-this-!@#$-we-just-don't-know-exactly-how-yet
> factor, I hope you get time to provide some more feedback on Takeshi
> Ideriha's work on shared caches, which doesn't seem a million miles
> from some of the things you're working on.
Thank you, I will look at Takeshi Ideriha's patch.

> Could you please fix these compiler warnings so we can see this
> running check-world on CI?
>
> https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.46324
> https://travis-ci.org/postgresql-cfbot/postgresql/builds/555180678
>
Sorry, I do not have access to Windows host, so can not check Win32
build myself.
I have fixed all the reported warnings but can not verify that Win32
build is now ok.




builtin_connection_proxy-5.patch (104K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

Thomas Munro-5
On Tue, Jul 9, 2019 at 8:30 AM Konstantin Knizhnik
<[hidden email]> wrote:
>>>> Rebased version of the patch is attached.

Thanks for including nice documentation in the patch, which gives a
good overview of what's going on.  I haven't read any code yet, but I
took it for a quick drive to understand the user experience.  These
are just some first impressions.

I started my server with -c connection_proxies=1 and tried to connect
to port 6543 and the proxy segfaulted on null ptr accessing
port->gss->enc.  I rebuilt without --with-gssapi to get past that.

Using SELECT pg_backend_pid() from many different connections, I could
see that they were often being served by the same process (although
sometimes it created an extra one when there didn't seem to be a good
reason for it to do that).  I could see the proxy managing these
connections with SELECT * FROM pg_pooler_state() (I suppose this would
be wrapped in a view with a name like pg_stat_proxies).  I could see
that once I did something like SET foo.bar = 42, a backend became
dedicated to my connection and no other connection could use it.  As
described.  Neat.

Obviously your concept of tainted backends (= backends that can't be
reused by other connections because they contain non-default session
state) is quite simplistic and would help only the very simplest use
cases.  Obviously the problems that need to be solved first to do
better than that are quite large.  Personally I think we should move
all GUCs into the Session struct, put the Session struct into shared
memory, and then figure out how to put things like prepared plans into
something like Ideriha-san's experimental shared memory context so
that they also can be accessed by any process, and then we'll mostly
be tackling problems that we'll have to tackle for threads too.  But I
think you made the right choice to experiment with just reusing the
backends that have no state like that.

On my FreeBSD box (which doesn't have epoll(), so it's latch.c's old
school poll() for now), I see the connection proxy process eating a
lot of CPU and the temperature rising.  I see with truss that it's
doing this as fast as it can:

poll({ 13/POLLIN 17/POLLIN|POLLOUT },2,1000)     = 1 (0x1)

Ouch.  I admit that I had the idea to test on FreeBSD because I
noticed the patch introduces EPOLLET and I figured this might have
been tested only on Linux.  FWIW the same happens on a Mac.

That's all I had time for today, but I'm planning to poke this some
more, and get a better understand of how this works at an OS level.  I
can see fd passing, IO multiplexing, and other interesting things
happening.  I suspect there are many people on this list who have
thoughts about the architecture we should use to allow a smaller
number of PGPROCs and a larger number of connections, with various
different motivations.

> Thank you, I will look at Takeshi Ideriha's patch.

Cool.

> > Could you please fix these compiler warnings so we can see this
> > running check-world on CI?
> >
> > https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.46324
> > https://travis-ci.org/postgresql-cfbot/postgresql/builds/555180678
> >
> Sorry, I do not have access to Windows host, so can not check Win32
> build myself.

  C:\projects\postgresql\src\include\../interfaces/libpq/libpq-int.h(33):
fatal error C1083: Cannot open include file: 'pthread-win32.h': No
such file or directory (src/backend/postmaster/proxy.c)
[C:\projects\postgresql\postgres.vcxproj]

These relative includes in proxy.c are part of the problem:

#include "../interfaces/libpq/libpq-fe.h"
#include "../interfaces/libpq/libpq-int.h"

I didn't dig into this much but some first reactions:

1.  I see that proxy.c uses libpq, and correctly loads it as a dynamic
library just like postgres_fdw.  Unfortunately it's part of core, so
it can't use the same technique as postgres_fdw to add the libpq
headers to the include path.

2.  libpq-int.h isn't supposed to be included by code outside libpq,
and in this case it fails to find pthead-win32.h which is apparently
expects to find in either the same directory or the include path.  I
didn't look into what exactly is going on (I don't have Windows
either) but I think we can say the root problem is that you shouldn't
be including that from backend code.

--
Thomas Munro
https://enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik


On 14.07.2019 8:03, Thomas Munro wrote:

> On Tue, Jul 9, 2019 at 8:30 AM Konstantin Knizhnik
> <[hidden email]> wrote:
>>>>> Rebased version of the patch is attached.
> Thanks for including nice documentation in the patch, which gives a
> good overview of what's going on.  I haven't read any code yet, but I
> took it for a quick drive to understand the user experience.  These
> are just some first impressions.
>
> I started my server with -c connection_proxies=1 and tried to connect
> to port 6543 and the proxy segfaulted on null ptr accessing
> port->gss->enc.  I rebuilt without --with-gssapi to get past that.

First of all a lot of thanks for your review.

Sorry, I failed to reproduce the problem with GSSAPI.
I have configured postgres with --with-openssl --with-gssapi
and then try to connect to the serverwith psql using the following
connection string:

psql "sslmode=require dbname=postgres port=6543 krbsrvname=POSTGRES"

There are no SIGFAULS and port->gss structure is normally initialized.

Can you please explain me more precisely how to reproduce the problem
(how to configure postgres and how to connect to it)?

>
> Using SELECT pg_backend_pid() from many different connections, I could
> see that they were often being served by the same process (although
> sometimes it created an extra one when there didn't seem to be a good
> reason for it to do that).  I could see the proxy managing these
> connections with SELECT * FROM pg_pooler_state() (I suppose this would
> be wrapped in a view with a name like pg_stat_proxies).  I could see
> that once I did something like SET foo.bar = 42, a backend became
> dedicated to my connection and no other connection could use it.  As
> described.  Neat.
>
> Obviously your concept of tainted backends (= backends that can't be
> reused by other connections because they contain non-default session
> state) is quite simplistic and would help only the very simplest use
> cases.  Obviously the problems that need to be solved first to do
> better than that are quite large.  Personally I think we should move
> all GUCs into the Session struct, put the Session struct into shared
> memory, and then figure out how to put things like prepared plans into
> something like Ideriha-san's experimental shared memory context so
> that they also can be accessed by any process, and then we'll mostly
> be tackling problems that we'll have to tackle for threads too.  But I
> think you made the right choice to experiment with just reusing the
> backends that have no state like that.
That was not actually my idea: it was proposed by Dimitri Fontaine.
In PgPRO-EE we have another version of builtin connection pooler
which maintains session context and allows to use GUCs, prepared statements
and temporary tables in pooled sessions.
But the main idea of this patch was to make connection pooler less
invasive and
minimize changes in Postgres core. This is why I have implemented proxy.


> On my FreeBSD box (which doesn't have epoll(), so it's latch.c's old
> school poll() for now), I see the connection proxy process eating a
> lot of CPU and the temperature rising.  I see with truss that it's
> doing this as fast as it can:
>
> poll({ 13/POLLIN 17/POLLIN|POLLOUT },2,1000)     = 1 (0x1)
>
> Ouch.  I admit that I had the idea to test on FreeBSD because I
> noticed the patch introduces EPOLLET and I figured this might have
> been tested only on Linux.  FWIW the same happens on a Mac.

Yehh.
This is really the problem. In addition to FreeBSD and MacOS, it also
takes place at Win32.
I have to think more how to solve it. We should somehow emulate
"edge-triggered" more for this system.
The source of the problem is that proxy is reading data from one socket
and writing it in another socket.
If write socket is busy, we have to wait until is is available. But at
the same time there can be data available for input,
so poll will return immediately unless we remove read event for this
socket. Not here how to better do it without changing
WaitEvenSet API.


>
>    C:\projects\postgresql\src\include\../interfaces/libpq/libpq-int.h(33):
> fatal error C1083: Cannot open include file: 'pthread-win32.h': No
> such file or directory (src/backend/postmaster/proxy.c)
> [C:\projects\postgresql\postgres.vcxproj]

I will investigate the problem with Win32 build once I get image of
Win32 for VirtualBox.

> These relative includes in proxy.c are part of the problem:
>
> #include "../interfaces/libpq/libpq-fe.h"
> #include "../interfaces/libpq/libpq-int.h"
>
> I didn't dig into this much but some first reactions:
I have added
override CPPFLAGS :=  $(CPPFLAGS) -I$(top_builddir)/src/port
-I$(top_srcdir)/src/port

in Makefile for postmaster in order to fix this problem (as in
interface/libpq/Makefile).
But looks like it is not enough. As I wrote above I will try to solve
this problem once I get access to Win32 environment.

> 1.  I see that proxy.c uses libpq, and correctly loads it as a dynamic
> library just like postgres_fdw.  Unfortunately it's part of core, so
> it can't use the same technique as postgres_fdw to add the libpq
> headers to the include path.
>
> 2.  libpq-int.h isn't supposed to be included by code outside libpq,
> and in this case it fails to find pthead-win32.h which is apparently
> expects to find in either the same directory or the include path.  I
> didn't look into what exactly is going on (I don't have Windows
> either) but I think we can say the root problem is that you shouldn't
> be including that from backend code.
>
Looks like proxy.c has to be moved somewhere outside core?
May be make it an extension? But it may be not so easy to implement because
proxy has to be tightly integrated with postmaster.


Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

Thomas Munro-5
On Mon, Jul 15, 2019 at 7:56 AM Konstantin Knizhnik
<[hidden email]> wrote:
> Can you please explain me more precisely how to reproduce the problem
> (how to configure postgres and how to connect to it)?

Maybe it's just that postmaster.c's ConnCreate() always allocates a
struct for port->gss if the feature is enabled, but the equivalent
code in proxy.c's proxy_loop() doesn't?

./configure
  --prefix=$HOME/install/postgres \
  --enable-cassert \
  --enable-debug \
  --enable-depend \
  --with-gssapi \
  --with-icu \
  --with-pam \
  --with-ldap \
  --with-openssl \
  --enable-tap-tests \
  --with-includes="/opt/local/include" \
  --with-libraries="/opt/local/lib" \
  CC="ccache cc" CFLAGS="-O0"

~/install/postgres/bin/psql postgres -h localhost -p 6543

2019-07-15 08:37:25.348 NZST [97972] LOG:  server process (PID 97974)
was terminated by signal 11: Segmentation fault: 11

(lldb) bt
* thread #1, stop reason = signal SIGSTOP
  * frame #0: 0x0000000104163e7e
postgres`secure_read(port=0x00007fda9ef001d0, ptr=0x00000001047ab690,
len=8192) at be-secure.c:164:6
    frame #1: 0x000000010417760d postgres`pq_recvbuf at pqcomm.c:969:7
    frame #2: 0x00000001041779ed postgres`pq_getbytes(s="??\x04~?,
len=1) at pqcomm.c:1110:8
    frame #3: 0x0000000104284147
postgres`ProcessStartupPacket(port=0x00007fda9ef001d0,
secure_done=true) at postmaster.c:2064:6
...
(lldb) f 0
frame #0: 0x0000000104163e7e
postgres`secure_read(port=0x00007fda9ef001d0, ptr=0x00000001047ab690,
len=8192) at be-secure.c:164:6
   161         else
   162     #endif
   163     #ifdef ENABLE_GSS
-> 164         if (port->gss->enc)
   165         {
   166             n = be_gssapi_read(port, ptr, len);
   167             waitfor = WL_SOCKET_READABLE;
(lldb) print port->gss
(pg_gssinfo *) $0 = 0x0000000000000000

> > Obviously your concept of tainted backends (= backends that can't be
> > reused by other connections because they contain non-default session
> > state) is quite simplistic and would help only the very simplest use
> > cases.  Obviously the problems that need to be solved first to do
> > better than that are quite large.  Personally I think we should move
> > all GUCs into the Session struct, put the Session struct into shared
> > memory, and then figure out how to put things like prepared plans into
> > something like Ideriha-san's experimental shared memory context so
> > that they also can be accessed by any process, and then we'll mostly
> > be tackling problems that we'll have to tackle for threads too.  But I
> > think you made the right choice to experiment with just reusing the
> > backends that have no state like that.
>
> That was not actually my idea: it was proposed by Dimitri Fontaine.
> In PgPRO-EE we have another version of builtin connection pooler
> which maintains session context and allows to use GUCs, prepared statements
> and temporary tables in pooled sessions.

Interesting.  Do you serialise/deserialise plans and GUCs using
machinery similar to parallel query, and did you changed temporary
tables to use shared buffers?  People have suggested that we do that
to allow temporary tables in parallel queries too.  FWIW I have also
wondered about per (database, user) pools for reusable parallel
workers.

> But the main idea of this patch was to make connection pooler less
> invasive and
> minimize changes in Postgres core. This is why I have implemented proxy.

How do you do it without a proxy?

One idea I've wondered about that doesn't involve feeding all network
IO through an extra process and extra layer of syscalls like this is
to figure out how to give back your PGPROC slot when idle.  If you
don't have one and can't acquire one at the beginning of a
transaction, you wait until one becomes free.  When idle and not in a
transaction you give it back to the pool, perhaps after a slight
delay.  That may be impossible for some reason or other, I don't know.
If it could be done, it'd deal with the size-of-procarray problem
(since we like to scan it) and provide a kind of 'admission control'
(limiting number of transactions that can run), but wouldn't deal with
the amount-of-backend-memory-wasted-on-per-process-caches problem
(maybe solvable by shared caching).

Ok, time for a little bit more testing.  I was curious about the user
experience when there are no free backends.

1.  I tested with connection_proxies=1, max_connections=4, and I began
3 transactions.  Then I tried to make a 4th connection, and it blocked
while trying to connect and the log shows a 'sorry, too many clients
already' message.  Then I committed one of my transactions and the 4th
connection was allowed to proceed.

2.  I tried again, this time with 4 already existing connections and
no transactions.  I began 3 concurrent transactions, and then when I
tried to begin a 4th transaction the BEGIN command blocked until one
of the other transactions committed.

Some thoughts:   Why should case 1 block?  Shouldn't I be allowed to
connect, even though I can't begin a transaction without waiting yet?
 Why can I run only 3 transactions when I said max_connection=4?  Ah,
that's probably because the proxy itself is eating one slot, and
indeed if I set connection_proxies to 2 I can now run only two
concurrent transactions.  And yet when there were no transactions
running I could still open 4 connections.  Hmm.

The general behaviour of waiting instead of raising an error seems
sensible, and that's how client-side connection pools usually work.
Perhaps some of the people who have wanted admission control were
thinking of doing it at the level of queries rather than whole
transactions though, I don't know.  I suppose in extreme cases it's
possible to create invisible deadlocks, if a client is trying to
control more than one transaction concurrently, but that doesn't seem
like a real problem.

> > On my FreeBSD box (which doesn't have epoll(), so it's latch.c's old
> > school poll() for now), I see the connection proxy process eating a
> > lot of CPU and the temperature rising.  I see with truss that it's
> > doing this as fast as it can:
> >
> > poll({ 13/POLLIN 17/POLLIN|POLLOUT },2,1000)     = 1 (0x1)
> >
> > Ouch.  I admit that I had the idea to test on FreeBSD because I
> > noticed the patch introduces EPOLLET and I figured this might have
> > been tested only on Linux.  FWIW the same happens on a Mac.
>
> Yehh.
> This is really the problem. In addition to FreeBSD and MacOS, it also
> takes place at Win32.
> I have to think more how to solve it. We should somehow emulate
> "edge-triggered" more for this system.
> The source of the problem is that proxy is reading data from one socket
> and writing it in another socket.
> If write socket is busy, we have to wait until is is available. But at
> the same time there can be data available for input,
> so poll will return immediately unless we remove read event for this
> socket. Not here how to better do it without changing
> WaitEvenSet API.

Can't you do this by removing events you're not interested in right
now, using ModifyWaitEvent() to change between WL_SOCKET_WRITEABLE and
WL_SOCKET_READABLE as appropriate?  Perhaps the problem you're worried
about is that this generates extra system calls in the epoll()
implementation?  I think that's not a problem for poll(), and could be
fixed for the kqueue() implementation I plan to commit eventually.  I
have no clue for Windows.

> Looks like proxy.c has to be moved somewhere outside core?
> May be make it an extension? But it may be not so easy to implement because
> proxy has to be tightly integrated with postmaster.

I'm not sure.  Seems like it should be solvable with the code in core.

--
Thomas Munro
https://enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik


On 15.07.2019 1:48, Thomas Munro wrote:

> On Mon, Jul 15, 2019 at 7:56 AM Konstantin Knizhnik
> <[hidden email]> wrote:
>> Can you please explain me more precisely how to reproduce the problem
>> (how to configure postgres and how to connect to it)?
> Maybe it's just that postmaster.c's ConnCreate() always allocates a
> struct for port->gss if the feature is enabled, but the equivalent
> code in proxy.c's proxy_loop() doesn't?
>
> ./configure
>    --prefix=$HOME/install/postgres \
>    --enable-cassert \
>    --enable-debug \
>    --enable-depend \
>    --with-gssapi \
>    --with-icu \
>    --with-pam \
>    --with-ldap \
>    --with-openssl \
>    --enable-tap-tests \
>    --with-includes="/opt/local/include" \
>    --with-libraries="/opt/local/lib" \
>    CC="ccache cc" CFLAGS="-O0"
>
> ~/install/postgres/bin/psql postgres -h localhost -p 6543
>
> 2019-07-15 08:37:25.348 NZST [97972] LOG:  server process (PID 97974)
> was terminated by signal 11: Segmentation fault: 11
>
> (lldb) bt
> * thread #1, stop reason = signal SIGSTOP
>    * frame #0: 0x0000000104163e7e
> postgres`secure_read(port=0x00007fda9ef001d0, ptr=0x00000001047ab690,
> len=8192) at be-secure.c:164:6
>      frame #1: 0x000000010417760d postgres`pq_recvbuf at pqcomm.c:969:7
>      frame #2: 0x00000001041779ed postgres`pq_getbytes(s="??\x04~?,
> len=1) at pqcomm.c:1110:8
>      frame #3: 0x0000000104284147
> postgres`ProcessStartupPacket(port=0x00007fda9ef001d0,
> secure_done=true) at postmaster.c:2064:6
> ...
> (lldb) f 0
> frame #0: 0x0000000104163e7e
> postgres`secure_read(port=0x00007fda9ef001d0, ptr=0x00000001047ab690,
> len=8192) at be-secure.c:164:6
>     161         else
>     162     #endif
>     163     #ifdef ENABLE_GSS
> -> 164         if (port->gss->enc)
>     165         {
>     166             n = be_gssapi_read(port, ptr, len);
>     167             waitfor = WL_SOCKET_READABLE;
> (lldb) print port->gss
> (pg_gssinfo *) $0 = 0x0000000000000000

Thank you, fixed (pushed to
https://github.com/postgrespro/postgresql.builtin_pool.git repository).
I am not sure that GSS authentication works as intended, but at least

psql "sslmode=require dbname=postgres port=6543 krbsrvname=POSTGRES"

is normally connected.


>>> Obviously your concept of tainted backends (= backends that can't be
>>> reused by other connections because they contain non-default session
>>> state) is quite simplistic and would help only the very simplest use
>>> cases.  Obviously the problems that need to be solved first to do
>>> better than that are quite large.  Personally I think we should move
>>> all GUCs into the Session struct, put the Session struct into shared
>>> memory, and then figure out how to put things like prepared plans into
>>> something like Ideriha-san's experimental shared memory context so
>>> that they also can be accessed by any process, and then we'll mostly
>>> be tackling problems that we'll have to tackle for threads too.  But I
>>> think you made the right choice to experiment with just reusing the
>>> backends that have no state like that.
>> That was not actually my idea: it was proposed by Dimitri Fontaine.
>> In PgPRO-EE we have another version of builtin connection pooler
>> which maintains session context and allows to use GUCs, prepared statements
>> and temporary tables in pooled sessions.
> Interesting.  Do you serialise/deserialise plans and GUCs using
> machinery similar to parallel query, and did you changed temporary
> tables to use shared buffers?  People have suggested that we do that
> to allow temporary tables in parallel queries too.  FWIW I have also
> wondered about per (database, user) pools for reusable parallel
> workers.

No. The main difference between PG-Pro (conn_pool) and vanilla
(conn_proxy) version of connection pooler
is that first one  bind sessions to pool workers while last one is using
proxy to scatter requests between workers.

So in conn_pool postmaster accepts client connection and the schedule it
(using one of provided scheduling policies, i.e. round robin, random,
load balancing,...) to one of worker backends. Then client socket is
transferred to this backend and client and backend are connected directly.
Session is never rescheduled, i.e. it is bounded to backend. One backend
is serving multiple sessions. Sessions GUCs and some static variables
are saved in session context. Each session has its own temporary
namespace, so temporary tables of different sessions do not interleave.

Direct connection of client and backend allows to eliminate proxy
overhead. But at the same time - binding session to backend it is the
main drawback of this approach. Long living transaction can block all
other sessions scheduled to this backend.
I have though a lot about possibility to reschedule sessions. The main
"show stopper" is actually temporary tables.
Implementation of temporary tables is one of the "bad smelling" places
in Postgres causing multiple problems:
parallel queries, using temporary table at replica, catalog bloating,
connection pooling...
This is why I have tried to implement "global" temporary tables (like in
Oracle).
I am going to publish this patch soon: in this case table definition is
global, but data is local for each session.
Also global temporary tables are accessed through shared pool which
makes it possible to use them in parallel queries.
But it is separate story, not currently related with connection pooling.

Approach with proxy doesn't suffer from this problem.



>> But the main idea of this patch was to make connection pooler less
>> invasive and
>> minimize changes in Postgres core. This is why I have implemented proxy.
> How do you do it without a proxy?

I hope I have explained it above.
Actually this version pf connection pooler is also available at github
https://github.com/postgrespro/postgresql.builtin_pool.git repository
branch conn_pool).

> Ok, time for a little bit more testing.  I was curious about the user
> experience when there are no free backends.
>
> 1.  I tested with connection_proxies=1, max_connections=4, and I began
> 3 transactions.  Then I tried to make a 4th connection, and it blocked
> while trying to connect and the log shows a 'sorry, too many clients
> already' message.  Then I committed one of my transactions and the 4th
> connection was allowed to proceed.
>
> 2.  I tried again, this time with 4 already existing connections and
> no transactions.  I began 3 concurrent transactions, and then when I
> tried to begin a 4th transaction the BEGIN command blocked until one
> of the other transactions committed.
>
> Some thoughts:   Why should case 1 block?  Shouldn't I be allowed to
> connect, even though I can't begin a transaction without waiting yet?
>   Why can I run only 3 transactions when I said max_connection=4?  Ah,
> that's probably because the proxy itself is eating one slot, and
> indeed if I set connection_proxies to 2 I can now run only two
> concurrent transactions.  And yet when there were no transactions
> running I could still open 4 connections.  Hmm.

max_connections is not the right switch to control behavior of
connection pooler.
You should adjust session_pool_size, connection_proxies and max_sessions
parameters.

What happen in 1) case? Default value of session_pool_size is 10. It
means that postgres will spawn up to 10 backens for each database/user
combination. But max_connection limit will exhausted earlier. May be it
is better to prohibit setting session_pool_size larger than max_connection
or automatically adjust it according to max_connections. But it is also
no so easy to enforce, because separate set of pool workers is created for
each database/user combination.

So I agree that observer behavior is confusing, but I do not have good
idea how to improve it.

>
> The general behaviour of waiting instead of raising an error seems
> sensible, and that's how client-side connection pools usually work.
> Perhaps some of the people who have wanted admission control were
> thinking of doing it at the level of queries rather than whole
> transactions though, I don't know.  I suppose in extreme cases it's
> possible to create invisible deadlocks, if a client is trying to
> control more than one transaction concurrently, but that doesn't seem
> like a real problem.
>
>>> On my FreeBSD box (which doesn't have epoll(), so it's latch.c's old
>>> school poll() for now), I see the connection proxy process eating a
>>> lot of CPU and the temperature rising.  I see with truss that it's
>>> doing this as fast as it can:
>>>
>>> poll({ 13/POLLIN 17/POLLIN|POLLOUT },2,1000)     = 1 (0x1)
>>>
>>> Ouch.  I admit that I had the idea to test on FreeBSD because I
>>> noticed the patch introduces EPOLLET and I figured this might have
>>> been tested only on Linux.  FWIW the same happens on a Mac.
>> Yehh.
>> This is really the problem. In addition to FreeBSD and MacOS, it also
>> takes place at Win32.
>> I have to think more how to solve it. We should somehow emulate
>> "edge-triggered" more for this system.
>> The source of the problem is that proxy is reading data from one socket
>> and writing it in another socket.
>> If write socket is busy, we have to wait until is is available. But at
>> the same time there can be data available for input,
>> so poll will return immediately unless we remove read event for this
>> socket. Not here how to better do it without changing
>> WaitEvenSet API.
> Can't you do this by removing events you're not interested in right
> now, using ModifyWaitEvent() to change between WL_SOCKET_WRITEABLE and
> WL_SOCKET_READABLE as appropriate?  Perhaps the problem you're worried
> about is that this generates extra system calls in the epoll()
> implementation?  I think that's not a problem for poll(), and could be
> fixed for the kqueue() implementation I plan to commit eventually.  I
> have no clue for Windows.
Window implementation is similar with poll().
Yes, definitely it can be done by removing read handle from wait even
set after each pending write operation.
But it seems to be very inefficient in case of epoll() implementation
(where changing event set requires separate syscall).
So either we have to add some extra function, i.e. WaitEventEdgeTrigger
which will do nothing for epoll(),
either somehow implement edge triggering inside WaitEvent*
implementation itself (not clear how to do it, since read/write
operations are
not performed through this API).



>> Looks like proxy.c has to be moved somewhere outside core?
>> May be make it an extension? But it may be not so easy to implement because
>> proxy has to be tightly integrated with postmaster.
> I'm not sure.  Seems like it should be solvable with the code in core.
>
I also think so. It is now working at Unix and I hope I can also fix
Win32 build.


Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik
In reply to this post by Thomas Munro-5


On 14.07.2019 8:03, Thomas Munro wrote:

>
> On my FreeBSD box (which doesn't have epoll(), so it's latch.c's old
> school poll() for now), I see the connection proxy process eating a
> lot of CPU and the temperature rising.  I see with truss that it's
> doing this as fast as it can:
>
> poll({ 13/POLLIN 17/POLLIN|POLLOUT },2,1000)     = 1 (0x1)
>
> Ouch.  I admit that I had the idea to test on FreeBSD because I
> noticed the patch introduces EPOLLET and I figured this might have
> been tested only on Linux.  FWIW the same happens on a Mac.
>
I have committed patch which emulates epoll EPOLLET flag and so should
avoid busy loop with poll().
I will be pleased if you can check it at FreeBSD  box.



builtin_connection_proxy-7.patch (106K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik


On 15.07.2019 17:04, Konstantin Knizhnik wrote:

>
>
> On 14.07.2019 8:03, Thomas Munro wrote:
>>
>> On my FreeBSD box (which doesn't have epoll(), so it's latch.c's old
>> school poll() for now), I see the connection proxy process eating a
>> lot of CPU and the temperature rising.  I see with truss that it's
>> doing this as fast as it can:
>>
>> poll({ 13/POLLIN 17/POLLIN|POLLOUT },2,1000)     = 1 (0x1)
>>
>> Ouch.  I admit that I had the idea to test on FreeBSD because I
>> noticed the patch introduces EPOLLET and I figured this might have
>> been tested only on Linux.  FWIW the same happens on a Mac.
>>
> I have committed patch which emulates epoll EPOLLET flag and so should
> avoid busy loop with poll().
> I will be pleased if you can check it at FreeBSD  box.
>
>
Sorry, attached patch was incomplete.
Please try this version of the patch.


builtin_connection_proxy-8.patch (106K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

ryanlambert
Hi Konstantin,

Thanks for your work on this.  I'll try to do more testing in the next few days, here's what I have so far.

make installcheck-world: passed

The v8 patch [1] applies, though I get indent and whitespace errors:

<stdin>:79: tab in indent.
                 "Each proxy launches its own subset of backends. So maximal number of non-tainted backends is "
<stdin>:80: tab in indent.
                  "session_pool_size*connection_proxies*databases*roles.
<stdin>:519: indent with spaces.
    char buf[CMSG_SPACE(sizeof(sock))];
<stdin>:520: indent with spaces.
    memset(buf, '\0', sizeof(buf));
<stdin>:522: indent with spaces.
    /* On Mac OS X, the struct iovec is needed, even if it points to minimal data */
warning: squelched 82 whitespace errors
warning: 87 lines add whitespace errors.


In connpool.sgml:

"but it can be changed to standard Postgres 4321"

Should be 5432?

" As far as pooled backends are not terminated on client exist, it will not
    be possible to drop database to which them are connected."

Active discussion in [2] might change that, it is also in this July commitfest [3].

"Unlike pgbouncer and other external connection poolera"

Should be "poolers"

"So developers of client applications still have a choice
    either to avoid using session-specific operations either not to use pooling."

That sentence isn't smooth for me to read.  Maybe something like:
"Developers of client applications have the choice to either avoid using session-specific operations, or not use built-in pooling."

On Tue, Jul 16, 2019 at 12:20 AM Konstantin Knizhnik <[hidden email]> wrote:


On 15.07.2019 17:04, Konstantin Knizhnik wrote:
>
>
> On 14.07.2019 8:03, Thomas Munro wrote:
>>
>> On my FreeBSD box (which doesn't have epoll(), so it's latch.c's old
>> school poll() for now), I see the connection proxy process eating a
>> lot of CPU and the temperature rising.  I see with truss that it's
>> doing this as fast as it can:
>>
>> poll({ 13/POLLIN 17/POLLIN|POLLOUT },2,1000)     = 1 (0x1)
>>
>> Ouch.  I admit that I had the idea to test on FreeBSD because I
>> noticed the patch introduces EPOLLET and I figured this might have
>> been tested only on Linux.  FWIW the same happens on a Mac.
>>
> I have committed patch which emulates epoll EPOLLET flag and so should
> avoid busy loop with poll().
> I will be pleased if you can check it at FreeBSD  box.
>
>
Sorry, attached patch was incomplete.
Please try this version of the patch.

Ryan Lambert
RustProof  Labs
Reply | Threaded
Open this post in threaded view
|

Re: Built-in connection pooler

konstantin knizhnik
Hi, Ryan

On 18.07.2019 6:01, Ryan Lambert wrote:
Hi Konstantin,

Thanks for your work on this.  I'll try to do more testing in the next few days, here's what I have so far.

make installcheck-world: passed

The v8 patch [1] applies, though I get indent and whitespace errors:

<stdin>:79: tab in indent.
                 "Each proxy launches its own subset of backends. So maximal number of non-tainted backends is "
<stdin>:80: tab in indent.
                  "session_pool_size*connection_proxies*databases*roles.
<stdin>:519: indent with spaces.
    char buf[CMSG_SPACE(sizeof(sock))];
<stdin>:520: indent with spaces.
    memset(buf, '\0', sizeof(buf));
<stdin>:522: indent with spaces.
    /* On Mac OS X, the struct iovec is needed, even if it points to minimal data */
warning: squelched 82 whitespace errors
warning: 87 lines add whitespace errors.


In connpool.sgml:

"but it can be changed to standard Postgres 4321"

Should be 5432?

" As far as pooled backends are not terminated on client exist, it will not
    be possible to drop database to which them are connected."

Active discussion in [2] might change that, it is also in this July commitfest [3].

"Unlike pgbouncer and other external connection poolera"

Should be "poolers"

"So developers of client applications still have a choice
    either to avoid using session-specific operations either not to use pooling."

That sentence isn't smooth for me to read.  Maybe something like:
"Developers of client applications have the choice to either avoid using session-specific operations, or not use built-in pooling."

Thank you for review.
I have fixed all reported issues except one related with "dropdb --force" discussion.
As far as this patch is not yet committed, I can not rely on it yet.
Certainly I can just remove this sentence from documentation,  assuming that this patch will be committed soon.
But then some extra efforts will be needed to terminated pooler backends of dropped database.




builtin_connection_proxy-9.patch (106K) Download Attachment
123