diff --git a/core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java b/core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java index ba04c683a4..ff1c2f3cff 100644 --- a/core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/cosmos/CosmosAdmin.java @@ -588,6 +588,7 @@ public void repairTable( throws ExecutionException { try { createTableInternal(namespace, table, metadata, true); + updateIndexingPolicy(namespace, table, metadata); } catch (IllegalArgumentException e) { throw e; } catch (Exception e) { diff --git a/core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java b/core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java index ffcb05e8ce..5a0b621170 100644 --- a/core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java @@ -1032,6 +1032,46 @@ columnName, getFullTableName(namespace, table)), TableMetadata.newBuilder(tableMetadata).addSecondaryIndex(columnName).build()); } + private boolean rawIndexExists(String nonPrefixedNamespace, String table, String indexName) { + Namespace namespace = Namespace.of(namespacePrefix, nonPrefixedNamespace); + String globalIndexName = + String.join( + ".", + getFullTableName(namespace, table), + DynamoAdmin.GLOBAL_INDEX_NAME_PREFIX, + indexName); + int retryCount = 0; + try { + while (true) { + DescribeTableResponse response = + client.describeTable( + DescribeTableRequest.builder() + .tableName(getFullTableName(namespace, table)) + .build()); + GlobalSecondaryIndexDescription description = + response.table().globalSecondaryIndexes().stream() + .filter(d -> d.indexName().equals(globalIndexName)) + .findFirst() + .orElse(null); + if (description == null) { + return false; + } + if (description.indexStatus() == IndexStatus.ACTIVE) { + return true; + } + if (retryCount++ >= MAX_RETRY_COUNT) { + throw new IllegalStateException( + String.format( + "Waiting for the secondary index %s on the %s table to be active failed", + indexName, getFullTableName(namespace, table))); + } + Uninterruptibles.sleepUninterruptibly(waitingDurationSecs, TimeUnit.SECONDS); + } + } catch (ResourceNotFoundException e) { + return false; + } + } + private void waitForIndexCreation(Namespace namespace, String table, String columnName) throws ExecutionException { try { @@ -1366,6 +1406,11 @@ public void repairTable( throws ExecutionException { try { createTableInternal(nonPrefixedNamespace, table, metadata, true, options); + for (String indexColumnName : metadata.getSecondaryIndexNames()) { + if (!rawIndexExists(nonPrefixedNamespace, table, indexColumnName)) { + createIndex(nonPrefixedNamespace, table, indexColumnName, options); + } + } } catch (RuntimeException e) { throw new ExecutionException( String.format( diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java index 726e8f3665..4bcad3970d 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java @@ -889,7 +889,8 @@ columnName, getFullTableName(namespace, table)), } } - private void createIndex( + @VisibleForTesting + void createIndex( Connection connection, String schema, String table, String indexedColumn, boolean ifNotExists) throws SQLException { String indexName = getIndexName(schema, table, indexedColumn); diff --git a/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java b/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java index e1f216f77d..679b5d0518 100644 --- a/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java @@ -7,6 +7,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -797,6 +798,43 @@ public void repairTable_ShouldCreateTableAndIndexesIfTheyDoNotExists() throws Ex } } + @Test + public void repairTable_WhenTableAlreadyExistsWithoutIndex_ShouldCreateIndex() + throws ExecutionException { + // Arrange + String namespace = "sample_ns"; + String table = "tbl"; + TableMetadata tableMetadata = + TableMetadata.newBuilder() + .addPartitionKey("c1") + .addClusteringKey("c4") + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.TEXT) + .addColumn("c3", DataType.BLOB) + .addColumn("c4", DataType.INT) + .addColumn("c5", DataType.BOOLEAN) + .addSecondaryIndex("c2") + .addSecondaryIndex("c4") + .build(); + // The table already exists + when(clusterManager.getMetadata(namespace, table)) + .thenReturn(mock(com.datastax.driver.core.TableMetadata.class)); + when(cassandraSession.getCluster()).thenReturn(cluster); + when(cluster.getMetadata()).thenReturn(metadata); + when(metadata.getKeyspace(any())).thenReturn(keyspaceMetadata); + when(keyspaceMetadata.getTable(table)) + .thenReturn(mock(com.datastax.driver.core.TableMetadata.class)); + + CassandraAdmin adminSpy = spy(cassandraAdmin); + + // Act + adminSpy.repairTable(namespace, table, tableMetadata, Collections.emptyMap()); + + // Assert + verify(adminSpy) + .createSecondaryIndexes(namespace, table, tableMetadata.getSecondaryIndexNames(), true); + } + @Test public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { // Arrange diff --git a/core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java b/core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java index bfeb562889..7506e98abf 100644 --- a/core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/cosmos/CosmosAdminTest.java @@ -893,6 +893,13 @@ public void repairTable_withStoredProcedure_ShouldNotAddStoredProcedure() when(scripts.getStoredProcedure(CosmosAdmin.STORED_PROCEDURE_FILE_NAME)) .thenReturn(storedProcedure); + // Existing container properties + CosmosContainerResponse response = mock(CosmosContainerResponse.class); + when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey")) + .thenReturn(response); + CosmosContainerProperties properties = mock(CosmosContainerProperties.class); + when(response.getProperties()).thenReturn(properties); + // Act Assert admin.repairTable(namespace, table, tableMetadata, Collections.emptyMap()); @@ -943,6 +950,13 @@ public void repairTable_withoutStoredProcedure_ShouldCreateStoredProcedure() when(cosmosException.getStatusCode()).thenReturn(404); when(storedProcedure.read()).thenThrow(cosmosException); + // Existing container properties + CosmosContainerResponse response = mock(CosmosContainerResponse.class); + when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey")) + .thenReturn(response); + CosmosContainerProperties properties = mock(CosmosContainerProperties.class); + when(response.getProperties()).thenReturn(properties); + // Act admin.repairTable(namespace, table, tableMetadata, Collections.emptyMap()); @@ -996,6 +1010,13 @@ public void repairTable_ShouldCreateTableIfNotExistsAndUpsertMetadata() when(cosmosException.getStatusCode()).thenReturn(404); when(storedProcedure.read()).thenThrow(cosmosException); + // Existing container properties + CosmosContainerResponse response = mock(CosmosContainerResponse.class); + when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey")) + .thenReturn(response); + CosmosContainerProperties properties = mock(CosmosContainerProperties.class); + when(response.getProperties()).thenReturn(properties); + // Act admin.repairTable(namespace, table, tableMetadata, Collections.emptyMap()); @@ -1024,6 +1045,49 @@ public void repairTable_ShouldCreateTableIfNotExistsAndUpsertMetadata() verify(scripts).createStoredProcedure(any()); } + @Test + public void repairTable_WhenTableAlreadyExistsWithoutIndex_ShouldCreateIndex() + throws ExecutionException { + // Arrange + String namespace = "ns"; + String table = "tbl"; + TableMetadata tableMetadata = + TableMetadata.newBuilder() + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.TEXT) + .addColumn("c3", DataType.BIGINT) + .addPartitionKey("c1") + .addSecondaryIndex("c3") + .build(); + when(client.getDatabase(namespace)).thenReturn(database); + when(database.getContainer(table)).thenReturn(container); + // Metadata container + CosmosContainer metadataContainer = mock(CosmosContainer.class); + CosmosDatabase metadataDatabase = mock(CosmosDatabase.class); + when(client.getDatabase(METADATA_DATABASE)).thenReturn(metadataDatabase); + when(metadataDatabase.getContainer(CosmosAdmin.TABLE_METADATA_CONTAINER)) + .thenReturn(metadataContainer); + // Stored procedure exists + CosmosScripts scripts = mock(CosmosScripts.class); + when(container.getScripts()).thenReturn(scripts); + CosmosStoredProcedure storedProcedure = mock(CosmosStoredProcedure.class); + when(scripts.getStoredProcedure(CosmosAdmin.STORED_PROCEDURE_FILE_NAME)) + .thenReturn(storedProcedure); + // Existing container properties + CosmosContainerResponse response = mock(CosmosContainerResponse.class); + when(database.createContainerIfNotExists(table, "/concatenatedPartitionKey")) + .thenReturn(response); + CosmosContainerProperties properties = mock(CosmosContainerProperties.class); + when(response.getProperties()).thenReturn(properties); + + // Act + admin.repairTable(namespace, table, tableMetadata, Collections.emptyMap()); + + // Assert + verify(container, times(1)).replace(properties); + verify(properties, times(1)).setIndexingPolicy(any(IndexingPolicy.class)); + } + @Test public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { // Arrange diff --git a/core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java b/core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java index 91a35740b4..c208d85c43 100644 --- a/core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java +++ b/core/src/test/java/com/scalar/db/storage/dynamo/DynamoAdminTestBase.java @@ -1302,6 +1302,75 @@ public void repairTable_WithNonExistingTableAndMetadataTables_shouldCreateBothTa .build()); } + @Test + public void repairTable_WhenTableAlreadyExistsWithoutIndex_ShouldCreateIndex() + throws ExecutionException { + // Arrange + TableMetadata metadata = + TableMetadata.newBuilder() + .addPartitionKey("c1") + .addColumn("c1", DataType.TEXT) + .addColumn("c2", DataType.INT) + .addColumn("c3", DataType.INT) + .addSecondaryIndex("c3") + .build(); + // The table to repair exists + when(client.describeTable(DescribeTableRequest.builder().tableName(getFullTableName()).build())) + .thenReturn(tableIsActiveResponse); + // The metadata table exists + when(client.describeTable( + DescribeTableRequest.builder().tableName(getFullMetadataTableName()).build())) + .thenReturn(tableIsActiveResponse); + // Continuous backup check + when(client.describeContinuousBackups(any(DescribeContinuousBackupsRequest.class))) + .thenReturn(backupIsEnabledResponse); + // Table metadata to return in createIndex + GetItemResponse getItemResponse = mock(GetItemResponse.class); + when(client.getItem(any(GetItemRequest.class))).thenReturn(getItemResponse); + when(getItemResponse.item()) + .thenReturn( + ImmutableMap.builder() + .put( + DynamoAdmin.METADATA_ATTR_TABLE, + AttributeValue.builder().s(getFullTableName()).build()) + .put( + DynamoAdmin.METADATA_ATTR_COLUMNS, + AttributeValue.builder() + .m( + ImmutableMap.builder() + .put("c1", AttributeValue.builder().s("text").build()) + .put("c2", AttributeValue.builder().s("int").build()) + .put("c3", AttributeValue.builder().s("int").build()) + .build()) + .build()) + .put( + DynamoAdmin.METADATA_ATTR_PARTITION_KEY, + AttributeValue.builder().l(AttributeValue.builder().s("c1").build()).build()) + .put( + DynamoAdmin.METADATA_ATTR_SECONDARY_INDEX, + AttributeValue.builder().ss("c3").build()) + .build()); + // For waitForTableCreation in createIndex + DescribeTableResponse describeTableResponse = mock(DescribeTableResponse.class); + when(client.describeTable(any(DescribeTableRequest.class))).thenReturn(describeTableResponse); + TableDescription tableDescription = mock(TableDescription.class); + when(describeTableResponse.table()).thenReturn(tableDescription); + GlobalSecondaryIndexDescription globalSecondaryIndexDescription = + mock(GlobalSecondaryIndexDescription.class); + when(tableDescription.globalSecondaryIndexes()) + .thenReturn(Collections.emptyList()) + .thenReturn(Collections.singletonList(globalSecondaryIndexDescription)); + String indexName = getFullTableName() + ".global_index.c3"; + when(globalSecondaryIndexDescription.indexName()).thenReturn(indexName); + when(globalSecondaryIndexDescription.indexStatus()).thenReturn(IndexStatus.ACTIVE); + + // Act + admin.repairTable(NAMESPACE, TABLE, metadata, ImmutableMap.of()); + + // Assert + verify(client, times(1)).updateTable(any(UpdateTableRequest.class)); + } + @Test public void addNewColumnToTable_ShouldWorkProperly() throws ExecutionException { // Arrange diff --git a/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java b/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java index a3a32be726..f2c0bede08 100644 --- a/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java +++ b/core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java @@ -1483,6 +1483,36 @@ public void repairTable_ShouldCallCreateTableAndAddTableMetadataCorrectly(RdbEng verify(adminSpy).addTableMetadata(connection, namespace, table, metadata, true, true); } + @ParameterizedTest + @EnumSource(RdbEngine.class) + public void repairTable_WhenTableAlreadyExistsWithoutIndex_ShouldCreateIndex(RdbEngine rdbEngine) + throws ExecutionException, SQLException { + // Arrange + String namespace = "my_ns"; + String table = "foo_table"; + TableMetadata metadata = + TableMetadata.newBuilder() + .addPartitionKey("c1") + .addClusteringKey("c2") + .addColumn("c1", DataType.INT) + .addColumn("c2", DataType.TEXT) + .addColumn("c3", DataType.BOOLEAN) + .addColumn("c4", DataType.BLOB) + .addSecondaryIndex("c3") + .addSecondaryIndex("c4") + .build(); + when(connection.createStatement()).thenReturn(mock(Statement.class)); + when(dataSource.getConnection()).thenReturn(connection); + JdbcAdmin adminSpy = spy(createJdbcAdminFor(rdbEngine)); + + // Act + adminSpy.repairTable(namespace, table, metadata, Collections.emptyMap()); + + // Assert + verify(adminSpy).createIndex(connection, namespace, table, "c3", true); + verify(adminSpy).createIndex(connection, namespace, table, "c4", true); + } + @Test public void createMetadataTableIfNotExists_WithInternalDbError_forMysql_shouldThrowInternalDbError() diff --git a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminRepairIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminRepairIntegrationTestBase.java index 2f52deb7dc..cff7fdeadb 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminRepairIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminRepairIntegrationTestBase.java @@ -1,6 +1,7 @@ package com.scalar.db.api; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.io.DataType; @@ -212,6 +213,21 @@ public void repairTable_ForNonExistingTableButExistingMetadata_ShouldCreateTable assertThat(admin.getTableMetadata(getNamespace(), getTable())).isEqualTo(getTableMetadata()); } + @Test + public void + repairTable_WhenTableAlreadyExistsWithoutIndexAndMetadataSpecifiesIndex_ShouldCreateIndex() + throws Exception { + // Arrange + admin.dropIndex(getNamespace(), getTable(), COL_NAME5); + + // Act + admin.repairTable(getNamespace(), getTable(), getTableMetadata(), getCreationOptions()); + + // Assert + assertThatCode(() -> admin.dropIndex(getNamespace(), getTable(), COL_NAME5)) + .doesNotThrowAnyException(); + } + @Test public void repairNamespace_ForExistingNamespace_ShouldDoNothing() throws Exception { // Act