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

Getting connection busy issue on using latest version z3c.sqlalchemy #21

Closed
Buvi1234 opened this issue Apr 1, 2024 · 7 comments · Fixed by #23
Closed

Getting connection busy issue on using latest version z3c.sqlalchemy #21

Buvi1234 opened this issue Apr 1, 2024 · 7 comments · Fixed by #23

Comments

@Buvi1234
Copy link

Buvi1234 commented Apr 1, 2024

Hi,

Upon observing the commit fb7d533
The mentioned above changes done to _createEngine method of ZopeWrapper, it seems that the self._session assignment is a single session object which is a thread local session, when on call upon the session function https://github.com/zopefoundation/z3c.sqlalchemy/blame/2.1.1/src/z3c/sqlalchemy/base.py#L103 to get session, for every web request call, due to Zope is a multi threaded framework, the same session object(thread local session)is attached. Consequently,the request has to wait until the previous request completes and releases the lock acquired by the first request (threading.lock by SQLAlchemy) for other requests to communicate with the database, even though the default connection pool set by SQLAlchemy is 5, only one connection becomes active.

Is it possible to try like this,

@property
def session(self):
    return self._session()


def _createEngine(self):
    self._engine = create_engine(self.dsn, **self.engine_options)
    self._sessionmaker = scoped_session(sessionmaker(bind=self._engine,
                                                     autocommit=not self.transactional,
                                                     twophase=self.twophase,
                                                     autoflush=True,
                                                     **self.session_options))
    register(self._sessionmaker, **self.extension_options)
    self._session = self._sessionmaker

Here instead of returning the single session(thread local session, https://github.com/zopefoundation/z3c.sqlalchemy/blob/master/src/z3c/sqlalchemy/base.py#L143), we can return the scoped_session. Upon calling the scoped_session() function, it will return the same session object on every web request call, else removing the self._session assignment and directly instantiate scoped session object in the session function like how it was in the previous version would also help eg(https://github.com/zopefoundation/z3c.sqlalchemy/blob/2.0/src/z3c/sqlalchemy/base.py#L105), and the connection pool, which is defined, will manage the idle and active sessions, thus eliminating the connection busy issue.

The above code is for example(as per observation), is there any other approach from your side?

MSSQL: 17 driver
PYODBC: 4.0.35
PYTHON: 3.10.9
Zope: 5.8.3
Sqlachemy: 1.4.51
zope.sqlalchemy: 3.1

@icemac
Copy link
Member

icemac commented Apr 3, 2024

Hi @Buvi1234, thank you for reporting this issue. Could you please create a pull request, to see in the diff what you are suggesting?

@Buvi1234
Copy link
Author

Buvi1234 commented Apr 3, 2024

hi @icemac,
https://github.com/zopefoundation/z3c.sqlalchemy/pull/22/files --> please do look into this

ale-rt added a commit that referenced this issue Apr 4, 2024
Partially revert commit fb7d533
which caused the sqlalchemy session instance to be persisted as an instance attribute.
Fixes #21
@Buvi1234
Copy link
Author

Buvi1234 commented Apr 4, 2024

hi @icemac, @ale-rt,

once the review is done, can you guys delegate this change to z3c.sqlalchemy version 2.1.1

@dataflake
Copy link
Member

What do you mean "delegate this change to z3c.sqlalchemy version 2.1.1"? A released version is not getting "updates". The changes will flow into 2.1.2 or 2.2.

@Buvi1234
Copy link
Author

Buvi1234 commented Apr 4, 2024

ok @dataflake

@icemac icemac closed this as completed in #23 Apr 9, 2024
icemac pushed a commit that referenced this issue Apr 9, 2024
…23)

Partially revert commit fb7d533
which caused the sqlalchemy session instance to be persisted as an instance attribute.
Fixes #21
@icemac
Copy link
Member

icemac commented Apr 9, 2024

After #24 is merged, I can cut a release with the fix for this issue.

@icemac
Copy link
Member

icemac commented Apr 9, 2024

Just released in https://pypi.org/project/z3c.sqlalchemy/2.2/.

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