belliottsmith commented on code in PR #7:
URL: https://github.com/apache/cassandra-accord/pull/7#discussion_r1011809337


##########
accord-core/src/main/java/accord/local/Command.java:
##########
@@ -399,158 +525,253 @@ private void onChangeInternal(PartialCommand command)
 
             case Committed:
             case ReadyToExecute:
-            case Executed:
+            case PreApplied:
             case Applied:
             case Invalidated:
-                if (isUnableToApply())
-                {
-                    updatePredecessor(command);
-                    if (isWaitingOnCommit())
-                    {
-                        removeWaitingOnCommit(command);
-                    }
-                }
-                else
-                {
-                    command.removeListener(this);
-                }
-                maybeExecute(false);
+                updatePredecessor(command, false);
+                maybeExecute(safeStore, progressShard(safeStore), false, true);
                 break;
         }
     }
 
-    @Override
-    public void onChange(Command command)
-    {
-        onChangeInternal(command);
-    }
-
-    protected void postApply()
+    protected void postApply(SafeCommandStore safeStore)
     {
         logger.trace("{} applied, setting status to Applied and notifying 
listeners", txnId());
-        status(Applied);
-        notifyListeners();
+        setStatus(Applied);
+        notifyListeners(safeStore);
     }
 
-    private static Function<CommandStore, Void> callPostApply(TxnId txnId)
+    private static Function<SafeCommandStore, Void> callPostApply(TxnId txnId)
     {
-        return commandStore -> {
-            commandStore.command(txnId).postApply();
+        return safeStore -> {
+            safeStore.command(txnId).postApply(safeStore);
             return null;
         };
     }
 
-    protected Future<Void> apply()
+    protected Future<Void> apply(SafeCommandStore safeStore)
     {
         // important: we can't include a reference to *this* in the lambda, 
since the C* implementation may evict
         // the command instance from memory between now and the write 
completing (and post apply being called)
-        return writes().apply(commandStore()).flatMap(unused ->
-            commandStore().process(this, callPostApply(txnId()))
+        CommandStore unsafeStore = safeStore.commandStore();
+        return writes().apply(safeStore).flatMap(unused ->
+            unsafeStore.submit(this, callPostApply(txnId()))
         );
     }
 
-    public Future<Data> read(Keys scope)
+    public Future<Data> read(SafeCommandStore safeStore)
     {
-        return txn().read(this);
+        return partialTxn().read(safeStore, this);
     }
 
-    private Future<Void> maybeExecute(boolean notifyListenersOnNoop)
+    // TODO: maybe split into maybeExecute and maybeApply?
+    private boolean maybeExecute(SafeCommandStore safeStore, ProgressShard 
shard, boolean alwaysNotifyListeners, boolean notifyWaitingOn)

Review Comment:
   Avoiding infinite loops. You will notice the only location we pass `false` 
for is the `NotifyWaitingOn` class, which may itself invoke `maybeExecute` 
(which may then invoke `NotifyWaitingOn`). Once we're in `NotifyWaitingOn`, we 
don't need to invoke it again for anything it is invoking, since it will do the 
necessary work already.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to