Skip to content

Commit ebf69fe

Browse files
feeblefakiebrfrn169
feeblefakie
and
brfrn169
authored
Backport to branch(3) : Optimize JDBC storage by using Connection.setReadOnly() (#2675)
Co-authored-by: Toshihiro Suzuki <[email protected]>
1 parent d4e5222 commit ebf69fe

File tree

8 files changed

+99
-32
lines changed

8 files changed

+99
-32
lines changed

‎core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java‎

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,8 @@ public TableMetadata getTableMetadata(String namespace, String table) throws Exe
394394
boolean tableExists = false;
395395

396396
try (Connection connection = dataSource.getConnection()) {
397+
rdbEngine.setReadOnly(connection, true);
398+
397399
if (!namespaceExistsInternal(connection, metadataSchema)) {
398400
return null;
399401
}
@@ -459,6 +461,8 @@ public TableMetadata getImportTableMetadata(
459461
}
460462

461463
try (Connection connection = dataSource.getConnection()) {
464+
rdbEngine.setReadOnly(connection, true);
465+
462466
String catalogName = rdbEngine.getCatalogName(namespace);
463467
String schemaName = rdbEngine.getSchemaName(namespace);
464468

@@ -545,19 +549,22 @@ public Set<String> getNamespaceTableNames(String namespace) throws ExecutionExce
545549
+ " WHERE "
546550
+ enclose(METADATA_COL_FULL_TABLE_NAME)
547551
+ " LIKE ?";
548-
try (Connection connection = dataSource.getConnection();
549-
PreparedStatement preparedStatement =
550-
connection.prepareStatement(selectTablesOfNamespaceStatement)) {
551-
String prefix = namespace + ".";
552-
preparedStatement.setString(1, prefix + "%");
553-
try (ResultSet results = preparedStatement.executeQuery()) {
554-
Set<String> tableNames = new HashSet<>();
555-
while (results.next()) {
556-
String tableName =
557-
results.getString(METADATA_COL_FULL_TABLE_NAME).substring(prefix.length());
558-
tableNames.add(tableName);
552+
try (Connection connection = dataSource.getConnection()) {
553+
rdbEngine.setReadOnly(connection, true);
554+
555+
try (PreparedStatement preparedStatement =
556+
connection.prepareStatement(selectTablesOfNamespaceStatement)) {
557+
String prefix = namespace + ".";
558+
preparedStatement.setString(1, prefix + "%");
559+
try (ResultSet results = preparedStatement.executeQuery()) {
560+
Set<String> tableNames = new HashSet<>();
561+
while (results.next()) {
562+
String tableName =
563+
results.getString(METADATA_COL_FULL_TABLE_NAME).substring(prefix.length());
564+
tableNames.add(tableName);
565+
}
566+
return tableNames;
559567
}
560-
return tableNames;
561568
}
562569
} catch (SQLException e) {
563570
// An exception will be thrown if the metadata table does not exist when executing the select
@@ -576,6 +583,8 @@ public boolean namespaceExists(String namespace) throws ExecutionException {
576583
}
577584

578585
try (Connection connection = dataSource.getConnection()) {
586+
rdbEngine.setReadOnly(connection, true);
587+
579588
return namespaceExistsInternal(connection, namespace);
580589
} catch (SQLException e) {
581590
throw new ExecutionException("Checking if the namespace exists failed", e);
@@ -804,18 +813,21 @@ public Set<String> getNamespaceNames() throws ExecutionException {
804813
+ enclose(METADATA_COL_FULL_TABLE_NAME)
805814
+ " FROM "
806815
+ encloseFullTableName(metadataSchema, METADATA_TABLE);
807-
try (Connection connection = dataSource.getConnection();
808-
Statement stmt = connection.createStatement();
809-
ResultSet rs = stmt.executeQuery(selectAllTableNames)) {
810-
Set<String> namespaceOfExistingTables = new HashSet<>();
811-
while (rs.next()) {
812-
String fullTableName = rs.getString(METADATA_COL_FULL_TABLE_NAME);
813-
String namespaceName = fullTableName.substring(0, fullTableName.indexOf('.'));
814-
namespaceOfExistingTables.add(namespaceName);
815-
}
816-
namespaceOfExistingTables.add(metadataSchema);
816+
try (Connection connection = dataSource.getConnection()) {
817+
rdbEngine.setReadOnly(connection, true);
818+
819+
try (Statement stmt = connection.createStatement();
820+
ResultSet rs = stmt.executeQuery(selectAllTableNames)) {
821+
Set<String> namespaceOfExistingTables = new HashSet<>();
822+
while (rs.next()) {
823+
String fullTableName = rs.getString(METADATA_COL_FULL_TABLE_NAME);
824+
String namespaceName = fullTableName.substring(0, fullTableName.indexOf('.'));
825+
namespaceOfExistingTables.add(namespaceName);
826+
}
827+
namespaceOfExistingTables.add(metadataSchema);
817828

818-
return namespaceOfExistingTables;
829+
return namespaceOfExistingTables;
830+
}
819831
} catch (SQLException e) {
820832
// An exception will be thrown if the namespace table does not exist when executing the select
821833
// query

‎core/src/main/java/com/scalar/db/storage/jdbc/JdbcDatabase.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ public Optional<Result> get(Get get) throws ExecutionException {
8282
Connection connection = null;
8383
try {
8484
connection = dataSource.getConnection();
85+
rdbEngine.setReadOnly(connection, true);
8586
return jdbcService.get(get, connection);
8687
} catch (SQLException e) {
8788
throw new ExecutionException(
@@ -97,6 +98,7 @@ public Scanner scan(Scan scan) throws ExecutionException {
9798
Connection connection = null;
9899
try {
99100
connection = dataSource.getConnection();
101+
rdbEngine.setReadOnly(connection, true);
100102
return jdbcService.getScanner(scan, connection);
101103
} catch (SQLException e) {
102104
close(connection);

‎core/src/main/java/com/scalar/db/storage/jdbc/JdbcUtils.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ public static BasicDataSource initDataSource(
6363
}
6464
});
6565

66+
dataSource.setDefaultReadOnly(false);
67+
6668
dataSource.setMinIdle(config.getConnectionPoolMinIdle());
6769
dataSource.setMaxIdle(config.getConnectionPoolMaxIdle());
6870
dataSource.setMaxTotal(config.getConnectionPoolMaxTotal());
@@ -89,6 +91,9 @@ public static BasicDataSource initDataSourceForTableMetadata(
8991
dataSource.setUrl(config.getJdbcUrl());
9092
config.getUsername().ifPresent(dataSource::setUsername);
9193
config.getPassword().ifPresent(dataSource::setPassword);
94+
95+
dataSource.setDefaultReadOnly(false);
96+
9297
dataSource.setMinIdle(config.getTableMetadataConnectionPoolMinIdle());
9398
dataSource.setMaxIdle(config.getTableMetadataConnectionPoolMaxIdle());
9499
dataSource.setMaxTotal(config.getTableMetadataConnectionPoolMaxTotal());
@@ -113,6 +118,9 @@ public static BasicDataSource initDataSourceForAdmin(
113118
dataSource.setUrl(config.getJdbcUrl());
114119
config.getUsername().ifPresent(dataSource::setUsername);
115120
config.getPassword().ifPresent(dataSource::setPassword);
121+
122+
dataSource.setDefaultReadOnly(false);
123+
116124
dataSource.setMinIdle(config.getAdminConnectionPoolMinIdle());
117125
dataSource.setMaxIdle(config.getAdminConnectionPoolMaxIdle());
118126
dataSource.setMaxTotal(config.getAdminConnectionPoolMaxTotal());

‎core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import com.scalar.db.storage.jdbc.query.UpsertQuery;
1414
import com.scalar.db.util.TimeRelatedColumnEncodingUtils;
1515
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
16+
import java.sql.Connection;
1617
import java.sql.Driver;
1718
import java.sql.JDBCType;
1819
import java.sql.ResultSet;
@@ -336,4 +337,9 @@ public TimestampTZColumn parseTimestampTZColumn(ResultSet resultSet, String colu
336337
public RdbEngineTimeTypeStrategy<Integer, Long, Long, Long> getTimeTypeStrategy() {
337338
return timeTypeEngine;
338339
}
340+
341+
@Override
342+
public void setReadOnly(Connection connection, boolean readOnly) {
343+
// Do nothing. SQLite does not support read-only mode.
344+
}
339345
}

‎core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.scalar.db.io.TimestampTZColumn;
1111
import com.scalar.db.storage.jdbc.query.SelectQuery;
1212
import com.scalar.db.storage.jdbc.query.UpsertQuery;
13+
import java.sql.Connection;
1314
import java.sql.Driver;
1415
import java.sql.JDBCType;
1516
import java.sql.ResultSet;
@@ -226,4 +227,8 @@ default String getProjectionsSqlForSelectQuery(TableMetadata metadata, List<Stri
226227
default void throwIfDuplicatedIndexWarning(SQLWarning warning) throws SQLException {
227228
// Do nothing
228229
}
230+
231+
default void setReadOnly(Connection connection, boolean readOnly) throws SQLException {
232+
connection.setReadOnly(readOnly);
233+
}
229234
}

‎core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTestBase.java‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1212
import static org.assertj.core.api.Assertions.catchThrowable;
1313
import static org.mockito.ArgumentMatchers.any;
14+
import static org.mockito.ArgumentMatchers.anyBoolean;
1415
import static org.mockito.ArgumentMatchers.anyInt;
1516
import static org.mockito.ArgumentMatchers.anyString;
1617
import static org.mockito.ArgumentMatchers.eq;
@@ -19,6 +20,7 @@
1920
import static org.mockito.Mockito.doNothing;
2021
import static org.mockito.Mockito.mock;
2122
import static org.mockito.Mockito.mockStatic;
23+
import static org.mockito.Mockito.never;
2224
import static org.mockito.Mockito.spy;
2325
import static org.mockito.Mockito.times;
2426
import static org.mockito.Mockito.verify;
@@ -53,6 +55,8 @@
5355
import org.apache.commons.dbcp2.BasicDataSource;
5456
import org.junit.jupiter.api.BeforeEach;
5557
import org.junit.jupiter.api.Test;
58+
import org.junit.jupiter.params.ParameterizedTest;
59+
import org.junit.jupiter.params.provider.EnumSource;
5660
import org.mockito.ArgumentCaptor;
5761
import org.mockito.Mock;
5862
import org.mockito.MockedStatic;
@@ -285,6 +289,11 @@ private void getTableMetadata_forX_ShouldReturnTableMetadata(
285289
.addSecondaryIndex("c4")
286290
.build();
287291
assertThat(actualMetadata).isEqualTo(expectedMetadata);
292+
if (rdbEngine == RdbEngine.SQLITE) {
293+
verify(connection, never()).setReadOnly(anyBoolean());
294+
} else {
295+
verify(connection).setReadOnly(true);
296+
}
288297
verify(connection).prepareStatement(expectedSelectStatements);
289298
}
290299

@@ -1702,6 +1711,11 @@ private void getNamespaceTables_forX_ShouldReturnTableNames(
17021711
Set<String> actualTableNames = admin.getNamespaceTableNames(namespace);
17031712

17041713
// Assert
1714+
if (rdbEngine == RdbEngine.SQLITE) {
1715+
verify(connection, never()).setReadOnly(anyBoolean());
1716+
} else {
1717+
verify(connection).setReadOnly(true);
1718+
}
17051719
verify(connection).prepareStatement(expectedSelectStatement);
17061720
assertThat(actualTableNames).containsExactly(table1, table2);
17071721
verify(preparedStatement).setString(1, namespace + ".%");
@@ -1771,6 +1785,11 @@ private void namespaceExists_forXWithExistingNamespace_ShouldReturnTrue(
17711785
// Assert
17721786
assertThat(admin.namespaceExists(namespace)).isTrue();
17731787

1788+
if (rdbEngine == RdbEngine.SQLITE) {
1789+
verify(connection, never()).setReadOnly(anyBoolean());
1790+
} else {
1791+
verify(connection).setReadOnly(true);
1792+
}
17741793
verify(selectStatement).executeQuery();
17751794
verify(connection).prepareStatement(expectedSelectStatement);
17761795
verify(selectStatement).setString(1, namespace + namespacePlaceholderSuffix);
@@ -2772,16 +2791,15 @@ private void addNewColumnToTable_ForX_ShouldWorkProperly(
27722791
}
27732792
}
27742793

2775-
@Test
2776-
public void getImportTableMetadata_ForX_ShouldWorkProperly()
2794+
@ParameterizedTest
2795+
@EnumSource(RdbEngine.class)
2796+
public void getImportTableMetadata_ForX_ShouldWorkProperly(RdbEngine rdbEngine)
27772797
throws SQLException, ExecutionException {
2778-
for (RdbEngine rdbEngine : RDB_ENGINES.keySet()) {
2779-
if (rdbEngine.equals(RdbEngine.SQLITE)) {
2780-
getImportTableMetadata_ForSQLite_ShouldThrowUnsupportedOperationException(rdbEngine);
2781-
} else {
2782-
getImportTableMetadata_ForOtherThanSQLite_ShouldWorkProperly(
2783-
rdbEngine, prepareSqlForTableCheck(rdbEngine, NAMESPACE, TABLE));
2784-
}
2798+
if (rdbEngine.equals(RdbEngine.SQLITE)) {
2799+
getImportTableMetadata_ForSQLite_ShouldThrowUnsupportedOperationException(rdbEngine);
2800+
} else {
2801+
getImportTableMetadata_ForOtherThanSQLite_ShouldWorkProperly(
2802+
rdbEngine, prepareSqlForTableCheck(rdbEngine, NAMESPACE, TABLE));
27852803
}
27862804
}
27872805

@@ -2848,6 +2866,7 @@ public Boolean answer(InvocationOnMock invocation) {
28482866
.execute(expectedCheckTableExistStatement);
28492867
assertThat(actual.getPartitionKeyNames()).hasSameElementsAs(ImmutableSet.of("pk1", "pk2"));
28502868
assertThat(actual.getColumnDataTypes()).containsExactlyEntriesOf(expectedColumns);
2869+
verify(connection).setReadOnly(true);
28512870
verify(rdbEngineStrategy)
28522871
.getDataTypeForScalarDb(
28532872
any(JDBCType.class),
@@ -3310,6 +3329,11 @@ private void getNamespaceNames_ForX_WithExistingTables_ShouldWorkProperly(
33103329
Set<String> actual = admin.getNamespaceNames();
33113330

33123331
// Assert
3332+
if (rdbEngine.equals(RdbEngine.SQLITE)) {
3333+
verify(connection, never()).setReadOnly(anyBoolean());
3334+
} else {
3335+
verify(connection).setReadOnly(true);
3336+
}
33133337
verify(connection).createStatement();
33143338
verify(getTableMetadataNamespacesStatementMock)
33153339
.executeQuery(getTableMetadataNamespacesStatement);

‎core/src/test/java/com/scalar/db/storage/jdbc/JdbcDatabaseTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ public void whenGetOperationExecuted_shouldCallJdbcService() throws Exception {
7171
jdbcDatabase.get(get);
7272

7373
// Assert
74+
verify(connection).setReadOnly(true);
7475
verify(jdbcService).get(any(), any());
7576
verify(connection).close();
7677
}
@@ -89,6 +90,7 @@ public void whenGetOperationExecuted_shouldCallJdbcService() throws Exception {
8990
jdbcDatabase.get(get);
9091
})
9192
.isInstanceOf(ExecutionException.class);
93+
verify(connection).setReadOnly(true);
9294
verify(connection).close();
9395
}
9496

@@ -104,6 +106,7 @@ public void whenScanOperationExecutedAndScannerClosed_shouldCallJdbcService() th
104106
scanner.close();
105107

106108
// Assert
109+
verify(connection).setReadOnly(true);
107110
verify(jdbcService).getScanner(any(), any());
108111
verify(connection).close();
109112
}
@@ -122,6 +125,7 @@ public void whenScanOperationExecutedAndScannerClosed_shouldCallJdbcService() th
122125
jdbcDatabase.scan(scan);
123126
})
124127
.isInstanceOf(ExecutionException.class);
128+
verify(connection).setReadOnly(true);
125129
verify(connection).close();
126130
}
127131

‎core/src/test/java/com/scalar/db/storage/jdbc/JdbcUtilsTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public void initDataSource_NonTransactional_ShouldReturnProperDataSource() throw
6666
assertThat(dataSource.getAutoCommitOnReturn()).isEqualTo(true);
6767
assertThat(dataSource.getDefaultTransactionIsolation())
6868
.isEqualTo(Connection.TRANSACTION_SERIALIZABLE);
69+
assertThat(dataSource.getDefaultReadOnly()).isFalse();
6970

7071
assertThat(dataSource.getMinIdle()).isEqualTo(10);
7172
assertThat(dataSource.getMaxIdle()).isEqualTo(20);
@@ -109,6 +110,7 @@ public void initDataSource_Transactional_ShouldReturnProperDataSource() throws S
109110
assertThat(dataSource.getAutoCommitOnReturn()).isEqualTo(false);
110111
assertThat(dataSource.getDefaultTransactionIsolation())
111112
.isEqualTo(Connection.TRANSACTION_READ_COMMITTED);
113+
assertThat(dataSource.getDefaultReadOnly()).isFalse();
112114

113115
assertThat(dataSource.getMinIdle()).isEqualTo(30);
114116
assertThat(dataSource.getMaxIdle()).isEqualTo(40);
@@ -180,6 +182,8 @@ public void initDataSourceForTableMetadata_ShouldReturnProperDataSource() throws
180182
assertThat(tableMetadataDataSource.getUsername()).isEqualTo("user");
181183
assertThat(tableMetadataDataSource.getPassword()).isEqualTo("oracle");
182184

185+
assertThat(tableMetadataDataSource.getDefaultReadOnly()).isFalse();
186+
183187
assertThat(tableMetadataDataSource.getMinIdle()).isEqualTo(100);
184188
assertThat(tableMetadataDataSource.getMaxIdle()).isEqualTo(200);
185189
assertThat(tableMetadataDataSource.getMaxTotal()).isEqualTo(300);
@@ -212,6 +216,8 @@ public void initDataSourceForAdmin_ShouldReturnProperDataSource() throws SQLExce
212216
assertThat(adminDataSource.getUsername()).isEqualTo("user");
213217
assertThat(adminDataSource.getPassword()).isEqualTo("sqlserver");
214218

219+
assertThat(adminDataSource.getDefaultReadOnly()).isFalse();
220+
215221
assertThat(adminDataSource.getMinIdle()).isEqualTo(100);
216222
assertThat(adminDataSource.getMaxIdle()).isEqualTo(200);
217223
assertThat(adminDataSource.getMaxTotal()).isEqualTo(300);

0 commit comments

Comments
 (0)