Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlalchemy.exc.ResourceClosedError: This Connection is closed #97

Open
ansgarstrother opened this issue Dec 11, 2019 · 9 comments
Open
Assignees
Labels

Comments

@ansgarstrother
Copy link

We are using from cockroachdb.sqlalchemy import run_transaction in a multi-threaded python application.

Occasionally we are receiving the following error that sqlalchemy.exc.ResourceClosedError: This Connection is closed

Based on my research I'm not sure if this is related to the application being multi-threaded or the need for connection disconnect error handling.

Would someone be able to confirm that run_transaction is, in fact, thread-safe?

If it is thread-safe is there a suggested strategy for implementing connection disconnect?
https://docs.sqlalchemy.org/en/13/core/pooling.html#pool-disconnects

Also, would this solution be something that should be added into run_transaction?

@awoods187
Copy link

@solongordon or @rafiss thoughts here?

@rafiss rafiss self-assigned this Dec 16, 2019
@rafiss
Copy link
Contributor

rafiss commented Dec 16, 2019

@ansgarstrother Are you re-using the same sqlalchemy.engine.Connection as the argument to run_transaction? That is not thread-safe, as noted in the SQLAlchemy docs.

If you don't think that's the issue, could you provide a code sample that reproduces the problem?

@ansgarstrother
Copy link
Author

ansgarstrother commented Dec 18, 2019

We are using the following:

from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
from cockroachdb.sqlalchemy import run_transaction

engine = create_engine(sys.argv[1])
session_maker = sessionmaker(bind=engine)

run_transaction(session_maker, databaseFunctionToRun)

As we increase the number of interactions in this service this issue is appearing regularly. I look forward to your comments.

@ansgarstrother
Copy link
Author

ansgarstrother commented Jan 2, 2020

So I've done some more work on this and run_transaction I've confirmed on my end that run_transaction does not support concurrency.

I confirmed this with an extremely naive basic proof of concept where invalidated each connection so no connection is reused.

@event.listens_for(engine, "checkin")
def receive_checkin(dbapi_connection, connection_record):
    connection_record.invalidate()
    connection_record.connection = None

This removed the error. Of course this isn't a production solution due to the overhead of creating a new connection each time.

For a production-grade solution, I started to look into: https://docs.sqlalchemy.org/en/13/orm/contextual.html

from sqlalchemy.orm import scoped_session
from sqlalchemy.orm import sessionmaker

session_factory = sessionmaker(bind=some_engine)
Session = scoped_session(session_factory)

run_transaction(Session, databaseFunctionToRun)

I extended run_transaction to support sqlalchemy.orm.scoped_session but then ran into a save point error.

engine = create_engine(sys.argv[1])
s_session = scoped_session(sessionmaker(bind=engine, autocommit=True))
session = s_session()
assert(isinstance(session, sqlalchemy.orm.Session))
return _txn_retry_loop(session, callback)

(psycopg2.errors.FeatureNotSupported) unimplemented: SAVEPOINT not supported except for cockroach_restart\nHINT: Retryable transactions with arbitrary SAVEPOINT names can be enabled with SET force_savepoint_restart=true\nSee: https://github.com/cockroachdb/cockroach/issues/10735\n'

Has anyone looked into this more previously? Is there already any work that has been done towards a thread save implementation of run_transaction?

Multi-threading is critical for our application thank you in advance for any help or suggestions. @solongordon @rafiss @awoods187

@ansgarstrother
Copy link
Author

Just checking back on this. Any thoughts?

@rafiss
Copy link
Contributor

rafiss commented Jan 21, 2020

@ansgarstrother Thanks for following up and providing the proof-of-concept. We have not been able to prioritize this work yet. (I myself haven't had time to repro it locally yet.) We don't have any other prior work on run_transaction concurrency as far as I'm aware.

One thing that may help us prioritize this is if you could share how critical this bug is for your application. How often does it occur and under how much load?

Also, if you can give a more robust code sample that we can run to get the error ourselves, that would help a lot.

A final suggestion, until we are able to prioritize this, is to check if any other users have encountered this error in our community Slack room: https://cockroa.ch/slack

@rafiss rafiss added the bug label Apr 23, 2020
@swazuz
Copy link

swazuz commented Aug 31, 2021

Hi, I have a similar issue and I wonder if it was solved at any point or investigated since we are in August 2021 now.
All efforts are much appreciated.

Thanks,
Aya

@arLevi
Copy link

arLevi commented Feb 23, 2023

Wanted to comment that it happens to me when using Python Tornado, where the server is always up, sometimes functions don't work and it's still up and running ( using the "watch" feature ) which makes it loose the connection in the background - but the front-end keeps going like nothing happened.

I don't know how to tell SQLAlchemy when you lost the connection - just bring it back

@gordthompson
Copy link
Collaborator

@arLevi - The following section of the SQLAlchemy documentation may be helpful:

https://docs.sqlalchemy.org/en/20/core/pooling.html#dealing-with-disconnects

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants