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

Added mechanism so that begin, commit, rollback can be used outside of with-transaction #84

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

beberman
Copy link

There is sometimes a need to push the semantics of transactions to higher levels in a code stack. In particular in my code there is a an object cache and then an interface on that cache that sits above the underlying database driver code. This cache has a need to wrap transactions around multiple connections to multiple schemas. Further some of the code supports database transactions and other code does not. To make this polymorphic the being/ commit/ rollback code needs to work outside of the macro with-transaction. As noted in a prior note the current code only supports transactions within with-transaction or with-savepoint. The core in this refactor is the global dynamic variable transaction-state which is used throughout the driver code through the calls to in-transaction and get-transaction-state. Its set within a dynamic let context in with-savepoint and %with-transaction. This doesn't work if the semantics are separated. Therefore I had to introduce a start-transaction and end-transaction function to set things outside of these macros. End-transaction calls an internal function remove-transaction-state. I suspect for multi-threading the operations on transaction-state need to be protected with a thread lock. Finally I added clear-transaction-state to enable debugging cleanup or server issue resolution.

There are probably better ways to support this functionality. The code was fairly subtle.

beberman added 3 commits April 9, 2024 09:55
In oder to move the start-transaction, commit, rollback to a higher
level had to create a separate start-transaction function which sets
the *transaction-state* and then calls the begin-transaction at the
driver level.  This means there also has to be an end-transaction
which clears the *transaction-state* because the macros rely on the
dynamic variable structure of *transaction-state*.  Also fixed the
release-savepoint which was sending the wrong message.
Because the driver code depends on the dynamic global
variable *transaction-state* there is a need to provide a clear in
case the state gets corrupted.  Begin/end act on this state which
makes the commit and rollback code work correctly.
@fukamachi
Copy link
Owner

I like the idea, and adding the global version of those functions is fine to me.
Is this code still in progress?

@beberman
Copy link
Author

beberman commented May 14, 2024 via email

@beberman
Copy link
Author

I found a potential issue that I am unable to find a clear work around. The issue is that if there are nested transactions when an error occurs within a savepoint the code for mysql has an error handler macro called with-error-handler. This code calls cl-mysql-system:release which ultimately does a return-or-close. What I see happening in the traces is that when an error occurs release gets called twice. It only gets called once on a successful insert/update. The net is that when the code goes to rollback a savepoint the savepoint is in the list within the transaction-state code but MySQL no longer knows about that savepoint so it through the error "DB Error: SAVEPOINT savepoint does not exist (Code: 1305)". I think what is happening here is that the connection has been closed which results in the transaction being closed and automatically rolledback, thus clearing all the savepoints. I've been unable to determine how to adjust the error handler to handle this issue.

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

Successfully merging this pull request may close these issues.

2 participants