You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "belliottsmith (via GitHub)" <gi...@apache.org> on 2025/01/22 13:13:35 UTC
[PR] Agree a distributed uniqueHlc to use for Apply [cassandra-accord]
belliottsmith opened a new pull request, #158:
URL: https://github.com/apache/cassandra-accord/pull/158
- detect and save whether RX can be used as a GC bound for HLCs
- have RX durably record if they witnessed any superseding epoch and use this for HLC GC
- rework GC to retain applyAt for writes
Also improve:
- PreCommitted overwrites fast path vote, then loses tie-break with NotAccept, so we don't recover
- track executeAtLeast and uniqueHlc separately from WaitingOn in CommandChange
- Always propose executeAt.hlc() >= txnId.hlc() Also fix:
- Dangling recovery when Commit rejected
- Failed epoch fetch handling in DefaultProgressLog
- CommandsForKey must track Accept executeAt for computing earlierWait collection on recovery
--
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
Re: [PR] Agree a distributed uniqueHlc to use for Apply [cassandra-accord]
Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith merged PR #158:
URL: https://github.com/apache/cassandra-accord/pull/158
--
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
Re: [PR] Agree a distributed uniqueHlc to use for Apply [cassandra-accord]
Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #158:
URL: https://github.com/apache/cassandra-accord/pull/158#discussion_r1925871369
##########
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:
I've opted to update the formatting. I'm not a huge fan of mixing return types, particularly at this level of one of each kind. I would be OK with switching to three return statements if you prefer though.
--
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
Re: [PR] Agree a distributed uniqueHlc to use for Apply [cassandra-accord]
Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #158:
URL: https://github.com/apache/cassandra-accord/pull/158#discussion_r1925868905
##########
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:
I opted not to because it's test-only, but can if you have a strong preference
--
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
Re: [PR] Agree a distributed uniqueHlc to use for Apply [cassandra-accord]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
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: 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
Re: [PR] Agree a distributed uniqueHlc to use for Apply [cassandra-accord]
Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #158:
URL: https://github.com/apache/cassandra-accord/pull/158#discussion_r1925869420
##########
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:
yes, good catch!
--
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
Re: [PR] Agree a distributed uniqueHlc to use for Apply [cassandra-accord]
Posted by "belliottsmith (via GitHub)" <gi...@apache.org>.
belliottsmith commented on code in PR #158:
URL: https://github.com/apache/cassandra-accord/pull/158#discussion_r1925534707
##########
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:
We aren’t, I’ve stopped WaitingOn from persisting either executesAtLeast or UniqueHLC. This also means an update to one of these components only requires updating that component. The WaitingOn provider accepts them as inputs 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: 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