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

libsql: fix deferred txn with embedded replicas #559

Closed
wants to merge 2 commits into from

Conversation

LucioFranco
Copy link
Contributor

This fixes an issue where embedded replica based connections would have issues dispatching executes on deferred statements. This is due to sqld using sqlite3_txn_state to return the connection's state. While this works for other transactions, in a deferred transaction the connection does not change its state to be in a transaction. This is due to the nature of a deferred transaction. The problem is that the connection does set is_autocommit to false. This means that sqld is returning that it is not in a txn state which is used by clients to set its is_autocommit state. For now the work around is to track when the client is in a DEFERRED transaction and to handle remote state transition's in such a way that it ignores the value from the server if it is currently in the deferred state.

This fixes an issue where embedded replica based connections would have
issues dispatching executes on deferred statements. This is due to sqld
using `sqlite3_txn_state` to return the connection's state. While this
works for other transactions, in a deferred transaction the connection
does not change its state to be in a transaction. This is due to the
nature of a deferred transaction. The problem is that the connection
does set `is_autocommit` to `false`. This means that sqld is returning
that it is not in a txn state which is used by clients to set its
`is_autocommit` state. For now the work around is to track when the
client is in a `DEFERRED` transaction and to handle remote state
transition's in such a way that it ignores the value from the server if
it is currently in the deferred state.
Copy link
Collaborator

@psarna psarna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One change is needed in the state transition code, but I have a more fundamental question.

Why would we use sqlite3_txn_state to determine autocommit mode in sqld, if we have sqlite3_get_autocommit? https://www.sqlite.org/c3ref/get_autocommit.html

libsql/src/replication/connection.rs Show resolved Hide resolved
@LucioFranco
Copy link
Contributor Author

Fixed via #577

@LucioFranco LucioFranco closed this Nov 7, 2023
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.

None yet

2 participants