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

TxDecisionMaker has been modified for READ_UNCOMMITTED Isolation Level #3788

Closed
wants to merge 4 commits into from
Closed

Conversation

fusunc
Copy link

@fusunc fusunc commented Apr 27, 2023

When Isolation level is READ_UNCOMMITTED if the transaction is not committed yet another transaction should not wait .

@katzyn
Copy link
Contributor

katzyn commented Apr 27, 2023

Hello!

I think your changes are incorrect.

Try this:

Connection conn1 = DriverManager.getConnection("jdbc:h2:mem:test");
Statement stat1 = conn1.createStatement();
stat1.execute("CREATE TABLE TEST(ID BIGINT PRIMARY KEY, V INT)");
stat1.executeUpdate("INSERT INTO TEST VALUES (1, 10), (2, 20)");
conn1.setAutoCommit(false);
Connection conn2 = DriverManager.getConnection("jdbc:h2:mem:test");
conn2.setTransactionIsolation(Connection.TRANSACTION_READ_UNCOMMITTED);
conn2.setAutoCommit(false);
Statement stat2 = conn2.createStatement();
ResultSet rs = stat1.executeQuery("SELECT * FROM TEST FOR UPDATE");
int r = 0;
while (rs.next()) {
    r++;
}
System.out.println("C1: Locked " + r + " rows");
try {
    r = stat2.executeUpdate("DELETE FROM TEST");
    System.out.println("C2: Deleted " + r + " rows");
} catch (SQLTimeoutException e) {
    System.out.println("C2: Timeout");
}
try {
    System.out.println("C1: Updated " + stat1.executeUpdate("UPDATE TEST SET V = V + 1") + " rows");
} catch (SQLTimeoutException e) {
    System.out.println("C1: Timeout");
}
conn1.commit();

Before your changes connection 1 is able to update previously locked rows:

C1: Locked 2 rows
C2: Timeout
C1: Updated 2 rows

After your changes it isn't possible any more:

C1: Locked 2 rows
C2: Deleted 2 rows
C1: Timeout

@andreitokar
Copy link
Contributor

andreitokar commented Apr 30, 2023

I think that I fail to understand the very purpose of this PR.
Is it an attempt to improve concurrency, as it sounds from

if the transaction is not committed yet another transaction should not wait

But with current multi-version implementation it is not waiting to get the value, just gets committed value, which existed before that not yet committed change, not a current uncommitted value.
READ_UNCOMMITTED allows (but not mandates) to read uncommitted value for the sake of possible performance gains. But is this case - there is nothing to gain here!
That is why implementation of READ_UNCOMMITTED in H2 has practically no differences from READ_COMMITTED.

@fusunc
Copy link
Author

fusunc commented May 5, 2023

We found the problem of not handling transactions correctly in our tests, thank you for your time.

@fusunc fusunc closed this May 5, 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

3 participants