[GitHub] [cassandra-accord] bdeggleston commented on a diff in pull request #10: Metadata persistence

[GitBox <gi...@apache.org>]

bdeggleston commented on code in PR #10:
URL: https://github.com/apache/cassandra-accord/pull/10#discussion_r988462846


##########
accord-core/src/main/java/accord/messages/Apply.java:
##########
@@ -54,15 +63,59 @@ public Apply(Node.Id to, Topologies topologies, TxnId txnId, Txn txn, Key homeKe
         this.result = result;
     }
 
+    @VisibleForImplementation
+    public Apply(Keys scope, long waitForEpoch, TxnId txnId, Txn txn, Key homeKey, Timestamp executeAt, Deps deps, Writes writes, Result result)
+    {
+        super(scope, waitForEpoch);
+        this.txnId = txnId;
+        this.txn = txn;
+        this.homeKey = homeKey;
+        this.executeAt = executeAt;
+        this.deps = deps;
+        this.writes = writes;
+        this.result = result;
+    }
+
+    static Future<Void> waitAndReduce(Future<Void> left, Future<Void> right)

Review Comment:
   
   > This kind approach really hamstrings Cassandra's ability to improve. 
   
   You mean a detailed and informed discussion of the pros and cons of different solutions? I feel like the project could use more of this ;)
   
   > Each Future with callback is an additional three CAS instructions and ~100 bytes of memory. This is a hot path, where we already removed a chain of about four Future when internally processing requests. This is the removal of perhaps a dozen CAS instructions and several hundred bytes allocated. 
   
   I understand this is a hot path, and prior to pushing back on this, I implemented a callback class that would meet the needs of apply, and the cas/bytes numbers above are way overstated. The way we distribute work to command stores negates a lot of the potential simplicity of callbacks. Specifically, we may have to talk to multiple command stores, and we don’t know how many until we’ve dispatched our tasks.
   
   In the case where we interact with a single command store, futures actually use slightly less memory than callbacks. In the worst case, a future will use 24 more bytes and one more cas operation than a callback. 
   
   Here’s the callback class if you’d like to take a look, it’s possible I’ve overlooked something: https://gist.github.pie.apple.com/beggleston/56e540bdb6408dd7f22ec88d095c42e2
   
   In the more complex use of futures, like this, callback state starts to look a lot like future state.



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org