dcapwell commented on code in PR #101:
URL: https://github.com/apache/cassandra-accord/pull/101#discussion_r1664546445


##########
accord-core/src/main/java/accord/config/LocalConfig.java:
##########
@@ -22,8 +22,22 @@
 
 public interface LocalConfig
 {
+    LocalConfig DEFAULT = new LocalConfig() {};
+
     default Duration getProgressLogScheduleDelay()
     {
         return Duration.ofSeconds(1);
     }
+
+    // How long before we start notifying waiters on an epoch of timeout,
+    default long epochFetchInitialTimeout()

Review Comment:
   what is the unit?  `Duration` is best as thats value + unit



##########
accord-core/src/main/java/accord/coordinate/CoordinatePreAccept.java:
##########
@@ -152,7 +152,12 @@ void onNewEpochTopologyMismatch(TopologyMismatch mismatch)
     void onPreAccepted(Topologies topologies)
     {
         Timestamp executeAt = foldl(oks, (ok, prev) -> 
mergeMax(ok.witnessedAt, prev), Timestamp.NONE);
-        node.withEpoch(executeAt.epoch(), () -> onPreAccepted(topologies, 
executeAt, oks));
+        node.withEpoch(executeAt.epoch(), (ignored, withEpochFailure) -> {

Review Comment:
   The issue with callbacks is that failures bubble up in random places, where 
as `AsyncChain` kinda forces `begin` to handle everything.  Right now if the 
epoch is known this call trace must handle the exceptions, and if not then we 
send to the `agent` (as it becomes a `AsyncChain` under the hood).  



##########
accord-core/src/main/java/accord/config/LocalConfig.java:
##########
@@ -22,8 +22,22 @@
 
 public interface LocalConfig
 {
+    LocalConfig DEFAULT = new LocalConfig() {};
+
     default Duration getProgressLogScheduleDelay()
     {
         return Duration.ofSeconds(1);
     }
+
+    // How long before we start notifying waiters on an epoch of timeout,
+    default long epochFetchInitialTimeout()
+    {
+        return 10_000;
+    }
+
+    // How often to check for timeout, and once an epoch has timed out, how 
often we timeout new waiters
+    default long epochFetchWatchdogIntervalMillis()

Review Comment:
   can you use `Duration`?  Forcing `mills` is something we try to avoid in C* 
config now



-- 
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