Skip to content

BrokenPipeError or struct.error does not invalidate connection in SQLAlchemy #6099

New issue
New issue
Closed
Closed
BrokenPipeError or struct.error does not invalidate connection in SQLAlchemy#6099
Labels
external driver issuesthe issue involves a misbehavior on the part of the DBAPI itself, probably not SQLAlchemy
Milestone
 
1.4.x
@hbusul

Description

@hbusul
hbusul
opened on Mar 19, 2021
Contributor

We are using pg8000 with SQLAlchemy in a docker environment. Everything works fine but while testing the application, whenever I restart the database(Postgresql) to simulate some error, the flask application cannot connect to the database. I'm getting either BrokenPipeError or struct.error. I found in the README of pg8000 this behaviour is mentioned. We tried to use SQLAlchemy's pre-pinging to deal with disconnects. But raising these exceptions seems to not invalidate the connection. Here is a sample log

2021-03-16 13:55:19,928 DEBUG sqlalchemy.pool.impl.QueuePool Connection <pg8000.legacy.Connection object at 0x7ff0bbac4150> checked out from pool
2021-03-16 13:55:19,928 DEBUG sqlalchemy.pool.impl.QueuePool Pool pre-ping on connection <pg8000.legacy.Connection object at 0x7ff0bbac4150>
[2021-03-16 13:55:19,929] ERROR in app: Exception on /namespaces/ [GET]
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/usr/local/lib/python3.7/site-packages/flask_restx/api.py", line 375, in wrapper
    resp = resource(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/flask/views.py", line 89, in view
    return self.dispatch_request(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/flask_restx/resource.py", line 44, in dispatch_request
    resp = meth(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/flask_httpauth.py", line 382, in decorated
    )(f)(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/flask_httpauth.py", line 149, in decorated
    user = self.authenticate(auth, password)
  File "/usr/local/lib/python3.7/site-packages/flask_httpauth.py", line 220, in authenticate
    return self.verify_password_callback(username, client_password)
  File "./core/model.py", line 104, in verify_password
    rci. Suspendisse ac accumsan ipsum. Suspendisse auctor venenatis magna
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 2684, in first
    return self.limit(1)._iter().first()
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 2771, in _iter
    execution_options={"_sa_orm_load_options": self.load_options},
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/session.py", line 1652, in execute
    conn = self._connection_for_bind(bind, close_with_result=True)
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/session.py", line 1503, in _connection_for_bind
    engine, execution_options
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/session.py", line 738, in _connection_for_bind
    conn = bind.connect()
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 3066, in connect
    return self._connection_cls(self, close_with_result=close_with_result)
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 91, in __init__
    else engine.raw_connection()
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 3145, in raw_connection
    return self._wrap_pool_connect(self.pool.connect, _connection)
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 3112, in _wrap_pool_connect
    return fn()
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 301, in connect
    return _ConnectionFairy._checkout(self)
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 781, in _checkout
    result = pool._dialect.do_ping(fairy.connection)
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 648, in do_ping
    cursor.execute(self._dialect_specific_select_one)
  File "/usr/local/lib/python3.7/site-packages/pg8000/legacy.py", line 175, in execute
    self._c.execute_unnamed("begin transaction")
  File "/usr/local/lib/python3.7/site-packages/pg8000/core.py", line 735, in execute_unnamed
    self.handle_messages(context)
  File "/usr/local/lib/python3.7/site-packages/pg8000/core.py", line 895, in handle_messages
    code, data_len = ci_unpack(self._read(5))
struct.error: unpack_from requires a buffer of at least 5 bytes
2021-03-16 13:55:19,933 DEBUG sqlalchemy.pool.impl.QueuePool Connection <pg8000.legacy.Connection object at 0x7ff0bbac4150> being returned to pool
2021-03-16 13:55:19,933 DEBUG sqlalchemy.pool.impl.QueuePool Connection <pg8000.legacy.Connection object at 0x7ff0bbac4150> rollback-on-return

I created a minimal example to reproduce the issue.

git pull https://github.com/hbusul/sqlalchemy_pg800_pre_ping

docker-compose up --build

After 10 secs, from another terminal docker-compose restart database

I already opened a ticket in pg8000

I guess these exceptions should be wrapped in other exceptions. My question is that which exception would be suitable for the issue?

Versions

  • OS: Linux
  • Python: 3.7
  • SQLAlchemy: 1.3.5
  • Database: Postgresql
  • DBAPI: pg8000?

Activity

hbusul
added
requires triageNew issue that requires categorization
on Mar 19, 2021
zzzeek
added
external driver issuesthe issue involves a misbehavior on the part of the DBAPI itself, probably not SQLAlchemy
and removed
requires triageNew issue that requires categorization
on Mar 19, 2021
zzzeek
added this to the 1.3.x milestone on Mar 19, 2021
zzzeek

zzzeek commented on Mar 19, 2021

@zzzeek
zzzeek
on Mar 19, 2021
Member

this is very poor behavior on the part of pg8000 and IMO it should be fixed on their end. When the database connection has died, the appropriate DBAPI error to raise is OperationalError.

zzzeek

zzzeek commented on Mar 19, 2021

@zzzeek
zzzeek
on Mar 19, 2021
Member

can confirm same exact error here just running the script and restarting PG


   self._c.execute_unnamed(self, "begin transaction")
  File "/home/classic/.venv3/lib/python3.8/site-packages/pg8000/core.py", line 1214, in execute_unnamed
    self.handle_messages(cursor)
  File "/home/classic/.venv3/lib/python3.8/site-packages/pg8000/core.py", line 1377, in handle_messages
    code, data_len = ci_unpack(self._read(5))
struct.error: unpack_from requires a buffer of at least 5 bytes for unpacking 5 bytes at offset 0 (actual buffer size is 0)

Any reason you aren't able to use psycopg2?

hbusul

hbusul commented on Mar 19, 2021

@hbusul
hbusul
on Mar 19, 2021
ContributorAuthor

I agree that this should be fixed on pg8000, I wanted to verify the correct behavior.

We started using it a bit while ago and it works quite ok so we did not feel the necessity to change it. But if fixing this issue as easy as wrapping that with OperationalError, I guess we can wait for the fix and everyone else using pg8000 would benefit as well.

Thanks for verifying the correct behaviour!

hbusul
mentioned this on Mar 19, 2021
CaselIT
modified the milestones: 1.3.x, 1.4.x on Mar 25, 2021
hbusul

hbusul commented on Mar 28, 2021

@hbusul
hbusul
on Mar 28, 2021 · edited by zzzeek
ContributorAuthor

@zzzeek in pg8000 now the exceptions are wrapped in InterfaceError. And I guess the reason is that the native interface only has InterfaceError and DatabaseError. But currently, those changes still do not invalidate the connection and I guess that's not due to type of the exception but due to exception message. I found in documentation:

  • As the Python DBAPI provides no standard system for determining the nature of an exception, all SQLAlchemy dialects include a system called is_disconnect() which will examine the contents of an exception object, including the string message and any potential error codes included with it, in order to determine if this exception indicates that the connection is no longer usable.

And I found pg8000.py

    def is_disconnect(self, e, connection, cursor):
        return "connection is closed" in str(e)

I guess this string never appears in pg8000 repository. And given the fix commit, uses network error on read, network error on write and network error on flush, @tlocke suggested that we can use

def is_disconnect(self, e, connection, cursor):
    return str(e).startswith("network error")

I already tried changing those error messages in pg8000 and it does work.
@zzzeek Do you think changing this function is OK?

zzzeek

zzzeek commented on Mar 28, 2021

@zzzeek
zzzeek
on Mar 28, 2021
Member

@zzzeek in pg8000 now the exceptions are wrapped in InterfaceError.

meaning, they fixed the issue ? did they release?

And I guess the reason is that the native interface only has InterfaceError and DatabaseError.

it's fine, lots of DBAPIs use InterfaceError and OperationalError interchangeably, the pep is much too vague

I guess this string never appears in pg8000 repository. And given the fix commit, uses network error on read, network error on write and network error on flush, @tlocke suggested that we can use

I already tried changing those error messages in pg8000 and it does work.
@zzzeek Do you think changing this function is OK?

yes this is the appropriate action to take on the SQLAlhcemy side, we can also assert that the exception object is an instance of InterfaceError. we can accept a PR for this now that pg8000 has defined how they will handle this.

tlocke

tlocke commented on Mar 28, 2021

@tlocke
tlocke
on Mar 28, 2021
Contributor

Hi all, just to confirm, I've just done a new release of pg8000 (1.19.0) that wraps all network error exceptions in an InterfaceError with a message that starts, network error. The original exception is set as the cause.

hbusul
added a commit that references this issue on Mar 28, 2021

update pg8000 dialect is_disconnect method Fixes: sqlalchemy#6099

cd58836
hbusul
mentioned this on Mar 28, 2021

17 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    external driver issuesthe issue involves a misbehavior on the part of the DBAPI itself, probably not SQLAlchemy

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @zzzeek@tlocke@sqla-tester@CaselIT@hbusul

        Issue actions
          BrokenPipeError or struct.error does not invalidate connection in SQLAlchemy · Issue #6099 · sqlalchemy/sqlalchemy