From 98c8ddaae6f417500fbc81872768699aed36fe68 Mon Sep 17 00:00:00 2001 From: Toshihiro Suzuki Date: Mon, 16 Jan 2023 14:38:02 +0900 Subject: [PATCH] Revert async commit default value (#797) --- .../consensuscommit/ConsensusCommitConfig.java | 2 +- .../consensuscommit/ConsensusCommitConfigTest.java | 12 ++++++------ docs/configurations-for-consensus-commit.md | 2 +- .../ConsensusCommitAdminIntegrationTestBase.java | 4 ---- ...susCommitAdminRepairTableIntegrationTestBase.java | 4 ---- .../ConsensusCommitIntegrationTestBase.java | 4 ---- .../ConsensusCommitSpecificIntegrationTestBase.java | 4 ---- .../TwoPhaseConsensusCommitIntegrationTestBase.java | 4 ---- ...seConsensusCommitSpecificIntegrationTestBase.java | 4 ---- ...onTestWithDistributedTransactionAdminService.java | 4 ---- ...onTestWithDistributedTransactionAdminService.java | 8 -------- ...grationTestWithDistributedTransactionService.java | 4 ---- ...tionTestWithTwoPhaseCommitTransactionService.java | 4 ---- 13 files changed, 8 insertions(+), 52 deletions(-) diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitConfig.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitConfig.java index 51c8ae798e..917a5c320b 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitConfig.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitConfig.java @@ -101,7 +101,7 @@ public ConsensusCommitConfig(DatabaseConfig databaseConfig) { getBoolean( databaseConfig.getProperties(), PARALLEL_ROLLBACK_ENABLED, parallelCommitEnabled); - asyncCommitEnabled = getBoolean(databaseConfig.getProperties(), ASYNC_COMMIT_ENABLED, true); + asyncCommitEnabled = getBoolean(databaseConfig.getProperties(), ASYNC_COMMIT_ENABLED, false); asyncRollbackEnabled = getBoolean(databaseConfig.getProperties(), ASYNC_ROLLBACK_ENABLED, asyncCommitEnabled); isIncludeMetadataEnabled = diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitConfigTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitConfigTest.java index f2b791e2d3..cb145f5998 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitConfigTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitConfigTest.java @@ -26,8 +26,8 @@ public void constructor_NoPropertiesGiven_ShouldLoadAsDefaultValues() { assertThat(config.isParallelValidationEnabled()).isTrue(); assertThat(config.isParallelCommitEnabled()).isTrue(); assertThat(config.isParallelRollbackEnabled()).isTrue(); - assertThat(config.isAsyncCommitEnabled()).isTrue(); - assertThat(config.isAsyncRollbackEnabled()).isTrue(); + assertThat(config.isAsyncCommitEnabled()).isFalse(); + assertThat(config.isAsyncRollbackEnabled()).isFalse(); assertThat(config.isIncludeMetadataEnabled()).isFalse(); } @@ -153,15 +153,15 @@ public void constructor_ParallelExecutionRelatedPropertiesGiven_ShouldLoadProper public void constructor_AsyncExecutionRelatedPropertiesGiven_ShouldLoadProperly() { // Arrange Properties props = new Properties(); - props.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); - props.setProperty(ConsensusCommitConfig.ASYNC_ROLLBACK_ENABLED, "false"); + props.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "true"); + props.setProperty(ConsensusCommitConfig.ASYNC_ROLLBACK_ENABLED, "true"); // Act ConsensusCommitConfig config = new ConsensusCommitConfig(new DatabaseConfig(props)); // Assert - assertThat(config.isAsyncCommitEnabled()).isFalse(); - assertThat(config.isAsyncRollbackEnabled()).isFalse(); + assertThat(config.isAsyncCommitEnabled()).isTrue(); + assertThat(config.isAsyncRollbackEnabled()).isTrue(); } @Test diff --git a/docs/configurations-for-consensus-commit.md b/docs/configurations-for-consensus-commit.md index d4b52240bf..6e6434c5d8 100644 --- a/docs/configurations-for-consensus-commit.md +++ b/docs/configurations-for-consensus-commit.md @@ -30,5 +30,5 @@ The Performance related configurations for Consensus Commit are as follows: | scalar.db.consensus_commit.parallel_validation.enabled | Whether or not the validation phase (in `EXTRA_READ`) is executed in parallel. | The value of `scalar.db.consensus_commit.parallel_commit.enabled` | | scalar.db.consensus_commit.parallel_commit.enabled | Whether or not the commit phase is executed in parallel. | true | | scalar.db.consensus_commit.parallel_rollback.enabled | Whether or not the rollback phase is executed in parallel. | The value of `scalar.db.consensus_commit.parallel_commit.enabled` | -| scalar.db.consensus_commit.async_commit.enabled | Whether or not the commit phase is executed asynchronously. | true | +| scalar.db.consensus_commit.async_commit.enabled | Whether or not the commit phase is executed asynchronously. | false | | scalar.db.consensus_commit.async_rollback.enabled | Whether or not the rollback phase is executed asynchronously. | The value of `scalar.db.consensus_commit.async_commit.enabled` | diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminIntegrationTestBase.java index 31079c1a43..45b441c48b 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminIntegrationTestBase.java @@ -45,10 +45,6 @@ protected final Properties getProperties(String testName) { ConsensusCommitConfig.COORDINATOR_NAMESPACE, Coordinator.NAMESPACE); properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); } return properties; } diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminRepairTableIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminRepairTableIntegrationTestBase.java index 3fd0367ea7..cec7c40f83 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminRepairTableIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminRepairTableIntegrationTestBase.java @@ -20,10 +20,6 @@ protected final Properties getProperties(String testName) { ConsensusCommitConfig.COORDINATOR_NAMESPACE, Coordinator.NAMESPACE); properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); } return properties; } diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitIntegrationTestBase.java index 951d508262..c1dc43046d 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitIntegrationTestBase.java @@ -67,10 +67,6 @@ protected final Properties getProperties(String testName) { ConsensusCommitConfig.COORDINATOR_NAMESPACE, Coordinator.NAMESPACE); properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); } return properties; } diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java index 41976b0c64..11e15f4fc3 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java @@ -93,10 +93,6 @@ public void beforeAll() throws Exception { properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + TEST_NAME); - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable it - // for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); - StorageFactory factory = StorageFactory.create(properties); admin = factory.getStorageAdmin(); databaseConfig = new DatabaseConfig(properties); diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitIntegrationTestBase.java index 579047ac6a..a5166b7a44 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitIntegrationTestBase.java @@ -83,10 +83,6 @@ private void modifyProperties(Properties properties, String testName) { properties.getProperty(ConsensusCommitConfig.COORDINATOR_NAMESPACE, Coordinator.NAMESPACE); properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); } protected abstract Properties getProps1(String testName); diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java index 6eec70e435..7ee7186188 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java @@ -102,10 +102,6 @@ private Properties modifyProperties(Properties properties) { properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + TEST_NAME); - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable it - // for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); - return properties; } diff --git a/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminIntegrationTestWithDistributedTransactionAdminService.java b/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminIntegrationTestWithDistributedTransactionAdminService.java index 5f4a9afafc..a08845e8c1 100644 --- a/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminIntegrationTestWithDistributedTransactionAdminService.java +++ b/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminIntegrationTestWithDistributedTransactionAdminService.java @@ -33,10 +33,6 @@ protected void initialize(String testName) throws IOException { properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); - server = new ScalarDbServer(properties); server.start(); diff --git a/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminRepairTableIntegrationTestWithDistributedTransactionAdminService.java b/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminRepairTableIntegrationTestWithDistributedTransactionAdminService.java index 8918cbcac8..6b5384a73a 100644 --- a/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminRepairTableIntegrationTestWithDistributedTransactionAdminService.java +++ b/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitAdminRepairTableIntegrationTestWithDistributedTransactionAdminService.java @@ -27,10 +27,6 @@ protected void initialize(String testName) throws IOException { properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); - server = new ScalarDbServer(properties); server.start(); } else { @@ -51,10 +47,6 @@ protected AdminTestUtils getAdminTestUtils(String testName) { properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); - return new ServerAdminTestUtils(properties); } diff --git a/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitIntegrationTestWithDistributedTransactionService.java b/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitIntegrationTestWithDistributedTransactionService.java index df2b7e24da..fecb307cac 100644 --- a/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitIntegrationTestWithDistributedTransactionService.java +++ b/server/src/integration-test/java/com/scalar/db/server/ConsensusCommitIntegrationTestWithDistributedTransactionService.java @@ -32,10 +32,6 @@ protected void initialize(String testName) throws IOException { properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); - server = new ScalarDbServer(properties); server.start(); diff --git a/server/src/integration-test/java/com/scalar/db/server/TwoPhaseConsensusCommitIntegrationTestWithTwoPhaseCommitTransactionService.java b/server/src/integration-test/java/com/scalar/db/server/TwoPhaseConsensusCommitIntegrationTestWithTwoPhaseCommitTransactionService.java index e50516c767..5b1ab35d07 100644 --- a/server/src/integration-test/java/com/scalar/db/server/TwoPhaseConsensusCommitIntegrationTestWithTwoPhaseCommitTransactionService.java +++ b/server/src/integration-test/java/com/scalar/db/server/TwoPhaseConsensusCommitIntegrationTestWithTwoPhaseCommitTransactionService.java @@ -49,10 +49,6 @@ private Properties modifyProperties(Properties properties, String testName) { properties.setProperty( ConsensusCommitConfig.COORDINATOR_NAMESPACE, coordinatorNamespace + "_" + testName); - // Async commit can cause unexpected lazy recoveries, which can fail the tests. So we disable - // it for now. - properties.setProperty(ConsensusCommitConfig.ASYNC_COMMIT_ENABLED, "false"); - return properties; }