Fix hang in maybe_changed_after

Author: MichaReiser

src/function/fetch.rs

@@ -200,35 +200,6 @@ where
                     // still valid for the current revision.
                     return unsafe { Some(self.extend_memo_lifetime(old_memo)) };
                 }
-
-                // If this is a provisional memo from the same revision, await all its cycle heads because
-                // we need to ensure that only one thread is iterating on a cycle at a given time.
-                // For example, if we have a nested cycle like so:
-                // ```
-                // a -> b -> c -> b
-                //        -> a
-                //
-                // d -> b
-                // ```
-                // thread 1 calls `a` and `a` completes the inner cycle `b -> c` but hasn't finished the outer cycle `a` yet.
-                // thread 2 now calls `b`. We don't want that thread 2 iterates `b` while thread 1 is iterating `a` at the same time
-                // because it can result in thread b overriding provisional memos that thread a has accessed already and still relies upon.
-                //
-                // By waiting, we ensure that thread 1 completes a (based on a provisional value for `b`) and `b`
-                // becomes the new outer cycle, which thread 2 drives to completion.
-                if old_memo.may_be_provisional()
-                    && old_memo.verified_at.load() == zalsa.current_revision()
-                {
-                    // Try to claim all cycle heads of the provisional memo. If we can't because
-                    // some head is running on another thread, drop our claim guard to give that thread
-                    // a chance to take ownership of this query and complete it as part of its fixpoint iteration.
-                    // We will then block on the cycle head and retry once all cycle heads completed.
-                    // if !old_memo.try_claim_heads(zalsa, zalsa_local) {
-                    //     drop(claim_guard);
-                    //     old_memo.block_on_heads(zalsa, zalsa_local);
-                    //     return None;
-                    // }
-                }
             }
         }
 

src/function/maybe_changed_after.rs

@@ -84,9 +84,12 @@ where
         loop {
             let database_key_index = self.database_key_index(id);
 
-            crate::tracing::debug!(
-                "{database_key_index:?}: maybe_changed_after(revision = {revision:?})"
-            );
+            let _span = crate::tracing::info_span!(
+                "maybe_changed_after",
+                ?revision,
+                query=?database_key_index
+            )
+            .entered();
 
             // Check if we have a verified version: this is the hot path.
             let memo_guard = self.get_memo_from_table_for(zalsa, id, memo_ingredient_index);
@@ -141,7 +144,7 @@ where
     ) -> Option<VerifyResult> {
         let database_key_index = self.database_key_index(key_index);
 
-        let _claim_guard = match self.sync_table.try_claim(zalsa, key_index, true) {
+        let _claim_guard = match self.sync_table.try_claim(zalsa, key_index, false) {
             ClaimResult::Claimed(guard) => guard,
             ClaimResult::Running(blocked_on) => {
                 blocked_on.block_on(zalsa);
@@ -175,10 +178,8 @@ where
 
             // If `validate_maybe_provisional` returns `true`, but only because all cycle heads are from the same iteration,
             // carry over the cycle heads so that the caller verifies them.
-            if old_memo.may_be_provisional() {
-                for head in old_memo.cycle_heads() {
-                    cycle_heads.insert_head(head.database_key_index);
-                }
+            for head in old_memo.cycle_heads() {
+                cycle_heads.insert_head(head.database_key_index);
             }
 
             return Some(if old_memo.revisions.changed_at > revision {
@@ -467,7 +468,7 @@ where
                 match cycle_head {
                     TryClaimHeadsResult::Cycle {
                         head_iteration_count,
-                        current_iteration_count,
+                        memo_iteration_count: current_iteration_count,
                         verified_at: head_verified_at,
                     } => {
                         if head_verified_at != verified_at {
@@ -487,7 +488,7 @@ where
             true
         }
 
-        crate::tracing::trace!(
+        crate::tracing::info!(
             "{database_key_index:?}: validate_same_iteration(memo = {memo:#?})",
             memo = memo.tracing_debug()
         );
@@ -506,6 +507,15 @@ where
             return false;
         }
 
+        // Always return `false` if this is a cycle initial memo (or the last provisional memo in an iteration)
+        // as this value has obviously not finished computing yet.
+        if cycle_heads
+            .iter()
+            .all(|head| head.database_key_index == database_key_index)
+        {
+            return false;
+        }
+
         validate_same_iteration_cold(zalsa, zalsa_local, cycle_heads, verified_at)
     }
 

src/function/memo.rs

@@ -197,7 +197,7 @@ impl<'db, C: Configuration> Memo<'db, C> {
             for claim_result in cycle_heads {
                 match claim_result {
                     TryClaimHeadsResult::Cycle {
-                        current_iteration_count,
+                        memo_iteration_count: current_iteration_count,
                         head_iteration_count,
                         ..
                     } => {
@@ -449,7 +449,7 @@ pub(super) enum TryClaimHeadsResult<'me> {
     /// Claiming the cycle head results in a cycle.
     Cycle {
         head_iteration_count: IterationCount,
-        current_iteration_count: IterationCount,
+        memo_iteration_count: IterationCount,
         verified_at: Revision,
     },
 
@@ -506,7 +506,7 @@ impl<'me> Iterator for TryClaimCycleHeadsIter<'me> {
             );
             return Some(TryClaimHeadsResult::Cycle {
                 head_iteration_count,
-                current_iteration_count,
+                memo_iteration_count: current_iteration_count,
                 verified_at: self.zalsa.current_revision(),
             });
         }
@@ -540,7 +540,7 @@ impl<'me> Iterator for TryClaimCycleHeadsIter<'me> {
                 };
 
                 Some(TryClaimHeadsResult::Cycle {
-                    current_iteration_count,
+                    memo_iteration_count: current_iteration_count,
                     head_iteration_count,
                     verified_at,
                 })

src/function/sync.rs

@@ -62,6 +62,7 @@ impl SyncTable {
                     SyncOwnerId::Transferred => {
                         let current_id = thread::current().id();
                         let database_key_index = DatabaseKeyIndex::new(self.ingredient, key_index);
+
                         return match zalsa
                             .runtime()
                             .claim_transferred(database_key_index, allow_reentry)

src/tracing.rs

@@ -25,6 +25,12 @@ macro_rules! debug_span {
     };
 }
 
+macro_rules! info_span {
+    ($($x:tt)*) => {
+        crate::tracing::span!(INFO, $($x)*)
+    };
+}
+
 macro_rules! event {
     ($level:ident, $($x:tt)*) => {{
         let event = {
@@ -51,4 +57,4 @@ macro_rules! span {
     }};
 }
 
-pub(crate) use {debug, debug_span, event, info, span, trace};
+pub(crate) use {debug, debug_span, event, info, info_span, span, trace};