Mark txns durable when persistence completes (#590)

* Mark txns durable when persistence completes

* Turn off all commit flags in metadata before setting the commit decision

* Simplify logic by restricting txn state transitions

production/db/core/inc/txn_metadata.hpp

@@ -55,6 +55,7 @@ class txn_metadata_t
     static void set_active_txn_submitted(gaia_txn_id_t begin_ts, gaia_txn_id_t commit_ts);
     static void set_active_txn_terminated(gaia_txn_id_t begin_ts);
     static void update_txn_decision(gaia_txn_id_t commit_ts, bool is_committed);
+    static void set_txn_durable(gaia_txn_id_t commit_ts);
     static bool set_txn_gc_complete(gaia_txn_id_t commit_ts);
 
     static gaia_txn_id_t txn_begin();

production/db/core/src/db_server.cpp

@@ -2315,6 +2315,11 @@ bool server_t::txn_commit()
     {
         txn_name = rdb->begin_txn(s_txn_id);
         // Prepare log for transaction.
+        // This is effectively asynchronous with validation, because if it takes
+        // too long, then another thread may recursively validate this txn,
+        // before the committing thread has a chance to do so.
+        // NB: We only mark the txn as durable after validation, to simplify
+        // reasoning about txn state transitions.
         rdb->prepare_wal_for_write(s_log, txn_name);
     }
 
@@ -2326,9 +2331,20 @@ bool server_t::txn_commit()
     // Update the txn metadata with our commit decision.
     txn_metadata_t::update_txn_decision(commit_ts, is_committed);
 
-    // Append commit or rollback marker to the WAL.
+    // Persist the commit decision.
+    // Eventually, we will return a decision to the client asynchronously with
+    // the decision being persisted (because the decision can be reconstructed
+    // from the durable log itself, without the decision record).
     if (rdb)
     {
+        // Mark txn as durable in metadata so we can GC the txn log.
+        // The txn may be considered durable even if it hasn't yet been
+        // validated, since the decision can be reconstructed from the durable
+        // log, but we only mark it durable after validation to simplify the
+        // state transitions: we only allow
+        // TXN_VALIDATING -> TXN_DECIDED -> TXN_DURABLE.
+        txn_metadata_t::set_txn_durable(commit_ts);
+
         if (is_committed)
         {
             rdb->append_wal_commit_marker(txn_name);

production/db/core/src/txn_metadata.cpp

@@ -217,53 +217,56 @@ void txn_metadata_t::update_txn_decision(gaia_txn_id_t commit_ts, bool is_commit
     // The commit_ts metadata must be in state TXN_VALIDATING or TXN_DECIDED.
     // We allow the latter to enable idempotent concurrent validation.
     txn_metadata_t commit_ts_metadata(commit_ts);
+
     common::retail_assert(
         commit_ts_metadata.is_validating() || commit_ts_metadata.is_decided(),
         "commit_ts metadata must be in validating or decided state!");
 
-    uint64_t decided_status_flags
-        = is_committed ? c_txn_status_committed : c_txn_status_aborted;
+    uint64_t decided_status_flags{is_committed ? c_txn_status_committed : c_txn_status_aborted};
 
-    // We can just reuse the log fd and begin_ts from the existing metadata.
-    //
-    // REVIEW: This condition is probably rare enough that it's not worth optimizing
-    // and we can just let the CAS fail if another thread validated the txn before we did.
-    //
-    // We may have already been validated by another committing txn.
-    if (commit_ts_metadata.is_decided())
-    {
-        // If another txn validated before us, they should have reached the same decision.
-        common::retail_assert(
-            commit_ts_metadata.is_committed() == is_committed,
-            "Inconsistent txn decision detected!");
+    txn_metadata_t decided_commit_ts_metadata = commit_ts_metadata;
 
-        return;
-    }
+    // This masks out just the commit_ts flag bits.
+    constexpr uint64_t c_commit_flags_mask = ~(~c_txn_status_commit_ts << c_txn_status_flags_shift);
 
-    txn_metadata_t expected_metadata = commit_ts_metadata;
+    // Turn off all commit flag bits before turning on the bits for this decision.
+    decided_commit_ts_metadata.m_value &= c_commit_flags_mask;
 
-    // It's safe to just OR in the new flags because the preceding states don't set
-    // any bits not present in the flags.
-    commit_ts_metadata.m_value |= (decided_status_flags << c_txn_status_flags_shift);
+    // Now set the decision flags.
+    decided_commit_ts_metadata.m_value |= (decided_status_flags << c_txn_status_flags_shift);
+
+    bool has_set_metadata = compare_exchange_strong(commit_ts_metadata, decided_commit_ts_metadata);
 
-    bool has_set_metadata = compare_exchange_strong(
-        expected_metadata, commit_ts_metadata);
     if (!has_set_metadata)
     {
-        // NB: expected_metadata is an inout argument holding the previous value on failure!
+        // The only state transition allowed from TXN_VALIDATING is to TXN_DECIDED.
         common::retail_assert(
-            expected_metadata.is_decided(),
+            commit_ts_metadata.is_decided(),
             "commit_ts metadata in validating state can only transition to a decided state!");
 
         // If another txn validated before us, they should have reached the same decision.
         common::retail_assert(
-            expected_metadata.is_committed() == is_committed,
+            commit_ts_metadata.is_committed() == is_committed,
             "Inconsistent txn decision detected!");
-
-        return;
     }
 }
 
+void txn_metadata_t::set_txn_durable(gaia_txn_id_t commit_ts)
+{
+    txn_metadata_t commit_ts_metadata(commit_ts);
+
+    txn_metadata_t durable_commit_ts_metadata;
+    do
+    {
+        // NB: commit_ts_metadata is an inout argument holding the previous value
+        // on failure!
+        durable_commit_ts_metadata = commit_ts_metadata;
+
+        durable_commit_ts_metadata.m_value |= (c_txn_persistence_complete << c_txn_persistence_flags_shift);
+
+    } while (!compare_exchange_weak(commit_ts_metadata, durable_commit_ts_metadata));
+}
+
 bool txn_metadata_t::set_txn_gc_complete(gaia_txn_id_t commit_ts)
 {
     txn_metadata_t commit_ts_metadata(commit_ts);