Skip to content

Commit f8ff99a

Browse files
ScottDugasScottDugas
authored
Allow enabling autoCommit without an active transaction (#3477)
This changes the client so that it sends a request to enable autoCommit when it is enabled on the client, rather than calling `commit` directly. By doing so, the server can determine whether it wants to commit or not based on whether it has an active transaction. Fixes: #3473
1 parent b81e185 commit f8ff99a

File tree

5 files changed

+113
-5
lines changed

5 files changed

+113
-5
lines changed

‎fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/jdbc.proto

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ message TransactionalRequest {
122122
InsertRequest insertRequest = 2;
123123
CommitRequest commitRequest = 3;
124124
RollbackRequest rollbackRequest = 4;
125+
EnableAutoCommitRequest enableAutoCommitRequest = 5;
125126
}
126127
}
127128

@@ -131,7 +132,10 @@ message TransactionalResponse {
131132
InsertResponse insertResponse = 2;
132133
CommitResponse commitResponse = 3;
133134
RollbackResponse rollbackResponse = 4;
135+
134136
google.protobuf.Any errorResponse = 5;
137+
138+
EnableAutoCommitResponse enableAutoCommitResponse = 6;
135139
}
136140
}
137141

@@ -163,6 +167,12 @@ message RollbackRequest {
163167
message RollbackResponse {
164168
}
165169

170+
message EnableAutoCommitRequest {
171+
}
172+
173+
message EnableAutoCommitResponse {
174+
}
175+
166176
message DatabaseMetaDataRequest {}
167177
message DatabaseMetaDataResponse {
168178
string url = 1;

‎fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalConnection.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.apple.foundationdb.relational.jdbc.grpc.GrpcSQLExceptionUtil;
3232
import com.apple.foundationdb.relational.jdbc.grpc.v1.CommitRequest;
3333
import com.apple.foundationdb.relational.jdbc.grpc.v1.DatabaseMetaDataRequest;
34+
import com.apple.foundationdb.relational.jdbc.grpc.v1.EnableAutoCommitRequest;
3435
import com.apple.foundationdb.relational.jdbc.grpc.v1.InsertRequest;
3536
import com.apple.foundationdb.relational.jdbc.grpc.v1.InsertResponse;
3637
import com.apple.foundationdb.relational.jdbc.grpc.v1.JDBCServiceGrpc;
@@ -345,7 +346,23 @@ public void setAutoCommit(boolean autoCommit) throws SQLException {
345346
this.autoCommit = false;
346347
} else {
347348
// commit any remaining work
348-
commit();
349+
try {
350+
TransactionalRequest.Builder transactionRequest = TransactionalRequest.newBuilder()
351+
.setEnableAutoCommitRequest(EnableAutoCommitRequest.newBuilder().build());
352+
// wait here for response
353+
final TransactionalResponse response = serverConnection.sendRequest(transactionRequest.build());
354+
checkForResponseError(response);
355+
if (!response.hasEnableAutoCommitResponse()) {
356+
throw new JdbcConnectionException("Wrong kind of response received, expected EnableAutoCommitResponse");
357+
}
358+
} catch (StatusRuntimeException statusRuntimeException) {
359+
final SQLException sqlException = toSQLException(statusRuntimeException);
360+
if (sqlException != null) {
361+
throw sqlException;
362+
} else {
363+
throw statusRuntimeException;
364+
}
365+
}
349366
this.autoCommit = true;
350367
serverConnection.close();
351368
serverConnection = null;

‎fdb-relational-jdbc/src/test/java/com/apple/foundationdb/relational/jdbc/JDBCAutoCommitTest.java

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public void tearDown() throws Exception {
6969
* Run a test with the default (auto-commit on) for sanity.
7070
*/
7171
@Test
72-
void autoCommitOn() throws SQLException, IOException {
72+
void autoCommitOn() throws SQLException {
7373
try (RelationalConnection connection = DriverManager.getConnection(getTestDbUri()).unwrap(RelationalConnection.class)) {
7474
try (RelationalStatement statement = connection.createStatement()) {
7575
insertOneRow(statement);
@@ -80,6 +80,69 @@ void autoCommitOn() throws SQLException, IOException {
8080
}
8181
}
8282

83+
@Test
84+
void autoCommitWithNoTransactionInBetween() throws SQLException {
85+
try (RelationalConnection connection = DriverManager.getConnection(getTestDbUri()).unwrap(RelationalConnection.class)) {
86+
connection.setAutoCommit(false);
87+
connection.setAutoCommit(true);
88+
}
89+
}
90+
91+
@Test
92+
void autoCommitStayOn() throws SQLException {
93+
try (RelationalConnection connection = DriverManager.getConnection(getTestDbUri()).unwrap(RelationalConnection.class)) {
94+
connection.setAutoCommit(true);
95+
}
96+
}
97+
98+
@Test
99+
void autoCommitStayOff() throws SQLException {
100+
try (RelationalConnection connection = DriverManager.getConnection(getTestDbUri()).unwrap(RelationalConnection.class)) {
101+
connection.setAutoCommit(false);
102+
connection.setAutoCommit(false);
103+
}
104+
}
105+
106+
@Test
107+
void commitEnableAutoCommit() throws SQLException {
108+
try (RelationalConnection connection = DriverManager.getConnection(getTestDbUri()).unwrap(RelationalConnection.class)) {
109+
connection.setAutoCommit(false);
110+
111+
try (RelationalStatement statement = connection.createStatement()) {
112+
insertOneRow(statement);
113+
}
114+
connection.commit();
115+
connection.setAutoCommit(true);
116+
try (RelationalStatement statement = connection.createStatement()) {
117+
insertOneRow(statement, 101);
118+
}
119+
try (RelationalStatement statement = connection.createStatement()) {
120+
RelationalResultSet resultSet = statement.executeQuery("SELECT * FROM test_table");
121+
assertNextResult(resultSet, 100, "one hundred");
122+
assertNextResult(resultSet, 101, "one hundred");
123+
assertNoNextResult(resultSet);
124+
}
125+
}
126+
}
127+
128+
@Test
129+
void rollbackThenEnableAutoCommit() throws SQLException {
130+
try (RelationalConnection connection = DriverManager.getConnection(getTestDbUri()).unwrap(RelationalConnection.class)) {
131+
connection.setAutoCommit(false);
132+
133+
try (RelationalStatement statement = connection.createStatement()) {
134+
insertOneRow(statement);
135+
}
136+
connection.rollback();
137+
connection.setAutoCommit(true);
138+
try (RelationalStatement statement = connection.createStatement()) {
139+
RelationalResultSet resultSet = statement.executeQuery("SELECT * FROM test_table");
140+
assertNoNextResult(resultSet);
141+
assertNoNextResult(resultSet);
142+
}
143+
}
144+
}
145+
83146
/**
84147
* Run a test with commit and then read.
85148
*/
@@ -169,7 +232,7 @@ void revertToAutoCommitOn() throws Exception {
169232

170233
try (RelationalStatement statement = connection.createStatement()) {
171234
insertOneRow(statement);
172-
connection.setAutoCommit(true);
235+
connection.setAutoCommit(true); // this should commit
173236

174237
RelationalResultSet resultSet = statement.executeQuery("SELECT * FROM test_table");
175238
assertNextResult(resultSet, 100, "one hundred");
@@ -344,8 +407,12 @@ protected String getHostPort() {
344407
}
345408

346409
private static void insertOneRow(final RelationalStatement statement) throws SQLException {
410+
insertOneRow(statement, 100);
411+
}
412+
413+
private static void insertOneRow(final RelationalStatement statement, int restNo) throws SQLException {
347414
RelationalStruct insert = JDBCRelationalStruct.newBuilder()
348-
.addLong("REST_NO", 100)
415+
.addLong("REST_NO", restNo)
349416
.addString("NAME", "one hundred")
350417
.build();
351418
int res = statement.executeInsert("TEST_TABLE", insert);

‎fdb-relational-server/src/main/java/com/apple/foundationdb/relational/server/FRL.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,11 @@ public void transactionalRollback(TransactionalToken token) throws SQLException
354354
token.getConnection().rollback();
355355
}
356356

357+
public void enableAutoCommit(TransactionalToken token) throws SQLException {
358+
assertValidToken(token);
359+
token.getConnection().setAutoCommit(true);
360+
}
361+
357362
public void transactionalClose(TransactionalToken token) throws SQLException {
358363
if (token != null && !token.expired()) {
359364
token.close();

‎fdb-relational-server/src/main/java/com/apple/foundationdb/relational/server/jdbc/v1/TransactionRequestHandler.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.apple.foundationdb.relational.jdbc.TypeConversion;
2727
import com.apple.foundationdb.relational.jdbc.grpc.GrpcSQLExceptionUtil;
2828
import com.apple.foundationdb.relational.jdbc.grpc.v1.CommitResponse;
29+
import com.apple.foundationdb.relational.jdbc.grpc.v1.EnableAutoCommitResponse;
2930
import com.apple.foundationdb.relational.jdbc.grpc.v1.InsertRequest;
3031
import com.apple.foundationdb.relational.jdbc.grpc.v1.InsertResponse;
3132
import com.apple.foundationdb.relational.jdbc.grpc.v1.RollbackResponse;
@@ -114,8 +115,16 @@ public void onNext(final TransactionalRequest transactionRequest) {
114115
logger.info("Handling rollback request");
115116
frl.transactionalRollback(transactionalToken);
116117
responseBuilder.setRollbackResponse(RollbackResponse.newBuilder().build());
118+
} else if (transactionRequest.hasEnableAutoCommitRequest()) {
119+
logger.info("Enabling autoCommit");
120+
// we don't actually call setAutoCommit(false) until an operation happens, so if there is no token
121+
// there's no connection that needs updating
122+
if (transactionalToken != null) {
123+
frl.enableAutoCommit(transactionalToken);
124+
}
125+
responseBuilder.setEnableAutoCommitResponse(EnableAutoCommitResponse.newBuilder().build());
117126
} else {
118-
throw new IllegalArgumentException("Unknown transactional request type in" + transactionRequest);
127+
throw new IllegalArgumentException("Unknown transactional request type: " + transactionRequest);
119128
}
120129
responseObserver.onNext(responseBuilder.build());
121130
} catch (SQLException e) {

0 commit comments

Comments
 (0)