Re: Designing a better connection pool for psycopg3

On Mon, 18 Jan 2021 at 15:05, Magnus Hagander <magnus@hagander.net> wrote:
>
> On Mon, Jan 18, 2021 at 2:50 PM Daniele Varrazzo
> <daniele.varrazzo@gmail.com> wrote:
> >
> > On Mon, 18 Jan 2021 at 14:13, Karsten Hilbert <Karsten.Hilbert@gmx.net> wrote:
> >
> > > I would strongly advise against making sys.exit() the default
> > > for pool.terminate() unless I misunderstand something.
> >
> > How would you terminate the program if a maintenance thread, not the
> > main one, thinks that the program is not in working state?
>
> Why would it be OK for a maintenance thread to terminate the program
> at all? And certainly by default?
>
> Wouldn't the reasonable thing to do be to flag the pool as broken, and
> then just stop trying. Then whenever the application makes an attempt
> to use the pool, it can he thrown an exception saying that this
> happened.

I'm trying to imagine what happens in a case such as a network
partition or reconfiguration, and the app server doesn't see the
database anymore. This node is arguably broken.

If the reconnection thread fails to obtain new connections, and the
ones currently in the pool are discarded as detected broken,
eventually the pool is depleted.

The requesting threads (e.g. web requests handlers) would try to
obtain a connection, time out after e.g. 30 seconds, and receive an
error. The error would result in a 500 for the user, probably a sentry
exception and log in a file, but the program would likely not die.

The program could remain in this condition for an arbitrary long time,
until someone notices by looking at the logs and scratches their head
to understand how to fix the problem.

If the program dies, its manager would try to restart it, insisting
until the configuration makes the database visible again. A service
down in a crash loop is more visible than a service up and running but
not serving.

Anyway I appreciate that the default of terminating a program is
probably too aggressive. So I would remove the `terminate()` function
and base implementation and leave a `connection_failed()` handler,
with a default no-op implementation, which people preferring their
program to terminate can subclass (with `sys.exit(1)` or whatever
termination strategy they find useful).

-- Daniele