ifesdjeen commented on code in PR #163:
URL: https://github.com/apache/cassandra-accord/pull/163#discussion_r1938295513
##########
accord-core/src/main/java/accord/api/ProtocolModifiers.java:
##########
@@ -203,6 +204,15 @@ public static class Toggles
public static boolean requiresUniqueHlcs() { return
requiresUniqueHlcs; }
public static void setRequiresUniqueHlcs(boolean
newRequiresUniqueHlcs) { requiresUniqueHlcs = newRequiresUniqueHlcs; }
+ private static int markStaleIfCannotExecute =
TinyEnumSet.encode(Txn.Kind.Write);
+ public static boolean markStaleIfCannotExecute(Txn.Kind kind) { return
TinyEnumSet.contains(markStaleIfCannotExecute, kind); }
+ public static void setMarkStaleIfCannotExecute(Txn.Kind ... kinds)
Review Comment:
nit: space before `...`
##########
accord-core/src/main/java/accord/impl/progresslog/DefaultProgressLogs.java:
##########
@@ -54,7 +56,7 @@ static boolean pauseForTest(DefaultProgressLog progressLog)
return false;
if (!paused.containsKey(progressLog))
- paused.putIfAbsent(progressLog, () ->
progressLog.commandStore.execute(progressLog));
+ paused.putIfAbsent(progressLog, () ->
progressLog.commandStore.execute(empty(), progressLog));
Review Comment:
discussed offline: looks like we may want to add `begin` here, since when
runnable runs, we only create an async chain but do not start it.
##########
accord-core/src/main/java/accord/coordinate/CoordinateGloballyDurable.java:
##########
@@ -31,12 +31,12 @@
import accord.utils.async.AsyncResult;
import accord.utils.async.AsyncResults.SettableResult;
-// TODO (expected): this does not need to query every shard; can disseminate
globally any sub-range of the ring
-// (indeed, we could slice both the query and dissemination only so that they
always overlap)
+// TODO (expected): this does not need to query every shard OR more than one
replica per shard;
+// can disseminate globally any sub-range of the ring.
+// Infact, we could simply autonomously disseminate our latest information to
some subset of replicas
Review Comment:
nit: honest question: is `infact` commonly spelled as one word?
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -923,51 +913,45 @@ private static Command purge(Command command, @Nonnull
StoreParticipants newPart
case TRUNCATE:
Invariants.require(command.saveStatus().compareTo(TruncatedApply) < 0);
- Invariants.requireArgument(command.known().is(ApplyAtKnown));
+ Invariants.requireArgument(command.known().is(ApplyAtKnown) ||
(command.known().is(ExecuteAtKnown) && !command.txnId().is(Write)));
Review Comment:
nit: could you add brackets to signify operator precedence 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]
