ifesdjeen commented on code in PR #158:
URL: https://github.com/apache/cassandra-accord/pull/158#discussion_r1925329419
##########
accord-core/src/main/java/accord/impl/AbstractFetchCoordinator.java:
##########
@@ -323,7 +323,7 @@ public static class FetchResponse extends ReadOk
public FetchResponse(@Nullable Ranges unavailable, @Nullable Data
data, @Nonnull Timestamp safeToReadAfter)
{
- super(unavailable, data);
+ super(unavailable, data, 0);
Review Comment:
Nit: should we introduce a NO_HLC magic constant? Just seen 0 on 2 occasions.
##########
accord-core/src/main/java/accord/impl/progresslog/DefaultProgressLog.java:
##########
@@ -457,23 +464,37 @@ private void addToRunBuffer(TxnState add)
runBuffer[runBufferCount++] = add;
}
+ private void rerunWithPendingEpoch()
+ {
+ long minEpoch = Long.MAX_VALUE;
+ for (int i = 0 ; i < awaitingEpochBufferCount ; ++i)
+ minEpoch = Math.min(awaitingEpochBuffer[i].run.txnId.epoch(),
minEpoch);
+ Invariants.checkArgument(minEpoch != Long.MAX_VALUE);
+ isAwaitingEpoch = true;
Review Comment:
nit: isAwaitingEpoch seems to be unused outside this method; was there a
check you intended to add?
##########
accord-core/src/test/java/accord/impl/basic/InMemoryJournal.java:
##########
@@ -497,6 +501,9 @@ private Diff(int flags, CommandUpdate update)
case EXECUTES_AT_LEAST:
changes.put(EXECUTES_AT_LEAST,
after.executesAtLeast());
break;
+ case MIN_UNIQUE_HLC:
Review Comment:
If we already store HLC in waitingOn, do we need to also save it separately?
##########
accord-core/src/main/java/accord/impl/InMemoryCommandStore.java:
##########
Review Comment:
nit: checkstyle complains about this file's imports
##########
accord-core/src/main/java/accord/impl/CommandChange.java:
##########
@@ -503,41 +523,48 @@ public static int getFlags(Command before, Command after)
flags = collectFlags(before, after, Command::participants, true,
PARTICIPANTS, flags);
flags = collectFlags(before, after, Command::partialTxn, false,
PARTIAL_TXN, flags);
flags = collectFlags(before, after, Command::partialDeps, false,
PARTIAL_DEPS, flags);
-
- // TODO: waitingOn vs WaitingOnWithExecutedAt?
- flags = collectFlags(before, after, CommandChange::getWaitingOn, true,
WAITING_ON, flags);
+ flags = collectFlags(before, after, Command::waitingOn,
WaitingOn::equalBitSets, true, WAITING_ON, flags);
flags = collectFlags(before, after, Command::writes, false, WRITES,
flags);
flags = collectFlags(before, after, Command::result, false, RESULT,
flags);
+ // make sure we have enough information to decide whether to expunge
timestamps (for unique ApplyAt HLC guarantees)
+ if (isChanged(EXECUTE_AT, flags) &&
after.saveStatus().known.is(ApplyAtKnown))
+ {
+ flags = setChanged(PARTICIPANTS, flags);
+ flags = setChanged(SAVE_STATUS, flags);
+ }
return flags;
}
private static <OBJ, VAL> int collectFlags(OBJ lo, OBJ ro, Function<OBJ,
VAL> convert, boolean allowClassMismatch, Field field, int flags)
+ {
+ return collectFlags(lo, ro, convert, Object::equals,
allowClassMismatch, field, flags);
+ }
+
+ private static <OBJ, VAL> int collectFlags(OBJ lo, OBJ ro, Function<OBJ,
VAL> convert, BiPredicate<VAL, VAL> equals, boolean allowClassMismatch, Field
field, int flags)
{
VAL l = null;
VAL r = null;
if (lo != null) l = convert.apply(lo);
if (ro != null) r = convert.apply(ro);
- if (l == r)
- return flags; // no change
-
- if (r == null)
- flags = setFieldIsNull(field, flags);
-
- if (l == null || r == null)
- return setFieldChanged(field, flags);
-
- assert allowClassMismatch || l.getClass() == r.getClass() :
String.format("%s != %s", l.getClass(), r.getClass());
-
- if (l.equals(r))
- return flags; // no change
+ if (l == r) return flags; // no change
+ if (r == null) return setFieldIsNullAndChanged(field, flags);
+ if (l == null) return setChanged(field, flags);
+ Invariants.checkState(allowClassMismatch || l.getClass() ==
r.getClass(), "%s != %s", l.getClass(), r.getClass());
+ if (equals.test(l, r)) return flags; // no change
+ return setChanged(field, flags);
+ }
- return setFieldChanged(field, flags);
+ private static <OBJ> int collectFlags(OBJ lo, OBJ ro, ToLongFunction<OBJ>
convert, Field field, int flags)
+ {
+ long l = 0, r = 0;
+ if (lo != null) l = convert.applyAsLong(lo);
+ if (ro != null) r = convert.applyAsLong(ro);
+ return l == r ? flags : r == 0 ? setFieldIsNullAndChanged(field,
flags) : setChanged(field, flags);
Review Comment:
nit: would you mind to replace this line with
```
if (l == r) return flags;
return r == 0 ? setFieldIsNullAndChanged(field, flags) :
setChanged(field, flags);
```
just to break up double-ternary
##########
accord-core/src/main/java/accord/impl/InMemoryCommandStore.java:
##########
@@ -763,7 +766,12 @@ CommandSummaries commandsForRanges()
summaries.put(summary.txnId, summary);
}
- return commandsForRanges = () -> summaries;
+ final Kinds kinds = new Kinds(Read, ExclusiveSyncPoint, SyncPoint);
Review Comment:
nit: should we turn this into a static constant?
--
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]
