Issue: Postgres - server down/up, connect with reuse, but error connection already close
Author: wibimaster
State: closed
Created: 2019-10-10 14:14:23

Hi, 

Sorry for the long title, I don't know how to summarize it :x

**Environment:**
* PostgreSQL server
* aiohttp project with Peewee as ORM
* Python 3.7

**How to reproduce**

* Start the aiohttp project, with a "db.connect(reuse=True)" (return True)
* Kill the DB server
* Restart the DB server
* Attempts a request to aiohttp controller
* It systematically try a "db.connect(reuse=True)", that return False (indicates that the DB connection it always established, which indeed is not true)
* It fails to execute a query with "peewee.InterfaceError: connection already closed"
* **EDIT** Sorry I missed: first error that occured is 
```
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/peewee.py", line 3005, in execute_sql
    cursor.execute(sql, params or ())
psycopg2.OperationalError: server closed the connection unexpectedly
	This probably means the server terminated abnormally
	before or while processing the request.

```

**What's expected**

In case of DB system disconnection, "db.open(reuse=True)" should return True and effectively retry a connection (it doesn't detect the connection close)

Is this an expected behavior ?

Thanks !

> What's expected

Read the description of the parameter: http://docs.peewee-orm.com/en/latest/peewee/api.html#Database.connect

Sure, I've read it:

Parameters: reuse_if_open (bool) – Do not raise an exception if a connection is already opened.
Returns: whether a new connection was opened.

In my case, the connection isn't opened at this time.
It was, but the connection was lost. So the parameters should returns "True" and "db.connect" should established a new connection, nop ?



The connection is actually managed by the database driver, e.g. psycopg2 or pymysql or whatever. Peewee obtains a ref to the connection and keeps it in a threadlocal by default - so the `reuse_if_open` pertains to the internal state of peewee - not the actual socket or whatever managed by the driver.

Blindly reconnecting has ramifications, e.g. transactional state getting messed up, most importantly. So by default, peewee won't reconnect if the db driver raises an error indicating the socket was closed / etc.

See #2007 for some discussion.

You could adopt the reconnect stuff from the `playhouse.shortcuts.ReconnectMixin` but I don't recommend it.

I have read #2007 ; I forgot to say that I'm using "PooledPostgresqlExtDatabase()".
You said : 

> the pool is slightly different, because that actually checks the "liveness" of connections

Not the case here ? How / When do you check this liveness ?

**EDIT**

If I follow the `playhouse/pool.py`, I see the `_can_reuse()` method that check the transaction status ; if we have an `TRANSACTION_STATUS_INERROR`, the connection object should be `reset()`.
The `reset()` method seems to make the connection `closed` status to `True`, so the `reuse_if_open` shouldn't return `False` because the Peewee internal state should know that the connection is lost.

Am I wrong ?

Depends on where you get the error. Most likely, if you're following good practices of calling connect() when you start processing and close() when you're done, then the database reset is happening while the connection is sitting in the pool's list of available connections.

So if you look at the pool's `_connect` method, you can see that before returning a connection to the application, it checks `if self._is_closed(conn)` to make sure we aren't returning a closed connection. The logic for postgres is:

```python
class _PooledPostgresqlDatabase(PooledDatabase):
    def _is_closed(self, conn):
        if conn.closed:
            return True

        txn_status = conn.get_transaction_status()
        if txn_status == TRANSACTION_STATUS_UNKNOWN:
            return True
        elif txn_status != TRANSACTION_STATUS_IDLE:
            conn.rollback()
        return False
```

Connections that appear to be closed are simply discarded and garbage-collected, they will not be returned to the user.

If you can provide a minimal script to replicate the issue, with a line indicating where you restart the server, that might help indicate where exactly the problem is occurring.

That being said, if the connection is checked-out and "in-use" by your application during the restart, there's nothing peewee will do about it.

Looking into this a bit more,

It seems to me that `conn.get_transaction_status()` does not indicate any problems if the connection to the server is severed.

I put together the following:

```python
from peewee import *
from playhouse.pool import PooledPostgresqlDatabase


db = PooledPostgresqlDatabase('peewee_test', user='postgres')


# Create a connection in the pool's list of available conns.
db.connect()
db.close()

input('Restart server now')

db.connect()
db.execute_sql('select 1').fetchall()
db.close()
```

And it raises the following when attempting to execute the "select 1":

```
psycopg2.errors.AdminShutdown: terminating connection due to unexpected postmaster exit
server closed the connection unexpectedly
	This probably means the server terminated abnormally
	before or while processing the request.
```

I looked closely, though, and `conn.get_transaction_status()` is returning `0` indicating no problems. So my sense is that psycopg2 just doesn't detect this particular scenario.

For what it's worth, it seems that aiopg is using similar logic to peewee: https://github.com/aio-libs/aiopg/blob/master/aiopg/pool.py#L229

Thanks for the investigations ; so finally, psycopg2 let the transaction status to Idle even if it's closed :/

That's a bad news..

Can you confirm a last thing for me ?

If I run a aiohttp server, but don't start any connection ; when a client comes, I open a connection, do some DB requests, return a result and close the connection.
If I don't use the pool system, each client creates a new DB connection. That could be massive number of opened connections if I receive a lot of traffic.
But if I use the pool system, a client could close a connection used by another client at the same time, right ?

Should I rely on pgpool instead ?

Thanks !


> so finally, psycopg2 let the transaction status to Idle even if it's closed

That's what it looked like to me.

Regarding the pool... The idea is that creating and closing connections has a specific cost. To mitigate that cost, the pool allows you to have some number of connections that will get reused. When you call connect() it will check-out a connection from the pool (creating one if none available). When you're done you close() and the connection returns to the pool of available connections.

In a multi-threaded environment it is possible for two separate threads to be running concurrently (~ish cuz gil). Peewee tracks connections using a threadlocal on the database instance, which is in turn what your model instances use to execute queries. So each thread will have its own connection from the pool and it isn't possible for one thread to close the connection of another thread.

For asyncio I'm not sure what you should do about this, as I'm not sure the threadlocal model will work. You'll have to test it out.

> Should I rely on pgpool instead ?

Did you have the chance to come up with a solution for your issue? I'm facing the exactly same problem in a asyncio app which  calls for peewee pooled...  

Peewee database connections are stored in a thread-local and are not async context-aware (unless you use gevent), so you may get unexpected behavior depending on how you use your connections.