diff --git a/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java b/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java index 9c6f4c2934..43fc00b41c 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cassandra/ConsensusCommitAdminIntegrationTestWithCassandra.java @@ -2,13 +2,15 @@ import com.scalar.db.config.DatabaseConfig; import com.scalar.db.transaction.consensuscommit.ConsensusCommitAdminIntegrationTestBase; +import com.scalar.db.transaction.consensuscommit.ConsensusCommitConfig; +import com.scalar.db.transaction.consensuscommit.Coordinator; import java.util.Properties; public class ConsensusCommitAdminIntegrationTestWithCassandra extends ConsensusCommitAdminIntegrationTestBase { @Override protected Properties getProps(String testName) { - return CassandraEnv.getProperties(testName); + return ConsensusCommitCassandraEnv.getProperties(testName); } @Override @@ -17,4 +19,20 @@ protected String getSystemNamespaceName(Properties properties) { .getSystemNamespaceName() .orElse(DatabaseConfig.DEFAULT_SYSTEM_NAMESPACE_NAME); } + + @Override + protected String getCoordinatorNamespaceName(String testName) { + return new ConsensusCommitConfig(new DatabaseConfig(getProperties(testName))) + .getCoordinatorNamespace() + .orElse(Coordinator.NAMESPACE); + } + + @Override + protected boolean isGroupCommitEnabled(String testName) { + return new ConsensusCommitConfig(new DatabaseConfig(getProperties(testName))) + .isCoordinatorGroupCommitEnabled(); + } + + @Override + protected void extraCheckOnCoordinatorTable() {} } diff --git a/core/src/integration-test/java/com/scalar/db/storage/cosmos/ConsensusCommitAdminIntegrationTestWithCosmos.java b/core/src/integration-test/java/com/scalar/db/storage/cosmos/ConsensusCommitAdminIntegrationTestWithCosmos.java index f8e6756eff..061dbb7e6d 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/cosmos/ConsensusCommitAdminIntegrationTestWithCosmos.java +++ b/core/src/integration-test/java/com/scalar/db/storage/cosmos/ConsensusCommitAdminIntegrationTestWithCosmos.java @@ -2,6 +2,8 @@ import com.scalar.db.config.DatabaseConfig; import com.scalar.db.transaction.consensuscommit.ConsensusCommitAdminIntegrationTestBase; +import com.scalar.db.transaction.consensuscommit.ConsensusCommitConfig; +import com.scalar.db.transaction.consensuscommit.Coordinator; import java.util.Map; import java.util.Properties; @@ -10,12 +12,12 @@ public class ConsensusCommitAdminIntegrationTestWithCosmos @Override protected Properties getProps(String testName) { - return CosmosEnv.getProperties(testName); + return ConsensusCommitCosmosEnv.getProperties(testName); } @Override protected Map getCreationOptions() { - return CosmosEnv.getCreationOptions(); + return ConsensusCommitCosmosEnv.getCreationOptions(); } @Override @@ -24,4 +26,17 @@ protected String getSystemNamespaceName(Properties properties) { .getTableMetadataDatabase() .orElse(DatabaseConfig.DEFAULT_SYSTEM_NAMESPACE_NAME); } + + @Override + protected String getCoordinatorNamespaceName(String testName) { + return new ConsensusCommitConfig(new DatabaseConfig(getProperties(testName))) + .getCoordinatorNamespace() + .orElse(Coordinator.NAMESPACE); + } + + @Override + protected boolean isGroupCommitEnabled(String testName) { + return new ConsensusCommitConfig(new DatabaseConfig(getProperties(testName))) + .isCoordinatorGroupCommitEnabled(); + } } diff --git a/core/src/integration-test/java/com/scalar/db/storage/dynamo/ConsensusCommitAdminIntegrationTestWithDynamo.java b/core/src/integration-test/java/com/scalar/db/storage/dynamo/ConsensusCommitAdminIntegrationTestWithDynamo.java index f2a218c30f..dd6de144da 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/dynamo/ConsensusCommitAdminIntegrationTestWithDynamo.java +++ b/core/src/integration-test/java/com/scalar/db/storage/dynamo/ConsensusCommitAdminIntegrationTestWithDynamo.java @@ -2,6 +2,8 @@ import com.scalar.db.config.DatabaseConfig; import com.scalar.db.transaction.consensuscommit.ConsensusCommitAdminIntegrationTestBase; +import com.scalar.db.transaction.consensuscommit.ConsensusCommitConfig; +import com.scalar.db.transaction.consensuscommit.Coordinator; import java.util.Map; import java.util.Properties; import org.junit.jupiter.api.Disabled; @@ -12,12 +14,12 @@ public class ConsensusCommitAdminIntegrationTestWithDynamo @Override protected Properties getProps(String testName) { - return DynamoEnv.getProperties(testName); + return ConsensusCommitDynamoEnv.getProperties(testName); } @Override protected Map getCreationOptions() { - return DynamoEnv.getCreationOptions(); + return ConsensusCommitDynamoEnv.getCreationOptions(); } @Override @@ -32,6 +34,19 @@ protected String getSystemNamespaceName(Properties properties) { .orElse(DatabaseConfig.DEFAULT_SYSTEM_NAMESPACE_NAME); } + @Override + protected String getCoordinatorNamespaceName(String testName) { + return new ConsensusCommitConfig(new DatabaseConfig(getProperties(testName))) + .getCoordinatorNamespace() + .orElse(Coordinator.NAMESPACE); + } + + @Override + protected boolean isGroupCommitEnabled(String testName) { + return new ConsensusCommitConfig(new DatabaseConfig(getProperties(testName))) + .isCoordinatorGroupCommitEnabled(); + } + // Since DynamoDB doesn't have the namespace concept, some behaviors around the namespace are // different from the other adapters. So disable several tests that check such behaviors diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java index 977e0cd950..f1d943c7d6 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java @@ -3,6 +3,8 @@ import com.scalar.db.config.DatabaseConfig; import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.transaction.consensuscommit.ConsensusCommitAdminIntegrationTestBase; +import com.scalar.db.transaction.consensuscommit.ConsensusCommitConfig; +import com.scalar.db.transaction.consensuscommit.Coordinator; import java.util.Properties; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIf; @@ -12,7 +14,7 @@ public class ConsensusCommitAdminIntegrationTestWithJdbcDatabase @Override protected Properties getProps(String testName) { - return JdbcEnv.getProperties(testName); + return ConsensusCommitJdbcEnv.getProperties(testName); } @Override @@ -22,6 +24,19 @@ protected String getSystemNamespaceName(Properties properties) { .orElse(DatabaseConfig.DEFAULT_SYSTEM_NAMESPACE_NAME); } + @Override + protected String getCoordinatorNamespaceName(String testName) { + return new ConsensusCommitConfig(new DatabaseConfig(getProperties(testName))) + .getCoordinatorNamespace() + .orElse(Coordinator.NAMESPACE); + } + + @Override + protected boolean isGroupCommitEnabled(String testName) { + return new ConsensusCommitConfig(new DatabaseConfig(getProperties(testName))) + .isCoordinatorGroupCommitEnabled(); + } + // Since SQLite doesn't have persistent namespaces, some behaviors around the namespace are // different from the other adapters. So disable several tests that check such behaviors. diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java index afc26fbc14..f60f699653 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdmin.java @@ -27,6 +27,7 @@ @ThreadSafe public class ConsensusCommitAdmin implements DistributedTransactionAdmin { + private final ConsensusCommitConfig config; private final DistributedStorageAdmin admin; private final String coordinatorNamespace; private final boolean isIncludeMetadataEnabled; @@ -35,7 +36,7 @@ public class ConsensusCommitAdmin implements DistributedTransactionAdmin { @Inject public ConsensusCommitAdmin(DistributedStorageAdmin admin, DatabaseConfig databaseConfig) { this.admin = admin; - ConsensusCommitConfig config = new ConsensusCommitConfig(databaseConfig); + config = new ConsensusCommitConfig(databaseConfig); coordinatorNamespace = config.getCoordinatorNamespace().orElse(Coordinator.NAMESPACE); isIncludeMetadataEnabled = config.isIncludeMetadataEnabled(); } @@ -44,7 +45,7 @@ public ConsensusCommitAdmin(DatabaseConfig databaseConfig) { StorageFactory storageFactory = StorageFactory.create(databaseConfig.getProperties()); admin = storageFactory.getStorageAdmin(); - ConsensusCommitConfig config = new ConsensusCommitConfig(databaseConfig); + config = new ConsensusCommitConfig(databaseConfig); coordinatorNamespace = config.getCoordinatorNamespace().orElse(Coordinator.NAMESPACE); isIncludeMetadataEnabled = config.isIncludeMetadataEnabled(); } @@ -54,6 +55,7 @@ public ConsensusCommitAdmin(DatabaseConfig databaseConfig) { DistributedStorageAdmin admin, ConsensusCommitConfig config, boolean isIncludeMetadataEnabled) { + this.config = config; this.admin = admin; coordinatorNamespace = config.getCoordinatorNamespace().orElse(Coordinator.NAMESPACE); this.isIncludeMetadataEnabled = isIncludeMetadataEnabled; @@ -67,7 +69,8 @@ public void createCoordinatorTables(Map options) throws Executio } admin.createNamespace(coordinatorNamespace, options); - admin.createTable(coordinatorNamespace, Coordinator.TABLE, Coordinator.TABLE_METADATA, options); + admin.createTable( + coordinatorNamespace, Coordinator.TABLE, getCoordinatorTableMetadata(), options); } @Override @@ -207,7 +210,8 @@ public void repairTable( @Override public void repairCoordinatorTables(Map options) throws ExecutionException { - admin.repairTable(coordinatorNamespace, Coordinator.TABLE, Coordinator.TABLE_METADATA, options); + admin.repairTable( + coordinatorNamespace, Coordinator.TABLE, getCoordinatorTableMetadata(), options); } @Override @@ -276,4 +280,12 @@ private void checkNamespace(String namespace) { CoreError.CONSENSUS_COMMIT_COORDINATOR_NAMESPACE_SPECIFIED.buildMessage(namespace)); } } + + private TableMetadata getCoordinatorTableMetadata() { + if (config.isCoordinatorGroupCommitEnabled()) { + return Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_ENABLED; + } else { + return Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_DISABLED; + } + } } diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/Coordinator.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/Coordinator.java index 3cdfb7dd8d..58863b6f84 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/Coordinator.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/Coordinator.java @@ -37,7 +37,14 @@ public class Coordinator { public static final String NAMESPACE = "coordinator"; public static final String TABLE = "state"; - public static final TableMetadata TABLE_METADATA = + public static final TableMetadata TABLE_METADATA_WITH_GROUP_COMMIT_DISABLED = + TableMetadata.newBuilder() + .addColumn(Attribute.ID, DataType.TEXT) + .addColumn(Attribute.STATE, DataType.INT) + .addColumn(Attribute.CREATED_AT, DataType.BIGINT) + .addPartitionKey(Attribute.ID) + .build(); + public static final TableMetadata TABLE_METADATA_WITH_GROUP_COMMIT_ENABLED = TableMetadata.newBuilder() .addColumn(Attribute.ID, DataType.TEXT) .addColumn(Attribute.CHILD_IDS, DataType.TEXT) diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminTestBase.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminTestBase.java index e4468b3632..1aac11df3d 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminTestBase.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminTestBase.java @@ -47,6 +47,7 @@ public abstract class ConsensusCommitAdminTestBase { public void setUp() throws Exception { MockitoAnnotations.openMocks(this).close(); when(config.getCoordinatorNamespace()).thenReturn(getCoordinatorNamespaceConfig()); + when(config.isCoordinatorGroupCommitEnabled()).thenReturn(false); admin = new ConsensusCommitAdmin(distributedStorageAdmin, config, false); coordinatorNamespaceName = getCoordinatorNamespaceConfig().orElse(Coordinator.NAMESPACE); } @@ -68,7 +69,29 @@ public void createCoordinatorTables_shouldCreateCoordinatorTableProperly() .createTable( coordinatorNamespaceName, Coordinator.TABLE, - Coordinator.TABLE_METADATA, + Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_DISABLED, + Collections.emptyMap()); + } + + @Test + public void createCoordinatorTables_WithGroupCommitEnabled_shouldCreateCoordinatorTableProperly() + throws ExecutionException { + // Arrange + when(config.isCoordinatorGroupCommitEnabled()).thenReturn(true); + ConsensusCommitAdmin adminWithGroupCommit = + new ConsensusCommitAdmin(distributedStorageAdmin, config, false); + + // Act + adminWithGroupCommit.createCoordinatorTables(); + + // Assert + verify(distributedStorageAdmin) + .createNamespace(coordinatorNamespaceName, Collections.emptyMap()); + verify(distributedStorageAdmin) + .createTable( + coordinatorNamespaceName, + Coordinator.TABLE, + Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_ENABLED, Collections.emptyMap()); } @@ -98,7 +121,34 @@ public void createCoordinatorTables_WithOptions_shouldCreateCoordinatorTableProp verify(distributedStorageAdmin).createNamespace(coordinatorNamespaceName, options); verify(distributedStorageAdmin) .createTable( - coordinatorNamespaceName, Coordinator.TABLE, Coordinator.TABLE_METADATA, options); + coordinatorNamespaceName, + Coordinator.TABLE, + Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_DISABLED, + options); + } + + @Test + public void + createCoordinatorTables_WithOptionsWithGroupCommitEnabled_shouldCreateCoordinatorTableProperly() + throws ExecutionException { + // Arrange + when(config.isCoordinatorGroupCommitEnabled()).thenReturn(true); + ConsensusCommitAdmin adminWithGroupCommit = + new ConsensusCommitAdmin(distributedStorageAdmin, config, false); + + Map options = ImmutableMap.of("name", "value"); + + // Act + adminWithGroupCommit.createCoordinatorTables(options); + + // Assert + verify(distributedStorageAdmin).createNamespace(coordinatorNamespaceName, options); + verify(distributedStorageAdmin) + .createTable( + coordinatorNamespaceName, + Coordinator.TABLE, + Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_ENABLED, + options); } @Test @@ -570,7 +620,32 @@ public void repairCoordinatorTables_ShouldCallJdbcAdminProperly() throws Executi // Assert verify(distributedStorageAdmin) .repairTable( - coordinatorNamespaceName, Coordinator.TABLE, Coordinator.TABLE_METADATA, options); + coordinatorNamespaceName, + Coordinator.TABLE, + Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_DISABLED, + options); + } + + @Test + public void repairCoordinatorTables_WithGroupCommitEnabled_ShouldCallJdbcAdminProperly() + throws ExecutionException { + // Arrange + when(config.isCoordinatorGroupCommitEnabled()).thenReturn(true); + ConsensusCommitAdmin adminWithGroupCommit = + new ConsensusCommitAdmin(distributedStorageAdmin, config, false); + + Map options = ImmutableMap.of("foo", "bar"); + + // Act + adminWithGroupCommit.repairCoordinatorTables(options); + + // Assert + verify(distributedStorageAdmin) + .repairTable( + coordinatorNamespaceName, + Coordinator.TABLE, + Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_ENABLED, + options); } @Test diff --git a/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java index 347b21627b..b1b15f4960 100644 --- a/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java @@ -8,7 +8,9 @@ import com.scalar.db.exception.transaction.TransactionException; import com.scalar.db.io.DataType; import com.scalar.db.io.Key; +import com.scalar.db.service.StorageFactory; import com.scalar.db.service.TransactionFactory; +import com.scalar.db.transaction.consensuscommit.Coordinator; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; @@ -69,6 +71,7 @@ public abstract class DistributedTransactionAdminIntegrationTestBase { .build(); protected TransactionFactory transactionFactory; protected DistributedTransactionAdmin admin; + protected DistributedStorageAdmin storageAdmin; protected String systemNamespaceName; protected String namespace1; protected String namespace2; @@ -81,6 +84,7 @@ public void beforeAll() throws Exception { Properties properties = getProperties(testName); transactionFactory = TransactionFactory.create(properties); admin = transactionFactory.getTransactionAdmin(); + storageAdmin = StorageFactory.create(properties).getStorageAdmin(); systemNamespaceName = getSystemNamespaceName(properties); namespace1 = getNamespaceBaseName() + testName + "1"; namespace2 = getNamespaceBaseName() + testName + "2"; @@ -100,6 +104,14 @@ protected String getNamespaceBaseName() { protected abstract String getSystemNamespaceName(Properties properties); + protected String getCoordinatorNamespaceName(String testName) { + throw new AssertionError("Shouldn't be called"); + } + + protected boolean isGroupCommitEnabled(String testName) { + return false; + } + private void createTables() throws ExecutionException { Map options = getCreationOptions(); for (String namespace : Arrays.asList(namespace1, namespace2)) { @@ -123,6 +135,14 @@ public void afterAll() throws Exception { logger.warn("Failed to drop tables", e); } + try { + if (storageAdmin != null) { + storageAdmin.close(); + } + } catch (Exception e) { + logger.warn("Failed to close storage admin", e); + } + try { if (admin != null) { admin.close(); @@ -764,6 +784,19 @@ public void addNewColumnToTable_ForAlreadyExistingColumn_ShouldThrowIllegalArgum .isInstanceOf(IllegalArgumentException.class); } + protected void extraCheckOnCoordinatorTable() throws ExecutionException { + TableMetadata expectedMetadata; + if (isGroupCommitEnabled(getTestName())) { + expectedMetadata = Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_ENABLED; + } else { + expectedMetadata = Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_DISABLED; + } + assertThat( + storageAdmin.getTableMetadata( + getCoordinatorNamespaceName(getTestName()), Coordinator.TABLE)) + .isEqualTo(expectedMetadata); + } + @Test public void createCoordinatorTables_ShouldCreateCoordinatorTablesCorrectly() throws ExecutionException { @@ -775,6 +808,8 @@ public void createCoordinatorTables_ShouldCreateCoordinatorTablesCorrectly() // Assert assertThat(admin.coordinatorTablesExist()).isTrue(); + + extraCheckOnCoordinatorTable(); } @Test diff --git a/integration-test/src/main/java/com/scalar/db/util/AdminTestUtils.java b/integration-test/src/main/java/com/scalar/db/util/AdminTestUtils.java index 51cc0a3052..3c80bb9bb4 100644 --- a/integration-test/src/main/java/com/scalar/db/util/AdminTestUtils.java +++ b/integration-test/src/main/java/com/scalar/db/util/AdminTestUtils.java @@ -49,10 +49,9 @@ public AdminTestUtils(Properties coordinatorStorageProperties) { * @throws Exception if an error occurs */ public boolean areTableMetadataForCoordinatorTablesPresent() throws Exception { - String coordinatorNamespace = - new ConsensusCommitConfig(new DatabaseConfig(coordinatorStorageProperties)) - .getCoordinatorNamespace() - .orElse(Coordinator.NAMESPACE); + ConsensusCommitConfig config = + new ConsensusCommitConfig(new DatabaseConfig(coordinatorStorageProperties)); + String coordinatorNamespace = config.getCoordinatorNamespace().orElse(Coordinator.NAMESPACE); String coordinatorTable = Coordinator.TABLE; // Use the DistributedStorageAdmin instead of the DistributedTransactionAdmin because the latter // expects the table to hold transaction table metadata columns which is not the case for the @@ -66,6 +65,12 @@ public boolean areTableMetadataForCoordinatorTablesPresent() throws Exception { if (tableMetadata == null) { return false; } - return tableMetadata.equals(Coordinator.TABLE_METADATA); + TableMetadata expectedMetadata; + if (config.isCoordinatorGroupCommitEnabled()) { + expectedMetadata = Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_ENABLED; + } else { + expectedMetadata = Coordinator.TABLE_METADATA_WITH_GROUP_COMMIT_DISABLED; + } + return tableMetadata.equals(expectedMetadata); } } diff --git a/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminIntegrationTestWithServer.java b/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminIntegrationTestWithServer.java index b31cda466a..cbb5386488 100644 --- a/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminIntegrationTestWithServer.java +++ b/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminIntegrationTestWithServer.java @@ -55,4 +55,7 @@ protected String getSystemNamespaceName(Properties properties) { @Test @Disabled("Retrieving the namespace names is not supported in ScalarDB Server") public void getNamespaceNames_ShouldReturnCreatedNamespaces() {} + + @Override + protected void extraCheckOnCoordinatorTable() {} }