diff --git a/libsql-itests/src/test/java/com/github/magnusp/libsql/itest/TransactionLifecycleIT.java b/libsql-itests/src/test/java/com/github/magnusp/libsql/itest/TransactionLifecycleIT.java index dbf4b69..04b205e 100644 --- a/libsql-itests/src/test/java/com/github/magnusp/libsql/itest/TransactionLifecycleIT.java +++ b/libsql-itests/src/test/java/com/github/magnusp/libsql/itest/TransactionLifecycleIT.java @@ -19,12 +19,15 @@ public class TransactionLifecycleIT extends SqldContainerBase { + /** sqld expires an idle Hrana stream after ten seconds. */ + private static final long STREAM_IDLE_TIMEOUT_MILLIS = 12_000; + private String getDriverJdbcUrl() { return "jdbc:libsql:" + getHttpUrl(); } @Test - void deferredTransactionBeginsEagerlyWithoutHoldingAWriteLock() throws Exception { + void deferredTransactionDoesNotHoldAWriteLock() throws Exception { String url = getDriverJdbcUrl(); try (Connection lockHolder = DriverManager.getConnection(url); Connection writer = DriverManager.getConnection(url)) { @@ -43,21 +46,23 @@ void deferredTransactionBeginsEagerlyWithoutHoldingAWriteLock() throws Exception } @Test - void immediateTransactionBeginsEagerlyAndHoldsTheWriteLock() throws Exception { + void immediateTransactionHoldsTheWriteLockFromTheFirstStatement() throws Exception { String url = getDriverJdbcUrl(); - try (Connection lockHolder = DriverManager.getConnection(url)) { + try (Connection lockHolder = DriverManager.getConnection(url); + Statement statement = lockHolder.createStatement()) { createTable(lockHolder, "immediate_lock"); LibsqlConnection libsql = lockHolder.unwrap(LibsqlConnection.class); libsql.setTransactionMode(LibsqlConnection.TransactionMode.IMMEDIATE); lockHolder.setAutoCommit(false); + statement.executeUpdate("INSERT INTO immediate_lock (id) VALUES (100)"); assertWriteIsBlockedUntilReleased(url, lockHolder, "immediate_lock", 1); } } @Test - void commitBeginsTheNextTransactionAndKeepsTheConfiguredLock() throws Exception { + void commitEndsTheTransactionAndTheNextStatementReacquiresTheConfiguredLock() throws Exception { String url = getDriverJdbcUrl(); try (Connection lockHolder = DriverManager.getConnection(url); Statement statement = lockHolder.createStatement()) { @@ -69,10 +74,68 @@ void commitBeginsTheNextTransactionAndKeepsTheConfiguredLock() throws Exception lockHolder.commit(); + // The next transaction, and its IMMEDIATE lock, begins on the next statement. + statement.executeUpdate("INSERT INTO commit_rebegin (id) VALUES (101)"); + assertWriteIsBlockedUntilReleased(url, lockHolder, "commit_rebegin", 1); } } + @Test + void aStatementAndCommitAfterAnIdlePeriodLongerThanTheStreamTimeoutSucceed() throws Exception { + String url = getDriverJdbcUrl(); + try (Connection connection = DriverManager.getConnection(url)) { + createTable(connection, "idle_pending_begin"); + + connection.setAutoCommit(false); + connection.commit(); + Thread.sleep(STREAM_IDLE_TIMEOUT_MILLIS); + + try (Statement statement = connection.createStatement()) { + assertThat(statement.executeUpdate("INSERT INTO idle_pending_begin (id) VALUES (1)")) + .isEqualTo(1); + } + connection.commit(); + + try (Statement statement = connection.createStatement(); + ResultSet rows = statement.executeQuery("SELECT COUNT(*) FROM idle_pending_begin")) { + assertThat(rows.next()).isTrue(); + assertThat(rows.getInt(1)).isEqualTo(1); + } + } + } + + @Test + void anIdleConnectionHoldsNoStreamAfterCommit() throws Exception { + String url = getDriverJdbcUrl(); + try (Connection connection = DriverManager.getConnection(url)) { + createTable(connection, "idle_after_commit"); + + connection.setAutoCommit(false); + try (Statement statement = connection.createStatement()) { + statement.executeUpdate("INSERT INTO idle_after_commit (id) VALUES (1)"); + } + connection.commit(); + + Thread.sleep(STREAM_IDLE_TIMEOUT_MILLIS); + + try (Statement statement = connection.createStatement()) { + assertThat(statement.executeUpdate("INSERT INTO idle_after_commit (id) VALUES (2)")) + .isEqualTo(1); + assertThat(statement.executeUpdate("INSERT INTO idle_after_commit (id) VALUES (3)")) + .isEqualTo(1); + } + connection.rollback(); + connection.commit(); + + try (Statement statement = connection.createStatement(); + ResultSet rows = statement.executeQuery("SELECT COUNT(*) FROM idle_after_commit")) { + assertThat(rows.next()).isTrue(); + assertThat(rows.getInt(1)).isEqualTo(1); + } + } + } + @Test void failedCommitCanBeRolledBackAndTheConnectionStaysUsable() throws Exception { String url = getDriverJdbcUrl(); diff --git a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlConnection.java b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlConnection.java index d51c8d7..611ce7a 100644 --- a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlConnection.java +++ b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlConnection.java @@ -198,11 +198,20 @@ private int[] extractBatchResults(PipelineRespBody resp, int expectedSize) throw } private void ensureTransactionStarted() throws SQLException { - if (baton == null) { + if (beginPending()) { beginTransaction(); } } + /** + * The begin-pending state is derived rather than stored: auto-commit is off and no stream is + * open, so the next statement must begin the transaction. A separate boolean would be a second + * source of truth for a fact the baton already carries. + */ + private boolean beginPending() { + return !autoCommit && baton == null; + } + private void beginTransaction() throws SQLException { PipelineRespBody resp; try { @@ -273,18 +282,24 @@ public String nativeSQL(String sql) throws SQLException { } /** - * Matches xerial sqlite-jdbc 3.53.4.0: + * Matches the observable JDBC behaviour of xerial sqlite-jdbc 3.53.4.0 while keeping the + * transaction begin lazy: * * * + *

This diverges from xerial only in when the server sees the begin: an {@code IMMEDIATE} or + * {@code EXCLUSIVE} transaction no longer takes its write lock until the next statement. The + * protocol requires it, because Hrana over HTTP expires idle streams. + * *

Use {@link #setTransactionMode(TransactionMode)} to select {@code IMMEDIATE} or {@code * EXCLUSIVE}, which acquire the write lock when the transaction begins. */ @@ -293,11 +308,10 @@ public void setAutoCommit(boolean autoCommit) throws SQLException { checkClosed(); if (this.autoCommit == autoCommit) return; if (autoCommit) { - endTransaction("COMMIT", false); + endTransaction("COMMIT"); this.autoCommit = true; } else { this.autoCommit = false; - beginTransaction(); } } @@ -313,7 +327,7 @@ public void commit() throws SQLException { if (autoCommit) { throw new SQLException("Cannot commit when in autoCommit mode"); } - endTransaction("COMMIT", true); + endTransaction("COMMIT"); } @Override @@ -322,26 +336,20 @@ public void rollback() throws SQLException { if (autoCommit) { throw new SQLException("Cannot rollback when in autoCommit mode"); } - endTransaction("ROLLBACK", true); + endTransaction("ROLLBACK"); } - private void endTransaction(String sql, boolean rebegin) throws SQLException { - if (baton == null) { - if (rebegin) { - throw new SQLException("No transaction is active"); - } + private void endTransaction(String sql) throws SQLException { + if (beginPending()) { + // No transaction has started, so there is nothing to send and nothing to close. return; } - List requests = new ArrayList<>(2); - requests.add(new StreamRequest.Execute(new Stmt(sql))); - if (rebegin) { - requests.add(new StreamRequest.Execute(new Stmt(transactionMode.beginSql()))); - } - PipelineRespBody resp; try { - resp = client.sendPipeline(new PipelineReqBody(baton, requests), baseUrl); + PipelineReqBody req = new PipelineReqBody(baton, + List.of(new StreamRequest.Execute(new Stmt(sql)))); + resp = client.sendPipeline(req, baseUrl); } catch (LibsqlException e) { throw LibsqlErrorMapper.toSQLException(e); } @@ -359,25 +367,10 @@ private void endTransaction(String sql, boolean rebegin) throws SQLException { throw new SQLException("Unexpected response: " + result); } - if (!rebegin) { - closeStreamSilently(); - return; - } - - if (resp.results().size() < 2) { - closeStreamSilently(); - throw new SQLException("Missing response for transaction begin"); - } - StreamResult begin = resp.results().get(1); - if (begin instanceof StreamResult.Error error) { - closeStreamSilently(); - throw new SQLException(error.error().message()); - } - if (!(begin instanceof StreamResult.Ok beginOk) - || !(beginOk.response() instanceof StreamResponse.Execute)) { - closeStreamSilently(); - throw new SQLException("Unexpected response: " + begin); - } + // The server committed or rolled back, so the stream has no state left worth holding. + // Closing it lets an idle connection drop its server-side stream. On an error above the + // baton stays, so the caller can retry or roll back on the same stream. + closeStreamSilently(); } @Override diff --git a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlConnectionTest.java b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlConnectionTest.java index 4a9b063..5d85fca 100644 --- a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlConnectionTest.java +++ b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlConnectionTest.java @@ -52,85 +52,213 @@ void getMetaDataIsCachedPerConnection() throws SQLException { } @Test - void disablingAutoCommitBeginsTransactionImmediately() throws SQLException { + void disablingAutoCommitDefersTheBeginUntilTheNextStatement() throws SQLException { ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); LibsqlConnection connection = new LibsqlConnection(CONFIG, client); connection.setAutoCommit(false); - assertThat(client.executedStatements()).containsExactly("BEGIN"); + assertThat(client.requests).isEmpty(); + + connection.executeStatement(new Stmt("SELECT 1")); + + assertThat(client.executedStatements()).containsExactly("BEGIN", "SELECT 1"); + assertThat(client.requests).hasSize(2); assertThat(client.requests.get(0).baton()).isNull(); + assertThat(client.requests.get(1).baton()).isEqualTo(BATON); } @ParameterizedTest @EnumSource(value = LibsqlConnection.TransactionMode.class, names = {"IMMEDIATE", "EXCLUSIVE"}) - void disablingAutoCommitUsesConfiguredTransactionMode(LibsqlConnection.TransactionMode mode) + void theDeferredBeginUsesTheConfiguredTransactionMode(LibsqlConnection.TransactionMode mode) throws SQLException { ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); LibsqlConnection connection = new LibsqlConnection(CONFIG, client); connection.setTransactionMode(mode); connection.setAutoCommit(false); + connection.executeStatement(new Stmt("SELECT 1")); String expected = switch (mode) { case IMMEDIATE -> "BEGIN IMMEDIATE"; case EXCLUSIVE -> "BEGIN EXCLUSIVE"; case DEFERRED -> "BEGIN"; }; - assertThat(client.executedStatements()).containsExactly(expected); + assertThat(client.executedStatements()).containsExactly(expected, "SELECT 1"); } @Test - void commitReBeginsInConfiguredTransactionMode() throws SQLException { + void commitDoesNotReBeginAndTheNextStatementUsesTheConfiguredMode() throws SQLException { ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); LibsqlConnection connection = new LibsqlConnection(CONFIG, client); connection.setTransactionMode("immediate"); connection.setAutoCommit(false); + connection.executeStatement(new Stmt("SELECT 1")); connection.commit(); + connection.executeStatement(new Stmt("SELECT 2")); assertThat(client.executedStatements()) - .containsExactly("BEGIN IMMEDIATE", "COMMIT", "BEGIN IMMEDIATE"); + .containsExactly("BEGIN IMMEDIATE", "SELECT 1", "COMMIT", "BEGIN IMMEDIATE", "SELECT 2"); assertThat(connection.getTransactionMode()) .isEqualTo(LibsqlConnection.TransactionMode.IMMEDIATE); } @Test - void commitBeginsNextTransactionOnTheSameStream() throws SQLException { + void commitClosesTheStreamAndTheNextStatementBeginsOnANewStream() throws SQLException { + ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); + LibsqlConnection connection = new LibsqlConnection(CONFIG, client); + connection.setAutoCommit(false); + connection.executeStatement(new Stmt("SELECT 1")); + + connection.commit(); + + assertThat(client.executedStatements()).containsExactly("BEGIN", "SELECT 1", "COMMIT"); + assertThat(client.requests).hasSize(4); + assertThat(client.requests.get(2).baton()).isNotNull(); + assertThat(client.requests.get(3).requests()).containsExactly(StreamRequest.Close.INSTANCE); + + connection.executeStatement(new Stmt("SELECT 2")); + + assertThat(client.requests.get(4).baton()).isNull(); + assertThat(client.requests.get(4).requests()) + .as("BEGIN must be checked on its own before the statement can run") + .hasSize(1); + assertThat(client.requests.get(5).baton()).isNotNull(); + } + + @Test + void rollbackClosesTheStreamAndTheNextStatementBeginsOnANewStream() throws SQLException { + ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); + LibsqlConnection connection = new LibsqlConnection(CONFIG, client); + connection.setAutoCommit(false); + connection.executeStatement(new Stmt("SELECT 1")); + + connection.rollback(); + + assertThat(client.executedStatements()).containsExactly("BEGIN", "SELECT 1", "ROLLBACK"); + assertThat(client.requests).hasSize(4); + assertThat(client.requests.get(3).requests()).containsExactly(StreamRequest.Close.INSTANCE); + + connection.executeStatement(new Stmt("SELECT 2")); + + assertThat(client.requests.get(4).baton()).isNull(); + } + + @Test + void commitWithNoWorkSendsNoRequestAndTheNextStatementCostsTwoRoundTrips() throws SQLException { ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); LibsqlConnection connection = new LibsqlConnection(CONFIG, client); connection.setAutoCommit(false); connection.commit(); - assertThat(client.executedStatements()).containsExactly("BEGIN", "COMMIT", "BEGIN"); + assertThat(client.requests).as("commit of a pending begin sends nothing").isEmpty(); + assertThat(connection.getAutoCommit()).isFalse(); + + connection.executeStatement(new Stmt("SELECT 1")); + + assertThat(client.executedStatements()).containsExactly("BEGIN", "SELECT 1"); assertThat(client.requests).hasSize(2); - assertThat(client.requests.get(1).baton()).isEqualTo(BATON); } @Test - void rollbackBeginsNextTransactionOnTheSameStream() throws SQLException { + void rollbackWithNoWorkSendsNoRequestAndTheNextStatementStillWorks() throws SQLException { ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); LibsqlConnection connection = new LibsqlConnection(CONFIG, client); connection.setAutoCommit(false); connection.rollback(); - assertThat(client.executedStatements()).containsExactly("BEGIN", "ROLLBACK", "BEGIN"); + assertThat(client.requests).isEmpty(); + assertThat(connection.getAutoCommit()).isFalse(); + + connection.executeStatement(new Stmt("SELECT 1")); + + assertThat(client.executedStatements()).containsExactly("BEGIN", "SELECT 1"); + } + + @Test + void aFailedBeginLeavesTheBeginPendingAndNoStatementRuns() throws SQLException { + AtomicInteger begins = new AtomicInteger(); + ScriptedClient client = new ScriptedClient((request, index) -> { + if ("BEGIN".equals(executedSql(request)) && begins.incrementAndGet() == 1) { + return errorResponse(new HranaError(SERVER_ERROR, "SQLITE_ERROR")); + } + return successResponse(request); + }); + LibsqlConnection connection = new LibsqlConnection(CONFIG, client); + connection.setAutoCommit(false); + + assertThatThrownBy(() -> connection.executeStatement(new Stmt("SELECT 1"))) + .isInstanceOf(SQLException.class) + .hasMessageContaining(SERVER_ERROR); + assertThat(client.requests).hasSize(2); - assertThat(client.requests.get(1).baton()).isEqualTo(BATON); + assertThat(client.requests.get(0).requests()) + .as("BEGIN is not pipelined with the statement, so the statement cannot run if it fails") + .hasSize(1); + assertThat(client.executedStatements()).containsExactly("BEGIN"); + + connection.executeStatement(new Stmt("SELECT 1")); + + assertThat(client.executedStatements()).containsExactly("BEGIN", "BEGIN", "SELECT 1"); + assertThat(client.requests.get(1).requests()) + .as("the failed begin releases its stream") + .containsExactly(StreamRequest.Close.INSTANCE); + assertThat(client.requests.get(2).baton()).isNull(); } @Test - void enablingAutoCommitCommitsAndClosesTheOpenTransaction() throws SQLException { + void enablingAutoCommitAfterWorkCommitsAndClosesTheOpenTransaction() throws SQLException { ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); LibsqlConnection connection = new LibsqlConnection(CONFIG, client); connection.setAutoCommit(false); + connection.executeStatement(new Stmt("INSERT INTO sample VALUES (1)")); connection.setAutoCommit(true); - assertThat(client.executedStatements()).containsExactly("BEGIN", "COMMIT"); - assertThat(client.requests.get(1).baton()).isEqualTo(BATON); + assertThat(client.executedStatements()) + .containsExactly("BEGIN", "INSERT INTO sample VALUES (1)", "COMMIT"); + assertThat(client.requests.get(2).baton()).isNotNull(); + assertThat(client.requests).hasSize(4); + assertThat(client.requests.get(3).requests()).containsExactly(StreamRequest.Close.INSTANCE); + assertThat(connection.getAutoCommit()).isTrue(); + } + + @Test + void enablingAutoCommitFromAPendingBeginSendsNoRequest() throws SQLException { + ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); + LibsqlConnection connection = new LibsqlConnection(CONFIG, client); + connection.setAutoCommit(false); + + connection.setAutoCommit(true); + + assertThat(client.requests).isEmpty(); + assertThat(connection.getAutoCommit()).isTrue(); + } + + @Test + void closeFromAPendingBeginSendsNoRequest() throws SQLException { + ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); + LibsqlConnection connection = new LibsqlConnection(CONFIG, client); + connection.setAutoCommit(false); + + connection.close(); + + assertThat(client.requests).isEmpty(); + assertThat(connection.isClosed()).isTrue(); + } + + @Test + void closeAfterWorkClosesTheOpenStream() throws SQLException { + ScriptedClient client = new ScriptedClient((request, index) -> successResponse(request)); + LibsqlConnection connection = new LibsqlConnection(CONFIG, client); + connection.setAutoCommit(false); + connection.executeStatement(new Stmt("SELECT 1")); + + connection.close(); + assertThat(client.requests).hasSize(3); assertThat(client.requests.get(2).requests()).containsExactly(StreamRequest.Close.INSTANCE); } @@ -157,10 +285,12 @@ void failedCommitCanBeRolledBackAndTheConnectionStaysUsable() throws SQLExceptio "BEGIN", "INSERT INTO sample VALUES (1)", "COMMIT", - "BEGIN", "ROLLBACK", "BEGIN", "INSERT INTO sample VALUES (2)"); + assertThat(client.requests.get(3).baton()) + .as("rollback after a failed commit retries on the same stream") + .isNotNull(); } @ParameterizedTest