Revert async commit default value (#797)

scalar-labs · Jan 16, 2023 · 98c8dda · 98c8dda
1 parent b9b1830
commit 98c8dda
Show file tree

Hide file tree

Showing 13 changed files with 8 additions and 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
@@ -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
@@ -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
@@ -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/...va/com/scalar/db/transaction/consensuscommit/ConsensusCommitAdminIntegrationTestBase.java b/...va/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/...ar/db/transaction/consensuscommit/ConsensusCommitAdminRepairTableIntegrationTestBase.java b/...ar/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/...in/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitIntegrationTestBase.java b/...in/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/...com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java b/...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/...com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitIntegrationTestBase.java b/...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/...ar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java b/...ar/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/.../db/server/ConsensusCommitAdminIntegrationTestWithDistributedTransactionAdminService.java b/.../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/...ConsensusCommitAdminRepairTableIntegrationTestWithDistributedTransactionAdminService.java b/...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/...com/scalar/db/server/ConsensusCommitIntegrationTestWithDistributedTransactionService.java b/...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/...db/server/TwoPhaseConsensusCommitIntegrationTestWithTwoPhaseCommitTransactionService.java b/...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;
   }