Skip to content

Commit 54ab4e6

Browse files
KodaiDKodaiD
authored
Create index if not exists in repairTable of CosmosAdmin and DynamoAdmin (#2800)
1 parent 895c6c5 commit 54ab4e6

File tree

8 files changed

+265
-1
lines changed

8 files changed

+265
-1
lines changed

‎core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,7 @@ public void repairTable(
588588
throws ExecutionException {
589589
try {
590590
createTableInternal(namespace, table, metadata, true);
591+
updateIndexingPolicy(namespace, table, metadata);
591592
} catch (IllegalArgumentException e) {
592593
throw e;
593594
} catch (Exception e) {

‎core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,6 +1032,46 @@ columnName, getFullTableName(namespace, table)),
10321032
TableMetadata.newBuilder(tableMetadata).addSecondaryIndex(columnName).build());
10331033
}
10341034

1035+
private boolean rawIndexExists(String nonPrefixedNamespace, String table, String indexName) {
1036+
Namespace namespace = Namespace.of(namespacePrefix, nonPrefixedNamespace);
1037+
String globalIndexName =
1038+
String.join(
1039+
".",
1040+
getFullTableName(namespace, table),
1041+
DynamoAdmin.GLOBAL_INDEX_NAME_PREFIX,
1042+
indexName);
1043+
int retryCount = 0;
1044+
try {
1045+
while (true) {
1046+
DescribeTableResponse response =
1047+
client.describeTable(
1048+
DescribeTableRequest.builder()
1049+
.tableName(getFullTableName(namespace, table))
1050+
.build());
1051+
GlobalSecondaryIndexDescription description =
1052+
response.table().globalSecondaryIndexes().stream()
1053+
.filter(d -> d.indexName().equals(globalIndexName))
1054+
.findFirst()
1055+
.orElse(null);
1056+
if (description == null) {
1057+
return false;
1058+
}
1059+
if (description.indexStatus() == IndexStatus.ACTIVE) {
1060+
return true;
1061+
}
1062+
if (retryCount++ >= MAX_RETRY_COUNT) {
1063+
throw new IllegalStateException(
1064+
String.format(
1065+
"Waiting for the secondary index %s on the %s table to be active failed",
1066+
indexName, getFullTableName(namespace, table)));
1067+
}
1068+
Uninterruptibles.sleepUninterruptibly(waitingDurationSecs, TimeUnit.SECONDS);
1069+
}
1070+
} catch (ResourceNotFoundException e) {
1071+
return false;
1072+
}
1073+
}
1074+
10351075
private void waitForIndexCreation(Namespace namespace, String table, String columnName)
10361076
throws ExecutionException {
10371077
try {
@@ -1366,6 +1406,11 @@ public void repairTable(
13661406
throws ExecutionException {
13671407
try {
13681408
createTableInternal(nonPrefixedNamespace, table, metadata, true, options);
1409+
for (String indexColumnName : metadata.getSecondaryIndexNames()) {
1410+
if (!rawIndexExists(nonPrefixedNamespace, table, indexColumnName)) {
1411+
createIndex(nonPrefixedNamespace, table, indexColumnName, options);
1412+
}
1413+
}
13691414
} catch (RuntimeException e) {
13701415
throw new ExecutionException(
13711416
String.format(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,8 @@ columnName, getFullTableName(namespace, table)),
889889
}
890890
}
891891

892-
private void createIndex(
892+
@VisibleForTesting
893+
void createIndex(
893894
Connection connection, String schema, String table, String indexedColumn, boolean ifNotExists)
894895
throws SQLException {
895896
String indexName = getIndexName(schema, table, indexedColumn);

‎core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import static org.mockito.ArgumentMatchers.any;
88
import static org.mockito.ArgumentMatchers.anyString;
99
import static org.mockito.Mockito.mock;
10+
import static org.mockito.Mockito.spy;
1011
import static org.mockito.Mockito.times;
1112
import static org.mockito.Mockito.verify;
1213
import static org.mockito.Mockito.when;
@@ -797,6 +798,43 @@ public void repairTable_ShouldCreateTableAndIndexesIfTheyDoNotExists() throws Ex
797798
}
798799
}
799800

801+
@Test
802+
public void repairTable_WhenTableAlreadyExistsWithoutIndex_ShouldCreateIndex()
803+
throws ExecutionException {
804+
// Arrange
805+
String namespace = "sample_ns";
806+
String table = "tbl";
807+
TableMetadata tableMetadata =
808+
TableMetadata.newBuilder()
809+
.addPartitionKey("c1")
810+
.addClusteringKey("c4")
811+
.addColumn("c1", DataType.INT)
812+
.addColumn("c2", DataType.TEXT)
813+
.addColumn("c3", DataType.BLOB)
814+
.addColumn("c4", DataType.INT)
815+
.addColumn("c5", DataType.BOOLEAN)
816+
.addSecondaryIndex("c2")
817+
.addSecondaryIndex("c4")
818+
.build();
819+
// The table already exists
820+
when(clusterManager.getMetadata(namespace, table))
821+
.thenReturn(mock(com.datastax.driver.core.TableMetadata.class));
822+
when(cassandraSession.getCluster()).thenReturn(cluster);
823+
when(cluster.getMetadata()).thenReturn(metadata);
824+
when(metadata.getKeyspace(any())).thenReturn(keyspaceMetadata);
825+
when(keyspaceMetadata.getTable(table))
826+
.thenReturn(mock(com.datastax.driver.core.TableMetadata.class));
827+
828+
CassandraAdmin adminSpy = spy(cassandraAdmin);
829+
830+
// Act
831+
adminSpy.repairTable(namespace, table, tableMetadata, Collections.emptyMap());
832+
833+
// Assert
834+
verify(adminSpy)
835+
.createSecondaryIndexes(namespace, table, tableMetadata.getSecondaryIndexNames(), true);
836+
}
837+
800838
@Test
801839
public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException {
802840
// Arrange

‎core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,13 @@ public void repairTable_withStoredProcedure_ShouldNotAddStoredProcedure()
893893
when(scripts.getStoredProcedure(CosmosAdmin.STORED_PROCEDURE_FILE_NAME))
894894
.thenReturn(storedProcedure);
895895

896+
// Existing container properties
897+
CosmosContainerResponse response = mock(CosmosContainerResponse.class);
898+
when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey"))
899+
.thenReturn(response);
900+
CosmosContainerProperties properties = mock(CosmosContainerProperties.class);
901+
when(response.getProperties()).thenReturn(properties);
902+
896903
// Act Assert
897904
admin.repairTable(namespace, table, tableMetadata, Collections.emptyMap());
898905

@@ -943,6 +950,13 @@ public void repairTable_withoutStoredProcedure_ShouldCreateStoredProcedure()
943950
when(cosmosException.getStatusCode()).thenReturn(404);
944951
when(storedProcedure.read()).thenThrow(cosmosException);
945952

953+
// Existing container properties
954+
CosmosContainerResponse response = mock(CosmosContainerResponse.class);
955+
when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey"))
956+
.thenReturn(response);
957+
CosmosContainerProperties properties = mock(CosmosContainerProperties.class);
958+
when(response.getProperties()).thenReturn(properties);
959+
946960
// Act
947961
admin.repairTable(namespace, table, tableMetadata, Collections.emptyMap());
948962

@@ -996,6 +1010,13 @@ public void repairTable_ShouldCreateTableIfNotExistsAndUpsertMetadata()
9961010
when(cosmosException.getStatusCode()).thenReturn(404);
9971011
when(storedProcedure.read()).thenThrow(cosmosException);
9981012

1013+
// Existing container properties
1014+
CosmosContainerResponse response = mock(CosmosContainerResponse.class);
1015+
when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey"))
1016+
.thenReturn(response);
1017+
CosmosContainerProperties properties = mock(CosmosContainerProperties.class);
1018+
when(response.getProperties()).thenReturn(properties);
1019+
9991020
// Act
10001021
admin.repairTable(namespace, table, tableMetadata, Collections.emptyMap());
10011022

@@ -1024,6 +1045,49 @@ public void repairTable_ShouldCreateTableIfNotExistsAndUpsertMetadata()
10241045
verify(scripts).createStoredProcedure(any());
10251046
}
10261047

1048+
@Test
1049+
public void repairTable_WhenTableAlreadyExistsWithoutIndex_ShouldCreateIndex()
1050+
throws ExecutionException {
1051+
// Arrange
1052+
String namespace = "ns";
1053+
String table = "tbl";
1054+
TableMetadata tableMetadata =
1055+
TableMetadata.newBuilder()
1056+
.addColumn("c1", DataType.INT)
1057+
.addColumn("c2", DataType.TEXT)
1058+
.addColumn("c3", DataType.BIGINT)
1059+
.addPartitionKey("c1")
1060+
.addSecondaryIndex("c3")
1061+
.build();
1062+
when(client.getDatabase(namespace)).thenReturn(database);
1063+
when(database.getContainer(table)).thenReturn(container);
1064+
// Metadata container
1065+
CosmosContainer metadataContainer = mock(CosmosContainer.class);
1066+
CosmosDatabase metadataDatabase = mock(CosmosDatabase.class);
1067+
when(client.getDatabase(METADATA_DATABASE)).thenReturn(metadataDatabase);
1068+
when(metadataDatabase.getContainer(CosmosAdmin.TABLE_METADATA_CONTAINER))
1069+
.thenReturn(metadataContainer);
1070+
// Stored procedure exists
1071+
CosmosScripts scripts = mock(CosmosScripts.class);
1072+
when(container.getScripts()).thenReturn(scripts);
1073+
CosmosStoredProcedure storedProcedure = mock(CosmosStoredProcedure.class);
1074+
when(scripts.getStoredProcedure(CosmosAdmin.STORED_PROCEDURE_FILE_NAME))
1075+
.thenReturn(storedProcedure);
1076+
// Existing container properties
1077+
CosmosContainerResponse response = mock(CosmosContainerResponse.class);
1078+
when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey"))
1079+
.thenReturn(response);
1080+
CosmosContainerProperties properties = mock(CosmosContainerProperties.class);
1081+
when(response.getProperties()).thenReturn(properties);
1082+
1083+
// Act
1084+
admin.repairTable(namespace, table, tableMetadata, Collections.emptyMap());
1085+
1086+
// Assert
1087+
verify(container, times(1)).replace(properties);
1088+
verify(properties, times(1)).setIndexingPolicy(any(IndexingPolicy.class));
1089+
}
1090+
10271091
@Test
10281092
public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException {
10291093
// Arrange

‎core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,6 +1302,75 @@ public void repairTable_WithNonExistingTableAndMetadataTables_shouldCreateBothTa
13021302
.build());
13031303
}
13041304

1305+
@Test
1306+
public void repairTable_WhenTableAlreadyExistsWithoutIndex_ShouldCreateIndex()
1307+
throws ExecutionException {
1308+
// Arrange
1309+
TableMetadata metadata =
1310+
TableMetadata.newBuilder()
1311+
.addPartitionKey("c1")
1312+
.addColumn("c1", DataType.TEXT)
1313+
.addColumn("c2", DataType.INT)
1314+
.addColumn("c3", DataType.INT)
1315+
.addSecondaryIndex("c3")
1316+
.build();
1317+
// The table to repair exists
1318+
when(client.describeTable(DescribeTableRequest.builder().tableName(getFullTableName()).build()))
1319+
.thenReturn(tableIsActiveResponse);
1320+
// The metadata table exists
1321+
when(client.describeTable(
1322+
DescribeTableRequest.builder().tableName(getFullMetadataTableName()).build()))
1323+
.thenReturn(tableIsActiveResponse);
1324+
// Continuous backup check
1325+
when(client.describeContinuousBackups(any(DescribeContinuousBackupsRequest.class)))
1326+
.thenReturn(backupIsEnabledResponse);
1327+
// Table metadata to return in createIndex
1328+
GetItemResponse getItemResponse = mock(GetItemResponse.class);
1329+
when(client.getItem(any(GetItemRequest.class))).thenReturn(getItemResponse);
1330+
when(getItemResponse.item())
1331+
.thenReturn(
1332+
ImmutableMap.<String, AttributeValue>builder()
1333+
.put(
1334+
DynamoAdmin.METADATA_ATTR_TABLE,
1335+
AttributeValue.builder().s(getFullTableName()).build())
1336+
.put(
1337+
DynamoAdmin.METADATA_ATTR_COLUMNS,
1338+
AttributeValue.builder()
1339+
.m(
1340+
ImmutableMap.<String, AttributeValue>builder()
1341+
.put("c1", AttributeValue.builder().s("text").build())
1342+
.put("c2", AttributeValue.builder().s("int").build())
1343+
.put("c3", AttributeValue.builder().s("int").build())
1344+
.build())
1345+
.build())
1346+
.put(
1347+
DynamoAdmin.METADATA_ATTR_PARTITION_KEY,
1348+
AttributeValue.builder().l(AttributeValue.builder().s("c1").build()).build())
1349+
.put(
1350+
DynamoAdmin.METADATA_ATTR_SECONDARY_INDEX,
1351+
AttributeValue.builder().ss("c3").build())
1352+
.build());
1353+
// For waitForTableCreation in createIndex
1354+
DescribeTableResponse describeTableResponse = mock(DescribeTableResponse.class);
1355+
when(client.describeTable(any(DescribeTableRequest.class))).thenReturn(describeTableResponse);
1356+
TableDescription tableDescription = mock(TableDescription.class);
1357+
when(describeTableResponse.table()).thenReturn(tableDescription);
1358+
GlobalSecondaryIndexDescription globalSecondaryIndexDescription =
1359+
mock(GlobalSecondaryIndexDescription.class);
1360+
when(tableDescription.globalSecondaryIndexes())
1361+
.thenReturn(Collections.emptyList())
1362+
.thenReturn(Collections.singletonList(globalSecondaryIndexDescription));
1363+
String indexName = getFullTableName() + ".global_index.c3";
1364+
when(globalSecondaryIndexDescription.indexName()).thenReturn(indexName);
1365+
when(globalSecondaryIndexDescription.indexStatus()).thenReturn(IndexStatus.ACTIVE);
1366+
1367+
// Act
1368+
admin.repairTable(NAMESPACE, TABLE, metadata, ImmutableMap.of());
1369+
1370+
// Assert
1371+
verify(client, times(1)).updateTable(any(UpdateTableRequest.class));
1372+
}
1373+
13051374
@Test
13061375
public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException {
13071376
// Arrange

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,6 +1483,36 @@ public void repairTable_ShouldCallCreateTableAndAddTableMetadataCorrectly(RdbEng
14831483
verify(adminSpy).addTableMetadata(connection, namespace, table, metadata, true, true);
14841484
}
14851485

1486+
@ParameterizedTest
1487+
@EnumSource(RdbEngine.class)
1488+
public void repairTable_WhenTableAlreadyExistsWithoutIndex_ShouldCreateIndex(RdbEngine rdbEngine)
1489+
throws ExecutionException, SQLException {
1490+
// Arrange
1491+
String namespace = "my_ns";
1492+
String table = "foo_table";
1493+
TableMetadata metadata =
1494+
TableMetadata.newBuilder()
1495+
.addPartitionKey("c1")
1496+
.addClusteringKey("c2")
1497+
.addColumn("c1", DataType.INT)
1498+
.addColumn("c2", DataType.TEXT)
1499+
.addColumn("c3", DataType.BOOLEAN)
1500+
.addColumn("c4", DataType.BLOB)
1501+
.addSecondaryIndex("c3")
1502+
.addSecondaryIndex("c4")
1503+
.build();
1504+
when(connection.createStatement()).thenReturn(mock(Statement.class));
1505+
when(dataSource.getConnection()).thenReturn(connection);
1506+
JdbcAdmin adminSpy = spy(createJdbcAdminFor(rdbEngine));
1507+
1508+
// Act
1509+
adminSpy.repairTable(namespace, table, metadata, Collections.emptyMap());
1510+
1511+
// Assert
1512+
verify(adminSpy).createIndex(connection, namespace, table, "c3", true);
1513+
verify(adminSpy).createIndex(connection, namespace, table, "c4", true);
1514+
}
1515+
14861516
@Test
14871517
public void
14881518
createMetadataTableIfNotExists_WithInternalDbError_forMysql_shouldThrowInternalDbError()

‎integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminRepairIntegrationTestBase.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.scalar.db.api;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatCode;
45

56
import com.scalar.db.exception.storage.ExecutionException;
67
import com.scalar.db.io.DataType;
@@ -212,6 +213,21 @@ public void repairTable_ForNonExistingTableButExistingMetadata_ShouldCreateTable
212213
assertThat(admin.getTableMetadata(getNamespace(), getTable())).isEqualTo(getTableMetadata());
213214
}
214215

216+
@Test
217+
public void
218+
repairTable_WhenTableAlreadyExistsWithoutIndexAndMetadataSpecifiesIndex_ShouldCreateIndex()
219+
throws Exception {
220+
// Arrange
221+
admin.dropIndex(getNamespace(), getTable(), COL_NAME5);
222+
223+
// Act
224+
admin.repairTable(getNamespace(), getTable(), getTableMetadata(), getCreationOptions());
225+
226+
// Assert
227+
assertThatCode(() -> admin.dropIndex(getNamespace(), getTable(), COL_NAME5))
228+
.doesNotThrowAnyException();
229+
}
230+
215231
@Test
216232
public void repairNamespace_ForExistingNamespace_ShouldDoNothing() throws Exception {
217233
// Act

0 commit comments

Comments
 (0)