On 01/19/2018 10:52 AM, Konstantin Knizhnik wrote: > > > On 18.01.2018 18:02, Tomas Vondra wrote: >> Hi Konstantin, >> >> On 01/18/2018 03:48 PM, Konstantin Knizhnik wrote: >>> On 17.01.2018 19:09, Konstantin Knizhnik wrote: >>>> Hi hackers, >>>> >>>> ... >> I haven't looked at the code yet, but after reading your message I have >> a simple question - gow iss this going to work with SSL? If you're only >> passing a file descriptor, that does not seem to be sufficient for the >> backends to do crypto (that requires the SSL stuff from Port). >> >> Maybe I'm missing something and it already works, though ... >> >> >> regards >> > Ooops, I missed this aspect with SSL. Thank you. > New version of the patch which correctly maintain session context is > attached. > Now each session has its own allocator which should be used instead > of TopMemoryAllocator. SSL connections work now. >
OK. I've looked at the code, but I have a rather hard time understanding it, because there are pretty much no comments explaining the intent of the added code :-( I strongly suggest improving that, to help reviewers. The questions I'm asking myself are mostly these: 1) When assigning a backend, we first try to get one from a pool, which happens right at the beginning of BackendStartup. If we find a usable backend, we send the info to the backend (pg_send_sock/pg_recv_sock). But AFAICS this only only happens at connection time, right? But it your initial message you say "Rescheduling is done at transaction level," which in my understanding means "transaction pooling". So, how does that part work? 2) How does this deal with backends for different databases? I don't see any checks that the requested database matches the backend database (not any code switching the backend from one db to another - which would be very tricky, I think). 3) Is there any sort of shrinking the pools? I mean, if the backend is idle for certain period of time (or when we need backends for other databases), does it get closed automatically? Furthermore, I'm rather confused about the meaning of session_pool_size. I mean, that GUC determines the number of backends in the pool, it has nothing to do with sessions per se, right? Which would mean it's a bit misleading to name it "session_..." (particularly if the pooling happens at transaction level, not session level - which is question #1). When I've been thinking about adding a built-in connection pool, my rough plan was mostly "bgworker doing something like pgbouncer" (that is, listening on a separate port and proxying everything to regular backends). Obviously, that has pros and cons, and probably would not work serve the threading use case well. But it would have some features that I find valuable - for example, it's trivial to decide which connection requests may or may not be served from a pool (by connection to the main port or pool port). That is not to say the bgworker approach is better than what you're proposing, but I wonder if that would be possible with your approach. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
