From 8c0496587d7b19443bd24f01df6dfb2c90358a01 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Mon, 16 Jun 2025 15:48:04 +0900 Subject: [PATCH 1/3] Fix the DynamoDB metadata table upsert operation to handle retries when table is not yet available. --- .../scalar/db/storage/dynamo/DynamoAdmin.java | 61 +++++++++++++------ 1 file changed, 43 insertions(+), 18 deletions(-) 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 99fd6632bd..22690c10a8 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 @@ -97,6 +97,7 @@ public class DynamoAdmin implements DistributedStorageAdmin { public static final String DEFAULT_NO_BACKUP = "false"; public static final String DEFAULT_REQUEST_UNIT = "10"; private static final int DEFAULT_WAITING_DURATION_SECS = 3; + private static final int DEFAULT_MAX_RETRY_COUNT = 10; @VisibleForTesting static final String PARTITION_KEY = "concatenatedPartitionKey"; @VisibleForTesting static final String CLUSTERING_KEY = "concatenatedClusteringKey"; @@ -238,15 +239,26 @@ public void createNamespace(String nonPrefixedNamespace, Map opt private void upsertIntoNamespacesTable(Namespace namespace) throws ExecutionException { Map itemValues = new HashMap<>(); itemValues.put(NAMESPACES_ATTR_NAME, AttributeValue.builder().s(namespace.prefixed()).build()); - try { - client.putItem( - PutItemRequest.builder() - .tableName(ScalarDbUtils.getFullTableName(metadataNamespace, NAMESPACES_TABLE)) - .item(itemValues) - .build()); - } catch (Exception e) { - throw new ExecutionException( - "Inserting the " + namespace + " namespace into the namespaces table failed", e); + int retryCount = 0; + while (true) { + try { + client.putItem( + PutItemRequest.builder() + .tableName(ScalarDbUtils.getFullTableName(metadataNamespace, NAMESPACES_TABLE)) + .item(itemValues) + .build()); + return; + } catch (ResourceNotFoundException e) { + if (retryCount >= DEFAULT_MAX_RETRY_COUNT) { + throw new ExecutionException( + "Inserting the " + namespace + " namespace into the namespaces table failed", e); + } + Uninterruptibles.sleepUninterruptibly(waitingDurationSecs, TimeUnit.SECONDS); + retryCount++; + } catch (Exception e) { + throw new ExecutionException( + "Inserting the " + namespace + " namespace into the namespaces table failed", e); + } } } @@ -470,15 +482,28 @@ private void upsertTableMetadata(Namespace namespace, String table, TableMetadat METADATA_ATTR_SECONDARY_INDEX, AttributeValue.builder().ss(metadata.getSecondaryIndexNames()).build()); } - try { - client.putItem( - PutItemRequest.builder() - .tableName(ScalarDbUtils.getFullTableName(metadataNamespace, METADATA_TABLE)) - .item(itemValues) - .build()); - } catch (Exception e) { - throw new ExecutionException( - "Adding the metadata for the " + getFullTableName(namespace, table) + " table failed", e); + int retryCount = 0; + while (true) { + try { + client.putItem( + PutItemRequest.builder() + .tableName(ScalarDbUtils.getFullTableName(metadataNamespace, METADATA_TABLE)) + .item(itemValues) + .build()); + return; + } catch (ResourceNotFoundException e) { + if (retryCount >= DEFAULT_MAX_RETRY_COUNT) { + throw new ExecutionException( + "Adding the metadata for the " + getFullTableName(namespace, table) + " table failed", + e); + } + Uninterruptibles.sleepUninterruptibly(waitingDurationSecs, TimeUnit.SECONDS); + retryCount++; + } catch (Exception e) { + throw new ExecutionException( + "Adding the metadata for the " + getFullTableName(namespace, table) + " table failed", + e); + } } } From 67c5e94937b471ccb8e1c2da8cd3728c7e6df84c Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Tue, 17 Jun 2025 15:29:00 +0900 Subject: [PATCH 2/3] Add unit tests --- .../scalar/db/storage/dynamo/DynamoAdmin.java | 2 +- .../storage/dynamo/DynamoAdminTestBase.java | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) 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 22690c10a8..d2333e5e33 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 @@ -97,7 +97,7 @@ public class DynamoAdmin implements DistributedStorageAdmin { public static final String DEFAULT_NO_BACKUP = "false"; public static final String DEFAULT_REQUEST_UNIT = "10"; private static final int DEFAULT_WAITING_DURATION_SECS = 3; - private static final int DEFAULT_MAX_RETRY_COUNT = 10; + @VisibleForTesting static final int DEFAULT_MAX_RETRY_COUNT = 10; @VisibleForTesting static final String PARTITION_KEY = "concatenatedPartitionKey"; @VisibleForTesting static final String CLUSTERING_KEY = "concatenatedClusteringKey"; 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 c2072e9445..e5a1f74b16 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 @@ -703,6 +703,39 @@ public void createTable_WhenMetadataTableExists_ShouldCreateOnlyTable() .isInstanceOf(IllegalArgumentException.class); } + @Test + public void createTable_WhenActualMetadataTableCreationIsDelayed_ShouldFailAfterRetries() { + // Arrange + when(client.describeTable(any(DescribeTableRequest.class))).thenReturn(tableIsActiveResponse); + when(client.describeContinuousBackups(any(DescribeContinuousBackupsRequest.class))) + .thenReturn(backupIsEnabledResponse); + when(client.putItem(any(PutItemRequest.class))).thenThrow(ResourceNotFoundException.class); + TableMetadata metadata = + TableMetadata.newBuilder() + .addPartitionKey("c1") + .addClusteringKey("c2", Order.DESC) + .addClusteringKey("c3", Order.ASC) + .addColumn("c1", DataType.TEXT) + .addColumn("c2", DataType.BIGINT) + .addColumn("c3", DataType.BOOLEAN) + .addColumn("c4", DataType.INT) + .addColumn("c5", DataType.BLOB) + .addColumn("c6", DataType.DOUBLE) + .addColumn("c7", DataType.FLOAT) + .addColumn("c8", DataType.DATE) + .addColumn("c9", DataType.TIME) + .addColumn("c10", DataType.TIMESTAMP) + .addColumn("c11", DataType.TIMESTAMPTZ) + .addSecondaryIndex("c4") + .build(); + + // Act Assert + assertThatThrownBy(() -> admin.createTable(NAMESPACE, TABLE, metadata)) + .isInstanceOf(ExecutionException.class); + verify(client, times(DynamoAdmin.DEFAULT_MAX_RETRY_COUNT + 1)) + .putItem(any(PutItemRequest.class)); + } + @Test public void dropTable_WithNoMetadataLeft_ShouldDropTableAndDeleteMetadata() throws ExecutionException { @@ -1436,6 +1469,18 @@ public void createNamespace_WithExistingNamespacesTable_ShouldAddNamespace() .build()); } + @Test + public void createNamespace_WhenActualNamespaceTableCreationIsDelayed_ShouldFailAfterRetries() { + // Arrange + when(client.putItem(any(PutItemRequest.class))).thenThrow(ResourceNotFoundException.class); + + // Act Assert + assertThatThrownBy(() -> admin.createNamespace(NAMESPACE, Collections.emptyMap())) + .isInstanceOf(ExecutionException.class); + verify(client, times(DynamoAdmin.DEFAULT_MAX_RETRY_COUNT + 1)) + .putItem(any(PutItemRequest.class)); + } + @Test public void namespaceExists_WithExistingNamespace_ShouldReturnTrue() throws ExecutionException { // Arrange From 0ee5ca2c436eebd481003cb167a5c4964cfcea35 Mon Sep 17 00:00:00 2001 From: Kodai Doki Date: Tue, 17 Jun 2025 17:52:56 +0900 Subject: [PATCH 3/3] Apply the suggestion --- .../main/java/com/scalar/db/storage/dynamo/DynamoAdmin.java | 6 +++--- .../com/scalar/db/storage/dynamo/DynamoAdminTestBase.java | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) 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 d2333e5e33..ffcb05e8ce 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 @@ -97,7 +97,7 @@ public class DynamoAdmin implements DistributedStorageAdmin { public static final String DEFAULT_NO_BACKUP = "false"; public static final String DEFAULT_REQUEST_UNIT = "10"; private static final int DEFAULT_WAITING_DURATION_SECS = 3; - @VisibleForTesting static final int DEFAULT_MAX_RETRY_COUNT = 10; + @VisibleForTesting static final int MAX_RETRY_COUNT = 10; @VisibleForTesting static final String PARTITION_KEY = "concatenatedPartitionKey"; @VisibleForTesting static final String CLUSTERING_KEY = "concatenatedClusteringKey"; @@ -249,7 +249,7 @@ private void upsertIntoNamespacesTable(Namespace namespace) throws ExecutionExce .build()); return; } catch (ResourceNotFoundException e) { - if (retryCount >= DEFAULT_MAX_RETRY_COUNT) { + if (retryCount >= MAX_RETRY_COUNT) { throw new ExecutionException( "Inserting the " + namespace + " namespace into the namespaces table failed", e); } @@ -492,7 +492,7 @@ private void upsertTableMetadata(Namespace namespace, String table, TableMetadat .build()); return; } catch (ResourceNotFoundException e) { - if (retryCount >= DEFAULT_MAX_RETRY_COUNT) { + if (retryCount >= MAX_RETRY_COUNT) { throw new ExecutionException( "Adding the metadata for the " + getFullTableName(namespace, table) + " table failed", e); 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 e5a1f74b16..91a35740b4 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 @@ -732,8 +732,7 @@ public void createTable_WhenActualMetadataTableCreationIsDelayed_ShouldFailAfter // Act Assert assertThatThrownBy(() -> admin.createTable(NAMESPACE, TABLE, metadata)) .isInstanceOf(ExecutionException.class); - verify(client, times(DynamoAdmin.DEFAULT_MAX_RETRY_COUNT + 1)) - .putItem(any(PutItemRequest.class)); + verify(client, times(DynamoAdmin.MAX_RETRY_COUNT + 1)).putItem(any(PutItemRequest.class)); } @Test @@ -1477,8 +1476,7 @@ public void createNamespace_WhenActualNamespaceTableCreationIsDelayed_ShouldFail // Act Assert assertThatThrownBy(() -> admin.createNamespace(NAMESPACE, Collections.emptyMap())) .isInstanceOf(ExecutionException.class); - verify(client, times(DynamoAdmin.DEFAULT_MAX_RETRY_COUNT + 1)) - .putItem(any(PutItemRequest.class)); + verify(client, times(DynamoAdmin.MAX_RETRY_COUNT + 1)).putItem(any(PutItemRequest.class)); } @Test