belliottsmith commented on code in PR #10:
URL: https://github.com/apache/cassandra-accord/pull/10#discussion_r990368581
##########
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:
I really don’t agree. Performance is also critical and these are *not* where
the difficulty lies with distributed consensus.
I also outlined trivial solutions to these concerns?
A higher method doing the wrong thing is also just as possible with Future:
you can start some work and just return another Future, for instance. This is
identical to invoking the callback erroneously, and I don’t see why one is much
more likely than another?
If more work is being done after invoking the lower method, what stops you
doing the wrong thing with a Future?
I really couldn’t disagree more here. If you really think this class of
problem is a serious issue, we just implement code style requirements that you
null out a callback after passing it, and use the type system as suggested
above to ensure it must be invoked before returning. A little bit of coding
discipline with no cognitive burden, like all good engineering.
What additional class of problem are we unable to prevent here, with these
accordances?
This isn’t a small thing either. Like streams, Future are prone to very
easily disguising very costly behaviours, even ignoring their inherent costs.
Once something is a Future there is a tendency to chain, and this is costly and
also something we have to burn mental cycles addressing or considering.
So the question is: are these really weak concerns, that we seem able to
address quite easily, more important than ensuring the performance of the
system? And I am emphatically answering no.
I am not opposed to costs for safety, but I see little to no additional
safety here.
--
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]
