Re: Built-in connection pooler

From: Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>
To: Ryan Lambert <ryan(at)rustprooflabs(dot)com>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Bruce Momjian <bruce(at)momjian(dot)us>, Dimitri Fontaine <dim(at)tapoueh(dot)org>
Subject: Re: Built-in connection pooler
Date: 2019-07-18 12:10:35
Message-ID: da9d967a-e5ef-1966-6f75-71d0e3bb7f4d@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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."
>
>
> [1]
> https://www.postgresql.org/message-id/attachment/102610/builtin_connection_proxy-8.patch
>
> [2]
> https://www.postgresql.org/message-id/flat/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7%2BNKz46H5EOO2k5H7OQ%40mail.gmail.com
>
> [3] https://commitfest.postgresql.org/23/2055/

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.

Attachment Content-Type Size
builtin_connection_proxy-9.patch text/x-patch 103.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jesper Pedersen 2019-07-18 12:39:48 Re: pg_receivewal documentation
Previous Message Amit Kapila 2019-07-18 11:40:07 Re: POC: Cleaning up orphaned files using undo logs