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

Refine Connection.setTransactionIsolationLevel documentation #265

Closed
rusher opened this issue Feb 7, 2022 · 2 comments
Closed

Refine Connection.setTransactionIsolationLevel documentation #265

rusher opened this issue Feb 7, 2022 · 2 comments
Labels
for: team-attention An issue we need to discuss as a team to make progress type: documentation A documentation update
Milestone

Comments

@rusher
Copy link

rusher commented Feb 7, 2022

While reading the specification, I'm not completly sure what are the exact behavior of Connecton.setTransactionIsolationLevel :
must transaction isolation be the default state when creating a transaction, or must that apply only for the next transaction.

JDBC Connection.setTransactionIsolation(int) indicates:

Attempts to change the transaction isolation level for this Connection object to the one given

Every transaction will default to newly the transaction isolation value.

R2DBC Connection.setTransactionIsolationLevel(IsolationLevel) :

Configures the isolation level for the current transaction.

https://github.com/r2dbc/r2dbc-spi/blob/main/r2dbc-spec/src/main/asciidoc/transactions.adoc doesn't clarify that point.

This differ from JDBC, and I think that's an error, expecting that Connection.beginTransaction(TransactionDefinition) to set a transaction isolation for the scope of the transaction, and Connection.setTransactionIsolationLevel(isolationLvl) to set default transaction level for the connection. (postgresql implementation does set isolation for connection, not only next transaction)

Could you confirm the expected behavior ?

@mp911de
Copy link
Member

mp911de commented Feb 14, 2022

Thanks for bringing this up. The description is indeed a bit imprecise as setting the isolation level applies the isolation level to the current connection and the state in which it resides.

When a transaction is in progress, it would change the isolation level for the current transaction (although we define vendor-specific behavior in the spec). When the connection has no ongoing transaction (auto-commit mode or auto-commit disabled and no transaction started or the previous transaction was cleaned up), then the isolation level is applied to each transaction that is started.

It is not specified how setting the isolation level during a transaction affects new transactions after the current transaction is stopped. It seems that some databases reset the isolation level to the level that was set before the transaction was started.

Going forward, calling Connection.begin(TransactionDefinition) should ideally apply transaction attributes such as the isolation level only to the transaction that is being started and it should not leak these attributes to every transaction that is started later.

For example, the SQL Server driver captures the currently set isolation level on Connection.begin(TransactionDefinition) to set it on commit/rollback.

Does that make sense?

@mp911de mp911de added the for: team-attention An issue we need to discuss as a team to make progress label Feb 14, 2022
@rusher
Copy link
Author

rusher commented Feb 16, 2022

Agree : to resume :

  • for Connection.setTransactionIsolationLevel if not in a transaction, the isolation level must be the new default isolation for all new transactions. Vendor specific: If in a transaction, can change the current transaction isolation level (MySQL/MariaDB doesn't support this kind of feature)
  • for Connection.beginTransaction isolation level must apply to the new transaction only.

@mp911de mp911de added the type: documentation A documentation update label Feb 16, 2022
@mp911de mp911de modified the milestones: 1.0.0.RELEASE, 0.9.2.RELEASE Feb 16, 2022
mp911de added a commit that referenced this issue Mar 16, 2022
[resolves #265]

Signed-off-by: Mark Paluch <mpaluch@vmware.com>
@mp911de mp911de changed the title Connection.setTransactionIsolationLevel expected behavior : session or next transaction ? Refine Connection.setTransactionIsolationLevel documentation Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention An issue we need to discuss as a team to make progress type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

2 participants