On 19.07.2019 6:36, Ryan Lambert wrote:
Here's what I found tonight in your latest patch (9). The output from git apply is better, fewer whitespace errors, but still getting the following. Ubuntu 18.04 if that helps.
git apply -p1 < builtin_connection_proxy-9.patch
<stdin>:79: tab in indent.
Each proxy launches its own subset of backends.
<stdin>:634: indent with spaces.
union {
<stdin>:635: indent with spaces.
struct sockaddr_in inaddr;
<stdin>:636: indent with spaces.
struct sockaddr addr;
<stdin>:637: indent with spaces.
} a;
warning: squelched 54 whitespace errors
warning: 59 lines add whitespace errors.
A few more minor edits. In config.sgml:
"If the <varname>max_sessions</varname> limit is reached new connection are not accepted."
Should be "connections".
"The default value is 10, so up to 10 backends will server each database,"
"sever" should be "serve" and the sentence should end with a period instead of a comma.
In postmaster.c:
/* The socket number we are listening for poolled connections on */
"poolled" --> "pooled"
"(errmsg("could not create listen socket for locahost")));"
"locahost" -> "localhost".
" * so to support order balancing we should do dome smart work here."
"dome" should be "some"?
I don't see any tests covering this new functionality. It seems that this is significant enough functionality to warrant some sort of tests, but I don't know exactly what those would/should be.
Thank you once again for this fixes.
Updated patch is attached.
Concerning testing: I do not think that connection pooler needs some kind of special tests.
The idea of built-in connection pooler is that it should be able to handle all requests normal postgres can do.
I have added to regression tests extra path with enabled connection proxies.
Unfortunately, pg_regress is altering some session variables, so it backend becomes tainted and so
pooling is not actually used (but communication through proxy is tested).
It is also possible to run pg_bench with different number of connections though connection pooler.