Skip to content

Commit

Permalink
Merge pull request #3579 from FOCONIS/rollbac-or-commit-before-close
Browse files Browse the repository at this point in the history
Rollback/Commit raw connections before close
  • Loading branch information
rbygrave authored Mar 3, 2025
2 parents 11e7839 + 7643dc9 commit b4ff574
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ protected List<Long> getMoreIds(int requestSize) {
if (newIds.isEmpty()) {
throw new PersistenceException("Always expecting more than 1 row from " + sql);
}

connection.commit();
return newIds;

} catch (SQLException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ private DatabasePlatform byDatabaseName(String dbName) {
*/
private DatabasePlatform byDataSource(DataSource dataSource) {
try (Connection connection = dataSource.getConnection()) {
return byDatabaseMeta(connection.getMetaData(), connection);
DatabasePlatform platform = byDatabaseMeta(connection.getMetaData(), connection);
if (!connection.getAutoCommit()) {
// we must roll back before close.
connection.rollback();
}
return platform;
} catch (SQLException ex) {
throw new PersistenceException(ex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ private boolean checkDataSource(DatabaseBuilder.Settings config) {
try (Connection connection = config.getDataSource().getConnection()) {
if (connection.getAutoCommit()) {
log.log(WARNING, "DataSource [{0}] has autoCommit defaulting to true!", config.getName());
} else {
connection.rollback();
}
return true;
} catch (SQLException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ private List<MetaQueryPlan> collectPlans(QueryPlanRequest request) {
while (req.hasNext()) {
req.nextCapture();
}
if (!connection.getAutoCommit()) {
// CHECKME: commit or rollback here?
// arguments for rollback: the collecting should never modify data.
// if there are collectors that may copy the plan into tables, it's up to the collector to
// commit the transaction.
connection.rollback();
}
return req.plans();
} catch (SQLException e) {
CoreLog.log.log(ERROR, "Error during query plan collection", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ public void usingConnection() throws SQLException {
.usingConnection(connection)
.findList();

connection.rollback();
assertThat(foo).hasSize(1);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ datasource.default=h2
datasource.h2.username=sa
datasource.h2.password=
datasource.h2.url=jdbc:h2:mem:tests;NON_KEYWORDS=KEY,VALUE
datasource.h2.closeWithinTxn=fail

datasource.pg.username=sa
datasource.pg.password=
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ private void testVersioning() {
config.setName(server().name());
config.loadFromProperties(server().pluginApi().config().getProperties());
config.setDataSource(server().dataSource());
config.setReadOnlyDataSource(server().dataSource());
config.setReadOnlyDataSource(server().readOnlyDataSource());
config.setDdlGenerate(false);
config.setDdlRun(false);
config.setRegister(false);
Expand Down Expand Up @@ -292,7 +292,7 @@ private void testReservedKeywords() {
config.setName(server().name());
config.loadFromProperties(server().pluginApi().config().getProperties());
config.setDataSource(server().dataSource());
config.setReadOnlyDataSource(server().dataSource());
config.setReadOnlyDataSource(server().readOnlyDataSource());
config.setDdlGenerate(false);
config.setDdlRun(false);
config.setRegister(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ public void findDuplicateColumnName() throws SQLException {
}
}
}
connection.rollback();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public void usingConnection() throws SQLException {
.findCount();

assertThat(count).isGreaterThan(0);
connection.rollback();
}
}

Expand Down
1 change: 1 addition & 0 deletions ebean-test/src/test/resources/ebean.properties
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ datasource.db.databaseDriver=org.h2.Driver
datasource.h2.username=sa
datasource.h2.password=
datasource.h2.url=jdbc:h2:mem:testsMem;DB_CLOSE_ON_EXIT=FALSE;NON_KEYWORDS=KEY,VALUE
datasource.h2.closeWithinTxn=fail
datasource.h2.poolListener=org.tests.basic.MyTestDataSourcePoolListener
#datasource.h2.minConnections=1
#datasource.h2.maxConnections=25
Expand Down

0 comments on commit b4ff574

Please sign in to comment.