belliottsmith commented on code in PR #233: URL: https://github.com/apache/cassandra-accord/pull/233#discussion_r2246274793
########## accord-core/src/main/java/accord/local/CommandSummaries.java: ########## @@ -60,185 +62,279 @@ enum SummaryStatus INVALIDATED; public static final SummaryStatus NONE = null; + + private static final SummaryStatus[] SUMMARY_STATUSES = values(); } - enum IsDep { IS_COORD_DEP, IS_NOT_COORD_DEP, NOT_ELIGIBLE, IS_STABLE_DEP, IS_NOT_STABLE_DEP } + enum IsDep + { + IS_COORD_DEP, IS_NOT_COORD_DEP, NOT_ELIGIBLE, IS_STABLE_DEP, IS_NOT_STABLE_DEP; + private static final IsDep[] IS_DEPS = values(); + } - class Summary + class Summary extends TxnId { - public final @Nonnull TxnId txnId; - public final @Nonnull Timestamp executeAt; - public final @Nonnull SummaryStatus status; - public final @Nonnull Unseekables<?> participants; + private static final int SUMMARY_STATUS_MASK = 0x7; + private static final int IS_DEP_SHIFT = 3; + final @Nonnull Timestamp executeAt; + final int encoded; + final Unseekables<?> participants; - public final IsDep dep; - public final TxnId findAsDep; + public Summary slice(Ranges ranges) + { + return new Summary(this, this.executeAt, encoded, participants.slice(ranges, Minimal)); + } @VisibleForTesting - public Summary(@Nonnull TxnId txnId, @Nonnull Timestamp executeAt, @Nonnull SummaryStatus status, @Nonnull Unseekables<?> participants, IsDep dep, TxnId findAsDep) + public Summary(@Nonnull TxnId txnId, @Nonnull Timestamp executeAt, @Nonnull SummaryStatus status, IsDep dep, Unseekables<?> participants) { - this.txnId = txnId; - this.executeAt = executeAt; - this.status = status; + super(txnId); this.participants = participants; - this.findAsDep = findAsDep; - this.dep = dep; + this.executeAt = executeAt.equals(txnId) ? this : executeAt; + this.encoded = status.ordinal() | (dep == null ? Integer.MIN_VALUE : (dep.ordinal() << IS_DEP_SHIFT)); + } + + private Summary(@Nonnull TxnId txnId, @Nonnull Timestamp executeAt, int encoded, Unseekables<?> participants) + { + super(txnId); + this.participants = participants; + this.executeAt = executeAt == txnId || executeAt.equals(txnId) ? this : executeAt; + this.encoded = encoded; + } + + public boolean is(IsDep isDep) + { + return (encoded >> IS_DEP_SHIFT) == isDep.ordinal(); } - public Summary slice(Ranges slice) + public IsDep isDep() { - return new Summary(txnId, executeAt, status, participants.slice(slice, Minimal), dep, findAsDep); + int ordinal = encoded >> IS_DEP_SHIFT; + return ordinal < 0 ? null : IsDep.IS_DEPS[ordinal]; + } + + public boolean is(SummaryStatus summaryStatus) + { + return (encoded & SUMMARY_STATUS_MASK) == summaryStatus.ordinal(); + } + + public SummaryStatus status() + { + int ordinal = encoded & SUMMARY_STATUS_MASK; + return SummaryStatus.SUMMARY_STATUSES[ordinal]; + } + + public TxnId plainTxnId() + { + return new TxnId(this); + } + + public Timestamp plainExecuteAt() + { + return executeAt == this ? new Timestamp(this) : executeAt; } @Override public String toString() { return "Summary{" + - "txnId=" + txnId + - ", executeAt=" + executeAt + - ", saveStatus=" + status + - ", participants=" + participants + - ", maybeDep=" + dep + - ", findAsDep=" + findAsDep + + "txnId=" + plainTxnId() + + ", executeAt=" + plainExecuteAt() + + ", saveStatus=" + status() + + ", isDep=" + isDep() + '}'; } + } - public static class Loader + class SummaryLoader + { + public interface Factory<L extends SummaryLoader, P> { - public interface Factory<L extends Loader> - { - L create(@Nullable TxnId primaryTxnId, Unseekables<?> searchKeysOrRanges, RedundantBefore redundantBefore, Kinds testKind, TxnId minTxnId, Timestamp maxTxnId, @Nullable TxnId findAsDep); - } + L create(P param, RedundantBefore redundantBefore, @Nullable MaxDecidedRX maxDecidedRX, TxnId primaryTxnId, Unseekables<?> searchKeysOrRanges, Kinds testKind, TxnId minTxnId, Timestamp maxTxnId, @Nullable TxnId findAsDep); + } - protected final Unseekables<?> searchKeysOrRanges; - protected final RedundantBefore redundantBefore; - // TODO (expected): separate out Kinds we need before/after primaryTxnId/executeAt - protected final Kinds testKind; - protected final TxnId minTxnId; - protected final Timestamp maxTxnId; - @Nullable protected final TxnId findAsDep; + protected final RedundantBefore redundantBefore; + protected final MaxDecidedRX maxDecidedRX; + protected final Unseekables<?> searchKeysOrRanges; + // TODO (expected): separate out Kinds we need before/after primaryTxnId/executeAt + protected final Kinds testKind; + protected final TxnId primaryTxnId, findAsDep, minTxnId, minDecidedId; + protected final Timestamp maxTxnId; +// protected final TxnId minDecidedId; + + // TODO (expected): provide executeAt to PreLoadContext so we can more aggressively filter what we load, esp. by Kind + public static SummaryLoader loader(RedundantBefore redundantBefore, MaxDecidedRX maxDecidedRX, PreLoadContext context) + { + return loader(redundantBefore, maxDecidedRX, context.primaryTxnId(), context.loadKeysFor(), context.keys()); + } - // TODO (expected): provide executeAt to PreLoadContext so we can more aggressively filter what we load, esp. by Kind - public static Loader loader(RedundantBefore redundantBefore, PreLoadContext context) - { - return loader(redundantBefore, context.primaryTxnId(), context.loadKeysFor(), context.keys()); - } + public static SummaryLoader loader(RedundantBefore redundantBefore, MaxDecidedRX maxDecidedRX, TxnId primaryTxnId, LoadKeysFor loadKeysFor, Unseekables<?> keysOrRanges) + { + return loader(null, redundantBefore, maxDecidedRX, primaryTxnId, loadKeysFor, keysOrRanges, SummaryLoader::new); + } - public static Loader loader(RedundantBefore redundantBefore, @Nullable TxnId primaryTxnId, LoadKeysFor loadKeysFor, Unseekables<?> keysOrRanges) - { - return loader(redundantBefore, primaryTxnId, loadKeysFor, keysOrRanges, Loader::new); - } + public static <L extends SummaryLoader, P> L loader(P param, RedundantBefore redundantBefore, MaxDecidedRX maxDecidedRX, PreLoadContext context, Factory<L, P> factory) + { + return loader(param, redundantBefore, maxDecidedRX, context.primaryTxnId(), context.loadKeysFor(), context.keys(), factory); + } - public static <L extends Loader> L loader(RedundantBefore redundantBefore, @Nullable TxnId primaryTxnId, LoadKeysFor loadKeysFor, Unseekables<?> keysOrRanges, Factory<L> factory) - { - TxnId minTxnId = redundantBefore.min(keysOrRanges, Bounds::gcBefore); - Timestamp maxTxnId = primaryTxnId == null || loadKeysFor == RECOVERY || !primaryTxnId.is(ExclusiveSyncPoint) ? Timestamp.MAX : primaryTxnId; - TxnId findAsDep = primaryTxnId != null && loadKeysFor == RECOVERY ? primaryTxnId : null; - Kinds kinds = primaryTxnId == null ? AnyGloballyVisible : primaryTxnId.witnesses().or(loadKeysFor == RECOVERY ? primaryTxnId.witnessedBy() : Nothing); - return factory.create(primaryTxnId, keysOrRanges, redundantBefore, kinds, minTxnId, maxTxnId, findAsDep); - } + public static <L extends SummaryLoader, P> L loader(P param, RedundantBefore redundantBefore, MaxDecidedRX maxDecidedRX, TxnId primaryTxnId, LoadKeysFor loadKeysFor, Unseekables<?> keysOrRanges, Factory<L, P> factory) + { + Invariants.require(primaryTxnId != null); + TxnId minTxnId = redundantBefore.min(keysOrRanges, Bounds::gcBefore); + Timestamp maxTxnId = loadKeysFor == RECOVERY || !primaryTxnId.is(ExclusiveSyncPoint) ? Timestamp.MAX : primaryTxnId; + TxnId findAsDep = loadKeysFor == RECOVERY ? primaryTxnId : null; + Kinds kinds = primaryTxnId.witnesses().or(loadKeysFor == RECOVERY ? primaryTxnId.witnessedBy() : Nothing); + if (!primaryTxnId.is(Txn.Kind.ExclusiveSyncPoint)) + maxDecidedRX = null; + return factory.create(param, redundantBefore, maxDecidedRX, primaryTxnId, keysOrRanges, kinds, minTxnId, maxTxnId, findAsDep); + } - public Loader(@Nullable TxnId primaryTxnId, Unseekables<?> searchKeysOrRanges, RedundantBefore redundantBefore, Kinds testKind, TxnId minTxnId, Timestamp maxTxnId, @Nullable TxnId findAsDep) - { - this.searchKeysOrRanges = searchKeysOrRanges; - this.redundantBefore = redundantBefore; - this.testKind = testKind; - this.minTxnId = minTxnId; - this.maxTxnId = maxTxnId; - this.findAsDep = findAsDep; - } + public SummaryLoader(Object ignore, RedundantBefore redundantBefore, MaxDecidedRX maxDecidedRX, TxnId primaryTxnId, Unseekables<?> searchKeysOrRanges, Kinds testKind, TxnId minTxnId, Timestamp maxTxnId, @Nullable TxnId findAsDep) + { + this.redundantBefore = redundantBefore; + this.maxDecidedRX = maxDecidedRX; + this.primaryTxnId = primaryTxnId; + this.searchKeysOrRanges = searchKeysOrRanges; + this.testKind = testKind; + this.minTxnId = minTxnId; + this.maxTxnId = maxTxnId; + this.findAsDep = findAsDep; + this.minDecidedId = minDecidedDependencyId(maxDecidedRX, searchKeysOrRanges, primaryTxnId); + } - public Summary ifRelevant(Command cmd) - { - return ifRelevant(cmd.txnId(), cmd.executeAtOrTxnId(), cmd.saveStatus(), cmd.participants(), cmd.partialDeps()); - } + public boolean isRelevant(CommandsForKey cfk) + { + if (cfk == null || cfk.size() == 0) + return false; - private boolean isEligibleDep(SummaryStatus status, TxnId findAsDep, TxnId txnId, Timestamp executeAt) + // NOTE: we CANNOT safely filter on first element, as we may have pruned dependencies we need to witness Review Comment: I mean we are filtering based on the last element in the CFK, and while it might seem like we could do equivalent filtering based on the first/min element and maxTxnId, we can't. -- 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-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org