Skip to content

Support begin in read-only mode for JDBC transactions #2738

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 9, 2025
Merged

Support begin in read-only mode for JDBC transactions #2738

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4524f78
Support begin in read-only mode for JDBC transaction
brfrn169 May 13, 2025
60c000f
Rename setReadOnly() to setConnectionToReadOnly()
brfrn169 Jun 6, 2025
3a2e33c
Fix
brfrn169 Jun 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions .../integration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
import java.util.Properties;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;

public class JdbcTransactionIntegrationTest extends DistributedTransactionIntegrationTestBase {

Expand Down Expand Up @@ -44,16 +42,4 @@ public void abort_forOngoingTransaction_ShouldAbortCorrectly() {}
@Override
@Test
public void rollback_forOngoingTransaction_ShouldRollbackCorrectly() {}

@Disabled("Implement later")
@Override
@Test
public void get_GetGivenForCommittedRecord_InReadOnlyMode_ShouldReturnRecord() {}

@Disabled("Implement later")
@Override
@ParameterizedTest
@EnumSource(ScanType.class)
public void scanOrGetScanner_ScanGivenForCommittedRecord_InReadOnlyMode_ShouldReturnRecords(
ScanType scanType) {}
}
82 changes: 56 additions & 26 deletions core/src/main/java/com/scalar/db/transaction/jdbc/JdbcTransactionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.scalar.db.api.Upsert;
import com.scalar.db.common.AbstractDistributedTransactionManager;
import com.scalar.db.common.AbstractTransactionManagerCrudOperableScanner;
import com.scalar.db.common.ReadOnlyDistributedTransaction;
import com.scalar.db.common.TableMetadataManager;
import com.scalar.db.common.checker.OperationChecker;
import com.scalar.db.common.error.CoreError;
Expand All @@ -36,6 +37,7 @@
import com.scalar.db.storage.jdbc.RdbEngineFactory;
import com.scalar.db.storage.jdbc.RdbEngineStrategy;
import com.scalar.db.util.ThrowableFunction;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -90,14 +92,39 @@ public JdbcTransactionManager(DatabaseConfig databaseConfig) {
@Override
public DistributedTransaction begin() throws TransactionException {
String txId = UUID.randomUUID().toString();
return begin(txId);
return begin(txId, false);
}

@Override
public DistributedTransaction begin(String txId) throws TransactionException {
return begin(txId, false);
}

@Override
public DistributedTransaction beginReadOnly() throws TransactionException {
String txId = UUID.randomUUID().toString();
return begin(txId, true);
}

@Override
public DistributedTransaction beginReadOnly(String txId) throws TransactionException {
return begin(txId, true);
}

private DistributedTransaction begin(String txId, boolean readOnly) throws TransactionException {
try {
JdbcTransaction transaction =
new JdbcTransaction(txId, jdbcService, dataSource.getConnection(), rdbEngine);
Connection connection = dataSource.getConnection();

DistributedTransaction transaction;
if (readOnly) {
rdbEngine.setReadOnly(connection, true);
Copy link
Collaborator Author

@brfrn169 brfrn169 Jun 5, 2025

Choose a reason for hiding this comment

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

Call Connection.setReadOnly() for read-only transactions.

Copy link
Contributor

@komamitsu komamitsu Jun 6, 2025

Choose a reason for hiding this comment

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

I noticed the method name setReadOnly() seems to change the rdbEngine to read-only mode. setConnectionToReadOnly() might be less confusing. What do you think?

Copy link
Collaborator Author

@brfrn169 brfrn169 Jun 6, 2025

Choose a reason for hiding this comment

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

Fixed in 60c000f. Thanks!

transaction =
new ReadOnlyDistributedTransaction(
new JdbcTransaction(txId, jdbcService, connection, rdbEngine));
Comment on lines +121 to +123
Copy link
Collaborator Author

@brfrn169 brfrn169 Jun 5, 2025

Choose a reason for hiding this comment

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

Wrap the transaction object with ReadOnlyDistributedTransaction.

} else {
transaction = new JdbcTransaction(txId, jdbcService, connection, rdbEngine);
}

getNamespace().ifPresent(transaction::withNamespace);
getTable().ifPresent(transaction::withTable);
return transaction;
Expand All @@ -109,16 +136,6 @@ public DistributedTransaction begin(String txId) throws TransactionException {
}
}

@Override
public DistributedTransaction beginReadOnly() {
throw new UnsupportedOperationException("implement later");
}

@Override
public DistributedTransaction beginReadOnly(String txId) {
throw new UnsupportedOperationException("implement later");
}

/** @deprecated As of release 2.4.0. Will be removed in release 4.0.0. */
@SuppressWarnings("InlineMeSuggester")
@Deprecated
Expand Down Expand Up @@ -173,19 +190,19 @@ public DistributedTransaction start(

@Override
public Optional<Result> get(Get get) throws CrudException, UnknownTransactionStatusException {
return executeTransaction(t -> t.get(copyAndSetTargetToIfNot(get)));
return executeTransaction(t -> t.get(copyAndSetTargetToIfNot(get)), true);
}

@Override
public List<Result> scan(Scan scan) throws CrudException, UnknownTransactionStatusException {
return executeTransaction(t -> t.scan(copyAndSetTargetToIfNot(scan)));
return executeTransaction(t -> t.scan(copyAndSetTargetToIfNot(scan)), true);
}

@Override
public Scanner getScanner(Scan scan) throws CrudException {
DistributedTransaction transaction;
try {
transaction = begin();
transaction = beginReadOnly();
} catch (TransactionNotFoundException e) {
throw new CrudConflictException(e.getMessage(), e, e.getTransactionId().orElse(null));
} catch (TransactionException e) {
Expand Down Expand Up @@ -277,7 +294,8 @@ public void put(Put put) throws CrudException, UnknownTransactionStatusException
t -> {
t.put(copyAndSetTargetToIfNot(put));
return null;
});
},
false);
}

/** @deprecated As of release 3.13.0. Will be removed in release 5.0.0. */
Expand All @@ -288,7 +306,8 @@ public void put(List<Put> puts) throws CrudException, UnknownTransactionStatusEx
t -> {
t.put(copyAndSetTargetToIfNot(puts));
return null;
});
},
false);
}

@Override
Expand All @@ -297,7 +316,8 @@ public void insert(Insert insert) throws CrudException, UnknownTransactionStatus
t -> {
t.insert(copyAndSetTargetToIfNot(insert));
return null;
});
},
false);
}

@Override
Expand All @@ -306,7 +326,8 @@ public void upsert(Upsert upsert) throws CrudException, UnknownTransactionStatus
t -> {
t.upsert(copyAndSetTargetToIfNot(upsert));
return null;
});
},
false);
}

@Override
Expand All @@ -315,7 +336,8 @@ public void update(Update update) throws CrudException, UnknownTransactionStatus
t -> {
t.update(copyAndSetTargetToIfNot(update));
return null;
});
},
false);
}

@Override
Expand All @@ -324,7 +346,8 @@ public void delete(Delete delete) throws CrudException, UnknownTransactionStatus
t -> {
t.delete(copyAndSetTargetToIfNot(delete));
return null;
});
},
false);
}

/** @deprecated As of release 3.13.0. Will be removed in release 5.0.0. */
Expand All @@ -335,7 +358,8 @@ public void delete(List<Delete> deletes) throws CrudException, UnknownTransactio
t -> {
t.delete(copyAndSetTargetToIfNot(deletes));
return null;
});
},
false);
}

@Override
Expand All @@ -345,15 +369,21 @@ public void mutate(List<? extends Mutation> mutations)
t -> {
t.mutate(copyAndSetTargetToIfNot(mutations));
return null;
});
},
false);
}

private <R> R executeTransaction(
ThrowableFunction<DistributedTransaction, R, TransactionException> throwableFunction)
ThrowableFunction<DistributedTransaction, R, TransactionException> throwableFunction,
boolean readOnly)
throws CrudException, UnknownTransactionStatusException {
DistributedTransaction transaction;
try {
transaction = begin();
if (readOnly) {
transaction = beginReadOnly();
} else {
transaction = begin();
}
} catch (TransactionNotFoundException e) {
throw new CrudConflictException(e.getMessage(), e, e.getTransactionId().orElse(null));
} catch (TransactionException e) {
Expand Down
Loading