diff --git a/modules/accord b/modules/accord index fff32de2e9..84ac4db032 160000 --- a/modules/accord +++ b/modules/accord @@ -1 +1 @@ -Subproject commit fff32de2e915772fbc70d16b1c32346313877838 +Subproject commit 84ac4db03251c3b842c98a29868e072792662f51 diff --git a/src/java/org/apache/cassandra/concurrent/CassandraThread.java b/src/java/org/apache/cassandra/concurrent/CassandraThread.java index d66adc2442..b013a5f0ad 100644 --- a/src/java/org/apache/cassandra/concurrent/CassandraThread.java +++ b/src/java/org/apache/cassandra/concurrent/CassandraThread.java @@ -31,6 +31,7 @@ public class CassandraThread extends FastThreadLocalThread implements AccordExec private ExecutorLocals executorLocals; private AccordExecutor accordActiveExecutor; private AccordExecutor accordLockedExecutor; + private int accordLockedExecutorDepth; private volatile AccordExecutor.Task accordActiveTask; private static final AtomicReferenceFieldUpdater accordActiveTaskUpdater = AtomicReferenceFieldUpdater.newUpdater(CassandraThread.class, AccordExecutor.Task.class, "accordActiveTask"); @@ -113,18 +114,19 @@ public class CassandraThread extends FastThreadLocalThread implements AccordExec } @Override - public final boolean trySetAccordLockedExecutor(AccordExecutor newLockedExecutor) + public final boolean tryEnterAccordLockedExecutor(AccordExecutor newLockedExecutor) { - if (accordLockedExecutor != null) - return false; - accordLockedExecutor = newLockedExecutor; + if (accordLockedExecutor == null) accordLockedExecutor = newLockedExecutor; + else if (accordLockedExecutor != newLockedExecutor) return false; + ++accordLockedExecutorDepth; return true; } @Override - public final void clearAccordLockedExecutor() + public final void exitAccordLockedExecutor() { - accordLockedExecutor = null; + if (--accordLockedExecutorDepth == 0) + accordLockedExecutor = null; } public final AccordExecutor.Task accordActiveTask() diff --git a/src/java/org/apache/cassandra/config/AccordConfig.java b/src/java/org/apache/cassandra/config/AccordConfig.java index fe226fe92f..9813f3ffa0 100644 --- a/src/java/org/apache/cassandra/config/AccordConfig.java +++ b/src/java/org/apache/cassandra/config/AccordConfig.java @@ -156,31 +156,12 @@ public class AccordConfig */ PHASE_ONLY, - /** - * Always pick the task by priority. - */ - PRIORITY_BUDGET, - - /** - * Pick by phase first, so the highest phase with budget ALWAYS runs. - * Within a phase, pick by priority. - */ - PHASE_BUDGET, - /** * Pick by phase first, selecting the phase that has processed the least work recently relative to arrivals. * Within a phase, pick by priority. */ PHASE_FAIR, - /** - * Pick by phase first, selecting the phase that has processed the least work recently relative to arrivals. - * However, each phase has a budget that is consumed on dispatch, and we pick only from those phases with budget. - * If there is no phase with budget, the budget resets. - * Within a phase, pick by priority. - */ - PHASE_BUDGET_FAIR, - /** * While phases are within a threshold of imbalance, pick tasks by priority. * Once the threshold is crossed, over-processed phases have a small penalty applied @@ -190,8 +171,6 @@ public class AccordConfig * Within a phase, pick by priority. */ BLENDED_PRIORITY_PHASE_FAIR, - - BLENDED_PRIORITY_PHASE_BUDGET_FAIR, } public QueueShardModel queue_shard_model = THREAD_POOL_PER_SHARD; @@ -212,8 +191,29 @@ public class AccordConfig public Integer queue_flow_imbalance_onset = null; public Integer queue_flow_imbalance_width_shift = null; + public String queue_active_limits; - public String queue_budgets; + + public Boolean queue_nonsync_enabled; + + /** + * Size at which we will begin processing a task that is ASYNC, INCR OR INCR_ATOMIC. + * Note that a size of zero will effectively give implicit priority to INCR_ATOMIC tasks, as they may immediately + * take a FIFO queue slot (which is processed preferentially). + */ + public Integer queue_nonsync_min_batch_size; + + /** + * If there are more than min_batch_size keys ready for an ASYNC, INCR or INCR_ATOMIC task, + * process up to this many keys at once. + */ + public Integer queue_nonsync_max_batch_size; + + /** + * An ASYNC, INCR or INCR_ATOMIC task that is ready to run but waiting for batch_size work will proceed + * once this number of tasks are blocked behind it, regardless of batch_size. + */ + public Integer queue_nonsync_blocked_limit; /** * If set, the signal loop does not match park/unpark pairs, but instead consumers perform timed-park spin waits @@ -240,9 +240,6 @@ public class AccordConfig */ public volatile OptionaldPositiveInt command_store_shard_count = OptionaldPositiveInt.UNDEFINED; - public volatile OptionaldPositiveInt max_queued_loads = OptionaldPositiveInt.UNDEFINED; - public volatile OptionaldPositiveInt max_queued_range_loads = OptionaldPositiveInt.UNDEFINED; - public volatile OptionaldPositiveInt progress_log_concurrency = OptionaldPositiveInt.UNDEFINED; public DurationSpec.IntMillisecondsBound progress_log_query_fallback_timeout = new DurationSpec.IntMillisecondsBound("1m"); diff --git a/src/java/org/apache/cassandra/db/virtual/AccordDebugKeyspace.java b/src/java/org/apache/cassandra/db/virtual/AccordDebugKeyspace.java index 86238688f5..3a8c6e32bd 100644 --- a/src/java/org/apache/cassandra/db/virtual/AccordDebugKeyspace.java +++ b/src/java/org/apache/cassandra/db/virtual/AccordDebugKeyspace.java @@ -1650,7 +1650,7 @@ public class AccordDebugKeyspace extends VirtualKeyspace { try (AccordCommandStore.ExclusiveCaches caches = commandStore.lockCaches()) { - AccordCacheEntry entry = caches.commands().getUnsafe(txnId); + AccordCacheEntry entry = caches.commands().getUnsafe(txnId); return entry == null ? null : entry.getExclusive(); } } @@ -1895,7 +1895,7 @@ public class AccordDebugKeyspace extends VirtualKeyspace AccordService.getBlocking(accord.node() .commandStores() .forId(commandStoreId) - .chain(ExecutionContext.contextFor(txnId, TXN_OPS), apply) + .chain(ExecutionContext.unsequenced(txnId, TXN_OPS), apply) .flatMap(i -> i)); } diff --git a/src/java/org/apache/cassandra/service/accord/AccordCache.java b/src/java/org/apache/cassandra/service/accord/AccordCache.java index 0d679b2f51..a75e6a6e27 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordCache.java +++ b/src/java/org/apache/cassandra/service/accord/AccordCache.java @@ -44,6 +44,7 @@ import org.slf4j.LoggerFactory; import accord.api.RoutingKey; import accord.local.Command; +import accord.local.SafeState; import accord.local.cfk.CommandsForKey; import accord.local.cfk.Serialize; import accord.primitives.Routable; @@ -66,6 +67,7 @@ import org.apache.cassandra.metrics.LogLinearHistogram; import org.apache.cassandra.metrics.ShardedHitRate; import org.apache.cassandra.service.accord.AccordCache.Adapter.Shrink; import org.apache.cassandra.service.accord.AccordCacheEntry.LoadExecutor; +import org.apache.cassandra.service.accord.AccordCacheEntry.Loading; import org.apache.cassandra.service.accord.AccordCacheEntry.Status; import org.apache.cassandra.service.accord.AccordSafeCommandsForKey.CommandsForKeyCacheEntry; import org.apache.cassandra.service.accord.events.CacheEvents; @@ -79,6 +81,8 @@ import org.apache.cassandra.utils.ObjectSizes; import static accord.utils.Invariants.illegalState; import static accord.utils.Invariants.require; +import static org.apache.cassandra.service.accord.AccordCacheEntry.AGE_MASK; +import static org.apache.cassandra.service.accord.AccordCacheEntry.GENERATION_MASK; import static org.apache.cassandra.service.accord.AccordCacheEntry.Status.EVICTED; import static org.apache.cassandra.service.accord.AccordCacheEntry.Status.LOADED; import static org.apache.cassandra.service.accord.AccordCacheEntry.Status.MODIFIED; @@ -106,7 +110,7 @@ public class AccordCache implements CacheSize VALIDATE_LOAD_ON_EVICT = value; } - public interface Adapter + public interface Adapter & AccordSafeState> { enum Shrink { EVICT, DONE, PERFORM_WITHOUT_LOCK } @@ -122,10 +126,10 @@ public class AccordCache implements CacheSize long estimateHeapSize(V value); long estimateShrunkHeapSize(Object shrunk); boolean validate(AccordCommandStore commandStore, K key, V value); - S safeRef(AccordCacheEntry node); + S safeRef(AccordCacheEntry node); default Comparator keyComparator() { return null; } - default AccordCacheEntry newEntry(K key, AccordCache.Type.Instance owner) + default AccordCacheEntry newEntry(K key, AccordCache.Type.Instance owner) { return AccordCacheEntry.createReadyToLoad(key, owner); } @@ -151,8 +155,8 @@ public class AccordCache implements CacheSize private final List> types = new CopyOnWriteArrayList<>(); final AccordCacheEntry.SaveExecutor saveExecutor; - private final IntrusiveLinkedList> evictQueue = new IntrusiveLinkedList<>(); - private final IntrusiveLinkedList> noEvictQueue = new IntrusiveLinkedList<>(); + private final IntrusiveLinkedList> evictQueue = new IntrusiveLinkedList<>(); + private final IntrusiveLinkedList> noEvictQueue = new IntrusiveLinkedList<>(); private long unreferencedBytes; private int unreferenced; @@ -192,16 +196,16 @@ public class AccordCache implements CacheSize */ void processNoEvictQueue() { - noEvictGeneration = (noEvictGeneration + 1) & 0xffff; + noEvictGeneration = (noEvictGeneration + 1) & GENERATION_MASK; if (noEvictQueue.isEmpty()) return; - Iterator> iter = noEvictQueue.iterator(); + Iterator> iter = noEvictQueue.iterator(); int skipCount = 3; while (skipCount > 0 && iter.hasNext()) { - AccordCacheEntry entry = iter.next(); - int age = (noEvictGeneration - entry.noEvictGeneration()) & 0xffff; + AccordCacheEntry entry = iter.next(); + int age = (noEvictGeneration - entry.noEvictGeneration()) & GENERATION_MASK; if (age >= entry.noEvictMaxAge()) { evictNoEvict.warn(entry, age, entry.noEvictMaxAge()); @@ -226,14 +230,14 @@ public class AccordCache implements CacheSize while (bytesCached > maxSizeInBytes && !evictQueue.isEmpty()) { - AccordCacheEntry node = evictQueue.peek(); + AccordCacheEntry node = evictQueue.peek(); shrinkOrEvict(lock, node); } tryShrinkOrEvict = false; } @VisibleForTesting - private void shrinkOrEvict(Lock lock, AccordCacheEntry node) + private void shrinkOrEvict(Lock lock, AccordCacheEntry node) { require(node.references() == 0); @@ -244,7 +248,7 @@ public class AccordCache implements CacheSize } else { - IntrusiveLinkedList> queue; + IntrusiveLinkedList> queue; queue = node.isNoEvict() ? noEvictQueue : evictQueue; node.unlink(); if (shrink == Shrink.DONE) @@ -272,7 +276,7 @@ public class AccordCache implements CacheSize } @VisibleForTesting - public void tryEvict(AccordCacheEntry node) + public void tryEvict(AccordCacheEntry node) { require(node.references() == 0); @@ -290,7 +294,6 @@ public class AccordCache implements CacheSize case LOADING: node.loading().loading.cancel(); case WAITING_TO_LOAD: - Invariants.paranoid(node.loadingOrWaiting().waiters == null); case LOADED: node.unlink(); evict(node, true); @@ -305,7 +308,7 @@ public class AccordCache implements CacheSize } } - public void saveWhenReadyExclusive(AccordCacheEntry entry, Runnable onSuccess) + public void saveWhenReadyExclusive(AccordCacheEntry entry, Runnable onSuccess) { if (!entry.isSavingOrWaiting() && !entry.saveWhenReady()) onSuccess.run(); @@ -313,7 +316,7 @@ public class AccordCache implements CacheSize entry.savingOrWaitingToSave().identity.onSuccess(onSuccess); } - private void evict(AccordCacheEntry node, boolean updateUnreferenced) + private void evict(AccordCacheEntry node, boolean updateUnreferenced) { if (logger.isTraceEnabled()) logger.trace("Evicting {}", node); @@ -336,25 +339,25 @@ public class AccordCache implements CacheSize if (node.status() == LOADED && VALIDATE_LOAD_ON_EVICT) owner.validateLoadEvicted(node); - AccordCacheEntry self = node.owner.remove(node.key()); + AccordCacheEntry self = node.owner.remove(node.key()); Invariants.require(self.references() == 0); require(self == node, "Leaked node detected; was attempting to remove %s but cache had %s", node, self); node.notifyListeners(Listener::onEvict); node.evicted(); } - Collection> load(LoadExecutor loadExecutor, P1 p1, P2 p2, AccordCacheEntry node) + Loading load(LoadExecutor loadExecutor, P1 p1, P2 p2, AccordCacheEntry node) { - return node.load(loadExecutor, p1, p2).waiters(); + return node.load(loadExecutor, p1, p2); } - void loaded(AccordCacheEntry node, V value) + void loaded(AccordCacheEntry node, V value) { node.loaded(value); node.notifyListeners(Listener::onUpdate); } - void failedToLoad(AccordCacheEntry node) + void failedToLoad(AccordCacheEntry node) { Invariants.require(node.references() == 0); if (node.isUnqueued()) @@ -367,38 +370,38 @@ public class AccordCache implements CacheSize evict(node, true); } - void saved(AccordCacheEntry node, Object identity, Throwable fail) + void saved(AccordCacheEntry node, Object identity, Throwable fail) { if (node.saved(identity, fail) && node.references() == 0 && node.isUnqueued()) evictQueue.addFirst(node); // add to front since we have just saved, so we were eligible for eviction } - public > void release(S safeRef, AccordTask owner) + public & AccordSafeState> void release(S safeRef, AccordTask owner) { safeRef.global().owner.release(safeRef, owner); } - public > Type newType(Class keyClass, Adapter adapter, AccordCacheMetrics.Shard metrics) + public & AccordSafeState> Type newType(Class keyClass, Adapter adapter, AccordCacheMetrics.Shard metrics) { Type instance = new Type<>(keyClass, adapter, metrics); types.add(instance); return instance; } - public > Type newType( + public & AccordSafeState> Type newType( Class keyClass, BiFunction loadFunction, QuadFunction saveFunction, Function quickShrink, TriFunction validateFunction, ToLongFunction heapEstimator, - Function, S> safeRefFactory, + Function, S> safeRefFactory, AccordCacheMetrics.Shard metrics) { return newType(keyClass, loadFunction, saveFunction, quickShrink, (i, j) -> j, (c, i, j) -> (V)j, validateFunction, heapEstimator, i -> 0, safeRefFactory, metrics); } - public > Type newType( + public & AccordSafeState> Type newType( Class keyClass, BiFunction loadFunction, QuadFunction saveFunction, @@ -408,7 +411,7 @@ public class AccordCache implements CacheSize TriFunction validateFunction, ToLongFunction heapEstimator, ToLongFunction shrunkHeapEstimator, - Function, S> safeRefFactory, + Function, S> safeRefFactory, AccordCacheMetrics.Shard metrics) { return newType(keyClass, new FunctionalAdapter<>(loadFunction, saveFunction, quickShrink, @@ -425,18 +428,18 @@ public class AccordCache implements CacheSize public interface Listener { - default void onAdd(AccordCacheEntry state) {} - default void onUpdate(AccordCacheEntry state) {} - default void onEvict(AccordCacheEntry state) {} + default void onAdd(AccordCacheEntry state) {} + default void onUpdate(AccordCacheEntry state) {} + default void onEvict(AccordCacheEntry state) {} } - public class Type> implements CacheSize + public class Type & AccordSafeState> implements CacheSize { - public class Instance implements Iterable> + public class Instance implements Iterable> { final AccordCommandStore commandStore; // TODO (desired): don't need to store key separately as stored in node; ideally use a hash set that allows us to get the current entry - private final Map> cache = new Object2ObjectHashMap<>(); + private final Map> cache = new Object2ObjectHashMap<>(); private List> listeners = null; // TODO (expected): update this after releasing the lock private OrderedKeys orderedKeys; @@ -446,50 +449,51 @@ public class AccordCache implements CacheSize this.commandStore = commandStore; } - public S acquire(K key) + public final S acquire(K key) { - AccordCacheEntry node = acquire(key, false); + AccordCacheEntry node = acquire(key, false); return adapter.safeRef(node); } - public S acquireIfLoaded(K key) + public final S acquireIfLoadedAndPermitted(K key) { - AccordCacheEntry node = acquire(key, true); + AccordCacheEntry node = acquire(key, true); if (node == null) return null; return adapter.safeRef(node); } - public S acquire(AccordCacheEntry node) + public final S acquire(AccordCacheEntry node) { Invariants.require(node.owner == this); acquireExisting(node, false); return adapter.safeRef(node); } - public void recordPreAcquired(AccordSafeState ref) + public final void recordPreAcquired(AccordCacheEntry entry) { - Invariants.require(ref.global().owner == this); - incrementCacheHits(); + Invariants.require(entry.owner == this); + if (entry.isLoaded()) incrementCacheHits(); + else incrementCacheMisses(); } - private AccordCacheEntry acquire(K key, boolean onlyIfLoaded) + private AccordCacheEntry acquire(K key, boolean onlyIfLoadedAndPermitted) { - AccordCacheEntry node = cache.get(key); + AccordCacheEntry node = cache.get(key); return node == null - ? acquireAbsent(key, onlyIfLoaded) - : acquireExisting(node, onlyIfLoaded); + ? acquireAbsent(key, onlyIfLoadedAndPermitted) + : acquireExisting(node, onlyIfLoadedAndPermitted); } /* * Can only return a LOADING Node (or null) */ - private AccordCacheEntry acquireAbsent(K key, boolean onlyIfLoaded) + private AccordCacheEntry acquireAbsent(K key, boolean onlyIfLoaded) { incrementCacheMisses(); if (onlyIfLoaded) return null; - AccordCacheEntry node = adapter.newEntry(key, this); + AccordCacheEntry node = adapter.newEntry(key, this); node.increment(); Object prev = cache.put(key, node); @@ -506,7 +510,7 @@ public class AccordCache implements CacheSize /* * Can't return EVICTED or INITIALIZED */ - private AccordCacheEntry acquireExisting(AccordCacheEntry node, boolean onlyIfLoaded) + private AccordCacheEntry acquireExisting(AccordCacheEntry node, boolean onlyIfLoadedAndPermitted) { boolean isLoaded = node.isLoaded(); if (isLoaded) @@ -514,8 +518,11 @@ public class AccordCache implements CacheSize else incrementCacheMisses(); - if (onlyIfLoaded && !isLoaded) - return null; + if (onlyIfLoadedAndPermitted) + { + if (!isLoaded || node.hasFifoOrLocked()) + return null; + } if (node.increment() == 1) { @@ -527,21 +534,21 @@ public class AccordCache implements CacheSize return node; } - public void release(AccordSafeState safeRef, AccordTask owner) + public final void release(S safeRef, AccordTask owner) { K key = safeRef.global().key(); logger.trace("Releasing resources for {}: {}", key, safeRef); - AccordCacheEntry node = cache.get(key); + AccordCacheEntry node = cache.get(key); - require(!safeRef.isUnsafe()); + require(!safeRef.isReleased()); require(safeRef.global() != null, "safeRef node is null for %s", key); require(safeRef.global() == node, "safeRef node not in map: %s != %s", safeRef.global(), node); require(node.references() > 0, "references (%d) are zero for %s (%s)", node.references(), key, node); require(node.isUnqueued()); boolean evict = false; - if (safeRef.hasUpdate()) + if (safeRef.isModified()) { V update = safeRef.current(); if (update != null) @@ -555,15 +562,12 @@ public class AccordCache implements CacheSize } node.notifyListeners(Listener::onUpdate); } - else if (node.isLoadingOrWaiting()) - { - node.loadingOrWaiting().remove(owner); - } else { evict = node.is(LOADED) && node.isNull(); } - safeRef.markUnsafe(); + node.remove(owner, safeRef.isSafe()); + safeRef.setReleased(); if (node.decrement() == 0) { @@ -596,9 +600,9 @@ public class AccordCache implements CacheSize tryShrinkOrEvict = true; } - AccordCacheEntry remove(K key) + final AccordCacheEntry remove(K key) { - AccordCacheEntry result = cache.remove(key); + AccordCacheEntry result = cache.remove(key); if (orderedKeys != null && result != null) orderedKeys.remove(key); return result; @@ -609,7 +613,7 @@ public class AccordCache implements CacheSize return Type.this; } - public Iterable keysBetween(K start, boolean startInclusive, K end, boolean endInclusive) + public final Iterable keysBetween(K start, boolean startInclusive, K end, boolean endInclusive) { if (orderedKeys == null) orderedKeys = new OrderedKeys<>(adapter.keyComparator(), cache.keySet()); @@ -618,15 +622,15 @@ public class AccordCache implements CacheSize } @Override - public Iterator> iterator() + public final Iterator> iterator() { return cache.values().iterator(); } - void validateLoadEvicted(AccordCacheEntry node) + final void validateLoadEvicted(AccordCacheEntry node) { @SuppressWarnings("unchecked") - AccordCacheEntry state = (AccordCacheEntry) node; + AccordCacheEntry state = (AccordCacheEntry) node; K key = state.key(); V evicted = state.tryGetFull(); if (evicted == null) @@ -649,46 +653,46 @@ public class AccordCache implements CacheSize } @VisibleForTesting - public AccordCacheEntry getUnsafe(K key) + public final AccordCacheEntry getUnsafe(K key) { return cache.get(key); } @VisibleForTesting - public boolean isReferenced(K key) + public final boolean isReferenced(K key) { - AccordCacheEntry node = cache.get(key); + AccordCacheEntry node = cache.get(key); return node != null && node.references() > 0; } @VisibleForTesting - boolean keyIsReferenced(Object key, Class> valClass) + final boolean keyIsReferenced(Object key, Class> valClass) { - AccordCacheEntry node = cache.get(key); + AccordCacheEntry node = cache.get(key); return node != null && node.references() > 0; } @VisibleForTesting - boolean keyIsCached(Object key, Class> valClass) + final boolean keyIsCached(Object key, Class> valClass) { - AccordCacheEntry node = cache.get(key); + AccordCacheEntry node = cache.get(key); return node != null; } @VisibleForTesting - int references(Object key, Class> valClass) + final int references(Object key, Class> valClass) { - AccordCacheEntry node = cache.get(key); + AccordCacheEntry node = cache.get(key); return node != null ? node.references() : 0; } - void notifyListeners(BiConsumer, AccordCacheEntry> notify, AccordCacheEntry node) + final void notifyListeners(BiConsumer, AccordCacheEntry> notify, AccordCacheEntry node) { notifyListeners(listeners, notify, node); notifyListeners(typeListeners, notify, node); } - void notifyListeners(List> listeners, BiConsumer, AccordCacheEntry> notify, AccordCacheEntry node) + final void notifyListeners(List> listeners, BiConsumer, AccordCacheEntry> notify, AccordCacheEntry node) { if (listeners != null) { @@ -698,20 +702,20 @@ public class AccordCache implements CacheSize } } - public void register(Listener l) + public final void register(Listener l) { if (listeners == null) listeners = new ArrayList<>(); listeners.add(l); } - public void unregister(Listener l) + public final void unregister(Listener l) { if (!tryUnregister(l)) throw illegalState("Listener was not registered"); } - public boolean tryUnregister(Listener l) + public final boolean tryUnregister(Listener l) { if (listeners == null || !listeners.remove(l)) return false; @@ -719,9 +723,23 @@ public class AccordCache implements CacheSize listeners = null; return true; } + + final boolean isCommandsForKey() + { + return getClass() == KeyInstance.class; + } } - private final Class keyClass; + // KeyInstance exists to provide us slightly easier discrimination about the Type an AccordCacheEntry is associated with + public final class KeyInstance extends Instance + { + public KeyInstance(AccordCommandStore commandStore) + { + super(commandStore); + } + } + + private final Class keyClass; // type of key, useful primarily for toString(), but also piggyback for deciding Instance type private Adapter adapter; private long bytesCached; private int size; @@ -734,6 +752,8 @@ public class AccordCache implements CacheSize public Type(Class keyClass, Adapter adapter, AccordCacheMetrics.Shard metrics) { + // Integer and String permitted for testing, but the Invariant exists only to enforce that we construct the right kind of Instance + Invariants.require(keyClass == RoutingKey.class || keyClass == TxnId.class || keyClass == String.class || keyClass == Integer.class); this.keyClass = keyClass; this.adapter = adapter; this.objectSize = metrics.objectSize; @@ -751,7 +771,7 @@ public class AccordCache implements CacheSize // can be safely garbage collected if empty Instance newInstance(AccordCommandStore commandStore) { - return new Instance(commandStore); + return keyClass == RoutingKey.class ? new KeyInstance(commandStore) : new Instance(commandStore); } private void incrementCacheHits() @@ -867,17 +887,17 @@ public class AccordCache implements CacheSize } @VisibleForTesting - AccordCacheEntry head() + AccordCacheEntry head() { - Iterator> iter = evictQueue.iterator(); + Iterator> iter = evictQueue.iterator(); return iter.hasNext() ? iter.next() : null; } @VisibleForTesting - AccordCacheEntry tail() + AccordCacheEntry tail() { - AccordCacheEntry last = null; - Iterator> iter = evictQueue.iterator(); + AccordCacheEntry last = null; + Iterator> iter = evictQueue.iterator(); while (iter.hasNext()) last = iter.next(); return last; @@ -888,7 +908,7 @@ public class AccordCache implements CacheSize return size() == 0; } - Iterable> evictionQueue() + Iterable> evictionQueue() { return evictQueue::iterator; } @@ -937,10 +957,10 @@ public class AccordCache implements CacheSize return; type.register(new AccordCache.Listener<>() { - private final IdentityHashMap, CacheEvents.Evict> pendingEvicts = new IdentityHashMap<>(); + private final IdentityHashMap, CacheEvents.Evict> pendingEvicts = new IdentityHashMap<>(); @Override - public void onAdd(AccordCacheEntry state) + public void onAdd(AccordCacheEntry state) { CacheEvents.Add add = new CacheEvents.Add(); CacheEvents.Evict evict = new CacheEvents.Evict(); @@ -957,7 +977,7 @@ public class AccordCache implements CacheSize } @Override - public void onEvict(AccordCacheEntry state) + public void onEvict(AccordCacheEntry state) { CacheEvents.Evict event = pendingEvicts.remove(state); if (event == null) return; @@ -967,7 +987,7 @@ public class AccordCache implements CacheSize }); } - private static void updateMutable(AccordCache.Type type, AccordCacheEntry state, CacheEvents event) + private static void updateMutable(AccordCache.Type type, AccordCacheEntry state, CacheEvents event) { event.status = state.status().name(); @@ -987,7 +1007,7 @@ public class AccordCache implements CacheSize event.update(); } - static class FunctionalAdapter implements Adapter + static class FunctionalAdapter & AccordSafeState> implements Adapter { final BiFunction load; final QuadFunction save; @@ -997,8 +1017,8 @@ public class AccordCache implements CacheSize final TriFunction validate; final ToLongFunction estimateHeapSize; final ToLongFunction estimateShrunkHeapSize; - final Function, S> newSafeRef; - final BiFunction.Instance, AccordCacheEntry> newNode; + final Function, S> newSafeRef; + final BiFunction.Instance, AccordCacheEntry> newNode; FunctionalAdapter(BiFunction load, QuadFunction save, @@ -1007,8 +1027,8 @@ public class AccordCache implements CacheSize TriFunction validate, ToLongFunction estimateHeapSize, ToLongFunction estimateShrunkHeapSize, - Function, S> newSafeRef, - BiFunction.Instance, AccordCacheEntry> newNode) + Function, S> newSafeRef, + BiFunction.Instance, AccordCacheEntry> newNode) { this.load = load; this.save = save; @@ -1082,13 +1102,13 @@ public class AccordCache implements CacheSize } @Override - public S safeRef(AccordCacheEntry node) + public S safeRef(AccordCacheEntry node) { return newSafeRef.apply(node); } @Override - public AccordCacheEntry newEntry(K key, Type.Instance owner) + public AccordCacheEntry newEntry(K key, Type.Instance owner) { return newNode.apply(key, owner); } @@ -1100,7 +1120,7 @@ public class AccordCache implements CacheSize } } - static class SettableWrapper extends FunctionalAdapter + static class SettableWrapper & AccordSafeState> extends FunctionalAdapter { volatile BiFunction load; @@ -1110,9 +1130,9 @@ public class AccordCache implements CacheSize this.load = super.load; } - public static Adapter loadOnly(BiFunction load) + public static & AccordSafeState> Adapter loadOnly(BiFunction load) { - SettableWrapper result = new SettableWrapper<>(new NoOpAdapter<>()); + SettableWrapper result = new SettableWrapper<>(new NoOpAdapter()); result.load = load; return result; } @@ -1124,7 +1144,7 @@ public class AccordCache implements CacheSize } } - static class NoOpAdapter implements Adapter + static class NoOpAdapter & AccordSafeState> implements Adapter { @Override public V load(AccordCommandStore commandStore, K key) { return null; } @Override public Runnable save(AccordCommandStore commandStore, K key, @Nullable V value, @Nullable Object shrunk) { return null; } @@ -1135,7 +1155,7 @@ public class AccordCache implements CacheSize @Override public long estimateHeapSize(V value) { return 0; } @Override public long estimateShrunkHeapSize(Object shrunk) { return 0; } @Override public boolean validate(AccordCommandStore commandStore, K key, V value) { return false; } - @Override public S safeRef(AccordCacheEntry node) { return null; } + @Override public S safeRef(AccordCacheEntry node) { return null; } } public static class CommandsForKeyAdapter implements Adapter @@ -1239,7 +1259,7 @@ public class AccordCache implements CacheSize } @Override - public AccordSafeCommandsForKey safeRef(AccordCacheEntry node) + public AccordSafeCommandsForKey safeRef(AccordCacheEntry node) { return new AccordSafeCommandsForKey(node); } @@ -1251,7 +1271,7 @@ public class AccordCache implements CacheSize } @Override - public AccordCacheEntry newEntry(RoutingKey key, Type.Instance owner) + public AccordCacheEntry newEntry(RoutingKey key, Type.Instance owner) { CommandsForKeyCacheEntry entry = new CommandsForKeyCacheEntry(key, owner); entry.readyToLoad(); @@ -1376,19 +1396,19 @@ public class AccordCache implements CacheSize } @Override - public AccordSafeCommand safeRef(AccordCacheEntry node) + public AccordSafeCommand safeRef(AccordCacheEntry node) { return new AccordSafeCommand(node); } @Override - public AccordCacheEntry newEntry(TxnId txnId, Type.Instance owner) + public AccordCacheEntry newEntry(TxnId txnId, Type.Instance owner) { - AccordCacheEntry node = new AccordCacheEntry<>(txnId, owner); + AccordCacheEntry node = new AccordCacheEntry<>(txnId, owner); if (txnId.is(Txn.Kind.EphemeralRead)) { node.initialize(null); - int maxAge = (int)Math.min(0xff, 1 + DatabaseDescriptor.getReadRpcTimeout(TimeUnit.SECONDS)); + int maxAge = (int)Math.min(AGE_MASK, 1 + DatabaseDescriptor.getReadRpcTimeout(TimeUnit.SECONDS)); node.markNoEvict(owner.parent().parent().noEvictGeneration, maxAge); } else diff --git a/src/java/org/apache/cassandra/service/accord/AccordCacheEntry.java b/src/java/org/apache/cassandra/service/accord/AccordCacheEntry.java index 77b56f3203..11d8a2b105 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordCacheEntry.java +++ b/src/java/org/apache/cassandra/service/accord/AccordCacheEntry.java @@ -18,27 +18,43 @@ package org.apache.cassandra.service.accord; import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.function.BiConsumer; +import java.util.function.BiPredicate; import javax.annotation.Nullable; import com.google.common.annotations.VisibleForTesting; import com.google.common.primitives.Ints; +import accord.local.SafeState; import accord.utils.ArrayBuffers.BufferList; import accord.utils.IntrusiveLinkedList; import accord.utils.IntrusiveLinkedListNode; import accord.utils.Invariants; +import accord.utils.SortedArrays; +import accord.utils.TriConsumer; +import accord.utils.UnhandledEnum; import accord.utils.async.Cancellable; import org.apache.cassandra.service.accord.AccordCache.Adapter; import org.apache.cassandra.service.accord.AccordCache.Adapter.Shrink; +import org.apache.cassandra.service.accord.AccordExecutor.IOTask; import org.apache.cassandra.utils.ObjectSizes; +import static accord.utils.Invariants.nonNull; +import static org.apache.cassandra.service.accord.AccordCacheEntry.LockMode.HOLD_QUEUE; +import static org.apache.cassandra.service.accord.AccordCacheEntry.LockMode.UNLOCKED; +import static org.apache.cassandra.service.accord.AccordCacheEntry.Queue.compare; +import static org.apache.cassandra.service.accord.AccordCacheEntry.RunnableStatus.NEWLY_BLOCKING_RUNNABLE; +import static org.apache.cassandra.service.accord.AccordCacheEntry.RunnableStatus.NEWLY_RUNNABLE; +import static org.apache.cassandra.service.accord.AccordCacheEntry.RunnableStatus.NOT_RUNNABLE; +import static org.apache.cassandra.service.accord.AccordCacheEntry.RunnableStatus.STILL_RUNNABLE; +import static org.apache.cassandra.service.accord.AccordCacheEntry.RunnableStatus.STILL_RUNNABLE_NEWLY_BLOCKING; import static org.apache.cassandra.service.accord.AccordCacheEntry.Status.EVICTED; import static org.apache.cassandra.service.accord.AccordCacheEntry.Status.FAILED_TO_LOAD; import static org.apache.cassandra.service.accord.AccordCacheEntry.Status.FAILED_TO_SAVE; @@ -52,7 +68,7 @@ import static org.apache.cassandra.service.accord.AccordCacheEntry.Status.WAITIN /** * Global (per CommandStore) state of a cached entity (Command or CommandsForKey). */ -public class AccordCacheEntry extends IntrusiveLinkedListNode +public class AccordCacheEntry & AccordSafeState> extends IntrusiveLinkedListNode { public enum Status { @@ -128,9 +144,12 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode } } - static final int STATUS_MASK = 0x0000001F; - static final int SHRUNK = 0x00000040; - static final int NO_EVICT = 0x00000020; + static final int STATUS_MASK = 0x0000001F; + static final int NO_EVICT = 0x00000020; + static final int SHRUNK = 0x00000040; + static final int LOCKED_MASK = 0x00000180; + static final int LOCKED_SHIFT = Integer.numberOfTrailingZeros(LOCKED_MASK); + static final int LOCKED_HOLDING_QUEUE = HOLD_QUEUE.ordinal() << LOCKED_SHIFT; static final int IS_NOT_EVICTED = 0xF; static final int IS_LOADED = 0x8; static final int IS_NESTED = 0x4; @@ -138,117 +157,1045 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode static final int IS_LOADING_OR_WAITING = 0x2; static final int IS_SAVING_OR_WAITING_MASK = 0xE; static final int IS_SAVING_OR_WAITING = 0xC; + static final int GENERATION_SHIFT = 9; + static final int GENERATION_MASK = 0x7fff; + static final int AGE_SHIFT = 24; + static final int AGE_MASK = 0xff; + + static class Queue + { + private static final int DEFAULT_CAPACITY = 4; + private static final int LOCKED_INDEX = 0; + private static final int PRIORITY_START_INDEX = LOCKED_INDEX + 1; + /** + * [priorityHead..priorityTail) stores a priority-sorted list of tasks + * (fifoTail...fifoHead] stores a fifo queue that runs ahead of any priority tasks + * (fifoTail-unsequencedCount...fifoTail] stores unsequenced tasks that are waiting for a queued incremental task. + * This only happens for TxnId cache entries, since they may lockAndHoldQueue. Once the lock is released, + * any pending unsequenced tasks are notified and immediately made (irrevocably) runnable for this entry. + */ + private AccordTask[] tasks; + // TODO (expected): use bytes/shorts for indexes to keep size down, and have an expanded version of the Queue + // with better algorithmic complexity (e.g. Hash -> IntrusivePriorityHeap) + private int priorityHead, priorityTail, fifoHead, fifoTail; + private int unsequencedSize; + + public Queue() + { + tasks = new AccordTask[DEFAULT_CAPACITY]; + priorityHead = priorityTail = PRIORITY_START_INDEX; + fifoHead = fifoTail = DEFAULT_CAPACITY - 1; + } + + private Queue(Queue copy) + { + tasks = copy.tasks.clone(); + priorityHead = copy.priorityHead; + priorityTail = copy.priorityTail; + fifoHead = copy.fifoHead; + fifoTail = copy.fifoTail; + } + + // returns true if no fifo tasks already queue (i.e. so we become head) + boolean addFifo(AccordTask task) + { + ensureCapacity(); + boolean isHead = fifoHead == fifoTail; + if (unsequencedSize > 0) // simply displace the unsequence task, as they're an unordered list + tasks[fifoTail - unsequencedSize] = tasks[fifoTail]; + tasks[fifoTail--] = task; + validate(); + return isHead; + } + + boolean addUnsequenced(AccordTask task) + { + Invariants.require(task.isUnsequenced()); + ensureCapacity(); + tasks[fifoTail - unsequencedSize++] = task; + return unsequencedSize == 1 && sequencedSize() == 1; + } + + boolean isLocked(AccordTask task) + { + return tasks[LOCKED_INDEX] == task; + } + + AccordTask lockedBy() + { + return tasks[LOCKED_INDEX]; + } + + boolean removeIfHead(AccordTask task) + { + return removeIfFifoHead(task) || removeIfPriorityHead(task); + } + + boolean removeIfFifoHead(AccordTask task) + { + if (!hasFifo()) + return false; + + if (task != tasks[fifoHead]) + return false; + tasks[fifoHead--] = null; + return true; + } + + boolean removeIfPriorityHead(AccordTask task) + { + if (!hasPriority()) + return false; + + if (task != tasks[priorityHead]) + return false; + tasks[priorityHead++] = null; + return true; + } + + void lock(AccordTask task) + { + tasks[LOCKED_INDEX] = task; + } + + void unlock(AccordTask task) + { + Invariants.require(tasks[LOCKED_INDEX] == task); + tasks[LOCKED_INDEX] = null; + } + + // should always return false, as should never be invoked on an empty queue, and returns true only if we're head of the queue + boolean addPrioritised(AccordTask task) + { + ensureCapacity(); + int insertPos = Arrays.binarySearch(tasks, priorityHead, priorityTail, task, Queue::compare); + if (insertPos < 0) + insertPos = -1 - insertPos; + + if (priorityHead == PRIORITY_START_INDEX || insertPos > (priorityTail + priorityHead)/2) + { + System.arraycopy(tasks, insertPos, tasks, insertPos + 1, priorityTail - insertPos); + tasks[insertPos] = task; + priorityTail++; + } + else + { + System.arraycopy(tasks, priorityHead, tasks, priorityHead - 1, insertPos - priorityHead); + tasks[insertPos - 1] = task; + priorityHead--; + } + + validate(); + return fifoHead == fifoTail && tasks[priorityHead] == task; + } + + // should always return false, as should never be invoked on an empty queue, and returns true only if we're head of the queue + void addWaitingToLoad(AccordTask task) + { + ensureCapacity(); + tasks[priorityTail++] = task; + } + + private boolean hasTailRoom() + { + if (priorityTail + unsequencedSize <= fifoTail) + return true; + Invariants.require(priorityTail + unsequencedSize == 1 + fifoTail); + return false; + } + + private void ensureCapacity() + { + if (!hasTailRoom()) + { + if (fifoHead == fifoTail && unsequencedSize == 0 && fifoTail < tasks.length - 1) fifoHead = fifoTail = tasks.length - 1; + else if (priorityHead == priorityTail && priorityHead > PRIORITY_START_INDEX) priorityHead = priorityTail = PRIORITY_START_INDEX; + else if (totalSize() >= (tasks.length - 1) / 2) compact(new AccordTask[tasks.length * 2]); + else compact(tasks); + Invariants.require(hasTailRoom()); + } + } + + private void compact(AccordTask[] into) + { + if (priorityHead == priorityTail) priorityHead = priorityTail = PRIORITY_START_INDEX; + else + { + int priorityLength = priorityTail - priorityHead; + System.arraycopy(tasks, priorityHead, into, PRIORITY_START_INDEX, priorityLength); + int newTail = PRIORITY_START_INDEX + priorityLength; + Invariants.require(newTail <= priorityTail); + if (into == tasks) + Arrays.fill(into, newTail, priorityTail, null); + priorityHead = PRIORITY_START_INDEX; + priorityTail = newTail; + } + + if (fifoHead == fifoTail && unsequencedSize == 0) fifoHead = fifoTail = into.length - 1; + else + { + int fifoLength = fifoHead - fifoTail; + int copyLength = fifoLength + unsequencedSize; + int copyFrom = (fifoTail - unsequencedSize) + 1; + int copyTo = into.length - copyLength; + Invariants.require(copyTo >= copyFrom); + System.arraycopy(tasks, copyFrom, into, copyTo, copyLength); + if (into == tasks) + Arrays.fill(into, copyFrom, copyTo, null); + fifoHead = into.length - 1; + fifoTail = fifoHead - fifoLength; + } + + if (tasks != into) + { + into[LOCKED_INDEX] = tasks[LOCKED_INDEX]; + tasks = into; + } + validate(); + } + + private void validate() + { + for (int i = PRIORITY_START_INDEX ; i < priorityHead ; ++i) + Invariants.require(tasks[i] == null); + for (int i = priorityHead ; i < priorityTail ; ++i) + Invariants.require(tasks[i] != null); + for (int i = priorityTail; i <= fifoTail - unsequencedSize; ++i) + Invariants.require(tasks[i] == null); + for (int i = (fifoTail - unsequencedSize) + 1; i <= fifoHead ; ++i) + Invariants.require(tasks[i] != null); + for (int i = fifoHead + 1 ; i < tasks.length ; ++i) + Invariants.require(tasks[i] == null); + } + + AccordTask peek() + { + if (hasFifo()) return tasks[fifoHead]; + if (hasPriority()) return tasks[priorityHead]; + return null; + } + + AccordTask peekFifo() + { + return hasFifo() ? tasks[fifoHead] : null; + } + + // second task + AccordTask peekBehind() + { + int fifoSize = fifoSize(); + if (fifoSize > 1) + return tasks[fifoHead - 1]; + int priorityIndex = priorityHead + (1 - fifoSize); + if (priorityIndex < priorityTail) + return tasks[priorityIndex]; + return null; + } + + boolean hasFifo() + { + return fifoHead != fifoTail; + } + + boolean hasPriority() + { + return priorityHead != priorityTail; + } + + boolean hasUnsequenced() + { + return unsequencedSize > 0; + } + + int sequencedSize() + { + return prioritySize() + fifoSize(); + } + + int unsequencedSize() + { + return unsequencedSize; + } + + int totalSize() + { + return sequencedSize() + unsequencedSize; + } + + int prioritySize() + { + return priorityTail - priorityHead; + } + + int fifoSize() + { + return fifoHead - fifoTail; + } + + // true iff was head + boolean removeFifoOrPriority(AccordTask task, boolean permitMissing) + { + int fifoIndex = fifoIndexOf(task); + if (fifoIndex >= 0) + { + if (fifoIndex == fifoHead) + { + tasks[fifoHead--] = null; + validate(); + return true; + } + else + { + if (remove(fifoIndex, fifoTail + 1, fifoHead + 1)) ++fifoTail; + else --fifoHead; + validate(); + return false; + } + } + + int priorityIndex = priorityIndexOf(task); + Invariants.require(priorityIndex >= 0 || permitMissing); + if (priorityIndex >= 0) + { + if (priorityIndex == priorityHead) + { + tasks[priorityHead++] = null; + return !hasFifo(); + } + + if (remove(priorityIndex, priorityHead, priorityTail)) ++priorityHead; + else --priorityTail; + return false; + } + + return false; + } + + boolean removeUnsequenced(AccordTask task) + { + int unsequencedIndex = unsequencedIndexOf(task); + if (unsequencedIndex < 0) + return false; + + --unsequencedSize; + tasks[unsequencedIndex] = tasks[fifoTail - unsequencedSize]; + tasks[fifoTail - unsequencedSize] = null; + return true; + } + + // return true IFF was head + private boolean removePriority(AccordTask task, boolean permitAbsent) + { + int i = priorityIndexOf(task); + if (i < 0) + { + Invariants.require(permitAbsent); + return false; + } + + boolean wasHead = i == priorityHead; + if (remove(i, priorityHead, priorityTail)) ++priorityHead; + else --priorityTail; + return wasHead; + } + + // return true if we move the start forwards, false if we moved the end back + private boolean remove(int i, int start, int end) + { + if (i < (start + end)/2) + { + System.arraycopy(tasks, start, tasks, start + 1, i - start); + tasks[start] = null; + return true; + } + else + { + System.arraycopy(tasks, i + 1, tasks, i, end - (i + 1)); + tasks[end - 1] = null; + return false; + } + } + + boolean contains(AccordTask task) + { + return indexOf(task) >= 0; + } + + private int indexOf(AccordTask task) + { + if (tasks[priorityHead] == task) + return priorityHead; + + if (tasks[fifoHead] == task) + return fifoHead; + + int i = priorityIndexOf(task); + if (i >= 0) + return i; + + return fifoIndexOf(task); + } + + private int priorityIndexOf(AccordTask task) + { + if (priorityTail - priorityHead > 16) + { + if (tasks[priorityHead] == task) + return priorityHead; + + int i = SortedArrays.binarySearch(tasks, priorityHead + 1, priorityTail, task, Queue::compare, SortedArrays.Search.CEIL); + if (i < 0) + return -1; + + while (i < priorityTail) + { + if (tasks[i] == task) + return i; + if (compare(task, tasks[i]) != 0) + break; + ++i; + } + } + + for (int i = priorityHead ; i < priorityTail ; ++i) + { + if (tasks[i] == task) + return i; + } + return -1; + } + + private int fifoIndexOf(AccordTask task) + { + for (int i = fifoHead ; i > fifoTail ; --i) + { + if (tasks[i] == task) + return i; + } + return -1; + } + + private int unsequencedIndexOf(AccordTask task) + { + for (int i = (fifoTail - unsequencedSize) + 1 ; i <= fifoTail ; ++i) + { + if (tasks[i] == task) + return i; + } + return -1; + } + + int drainUnsequenced(TriConsumer, P1, P2> forEach, P1 p1, P2 p2) + { + for (int i = (fifoTail - unsequencedSize) + 1 ; i <= fifoTail ; ++i) + { + AccordTask task = tasks[i]; + tasks[i] = null; + // should not be reentrant + forEach.accept(task, p1, p2); + } + int count = unsequencedSize; + unsequencedSize = 0; + return count; + } + + RunnableStatus ensureHeadFifo(AccordTask task) + { + if (hasFifo()) + { + Invariants.require(tasks[fifoHead] == task); + return NOT_RUNNABLE; + } + + if (tasks[priorityHead] == task) + { + tasks[priorityHead++] = null; + addFifo(task); + return STILL_RUNNABLE; + } + else + { + boolean wasPriorityHead = removePriority(task, false); + boolean isFifoHead = addFifo(task); + if (!isFifoHead) + return NOT_RUNNABLE; + if (wasPriorityHead) + return STILL_RUNNABLE; + if (hasPriority() || hasUnsequenced()) + return NEWLY_BLOCKING_RUNNABLE; + return NEWLY_RUNNABLE; + } + } + + static int compare(AccordTask a, AccordTask b) + { + Invariants.require(a != null && b != null); + int c = Long.compare(a.position, b.position); + if (c == 0) + c = a.executionContext().executionKind().compareTo(b.executionContext().executionKind()); + if (c == 0) + c = Long.compare(a.createdAt, b.createdAt); + if (c == 0) + c = Long.compare(a.loadedAt, b.loadedAt); + if (c == 0) + c = a.loggingId().compareTo(b.loggingId()); + return c; + } + + public boolean hasQueued() + { + return hasFifo() || hasPriority(); + } + } + static final long EMPTY_SIZE = ObjectSizes.measure(new AccordCacheEntry<>(null, null)); private final K key; - final AccordCache.Type.Instance owner; + final AccordCache.Type.Instance owner; private Object state; + /** + * Either a single AccordTask or a Queue object. The meaning of a single task is defined by various flags. + * If locked, then the task is not logically part of the queue unless LOCKED_HOLDING_QUEUE. + * If unlocked, or LOCKED_HOLDING_QUEUE, the task represents a single-item queue. + * If the task forms a single-item queue, whether it is FIFO or prioritised is determined by the task's isCacheQueuedFifo flag. + */ + private Object queue; // private int status; + private int unsequenced; int sizeOnHeap; private volatile int references; private static final AtomicIntegerFieldUpdater referencesUpdater = AtomicIntegerFieldUpdater.newUpdater(AccordCacheEntry.class, "references"); - AccordCacheEntry(K key, AccordCache.Type.Instance owner) + AccordCacheEntry(K key, AccordCache.Type.Instance owner) { this.key = key; this.owner = owner; } - void unlink() + private RunnableStatus validate(RunnableStatus status) + { + Invariants.require(queue != null); + AccordTask head = queue instanceof Queue ? ((Queue) queue).peek() : (AccordTask) queue; + Invariants.require(isRunnable(head) || status == NOT_RUNNABLE); + return status; + } + + // TODO (expected): don't unwrap when only one entry, since this may cause us to flap when locking unsequenced tasks + private void maybeUnwrap(Queue q) + { + int size = q.sequencedSize(); + switch (size) + { + case 0: + Invariants.require(q.unsequencedSize() == 0); + queue = isLocked() ? nonNull(q.lockedBy()) : null; + break; + + case 1: + if (isLocked() || q.unsequencedSize() > 0) + break; + queue = q.peek(); + } + } + + private boolean maybeUnwrap(Queue q, AccordTask lockedBy) + { + if (q.sequencedSize() == 0) + { + Invariants.require(q.unsequencedSize() == 0); + queue = lockedBy; + return true; + } + return false; + } + + // assumes already queued with priority + final RunnableStatus moveToFifo(AccordTask task) + { + if (queue != task) + { + Queue q = (Queue) queue; + RunnableStatus status = q.ensureHeadFifo(task); + if (status == NEWLY_BLOCKING_RUNNABLE && isLoaded()) + onChangedHead(q, null, q.peekBehind()); + return validate(isRunnable(task) ? status : NOT_RUNNABLE); + } + return validate(isRunnable(task) ? STILL_RUNNABLE : NOT_RUNNABLE); + } + + // drains ONLY those queued with addWaitingToLoad; addFifo are included in the result but are not removed from the collection + public final BufferList> drainWaitingToLoad() + { + Invariants.require(isLoading()); + Invariants.require(!isLocked()); + BufferList> list = new BufferList<>(); + if (queue != null) + { + if (queue instanceof Queue) + { + Queue q = (Queue) queue; + for (int i = q.priorityHead ; i < q.priorityTail ; ++i) + { + list.add(q.tasks[i]); + q.tasks[i] = null; + } + q.priorityHead = q.priorityTail = Queue.PRIORITY_START_INDEX; + for (int i = q.fifoHead ; i > q.fifoTail ; --i) + list.add(q.tasks[i]); + + maybeUnwrap(q); + } + else + { + AccordTask task = (AccordTask) queue; + list.add(task); + Invariants.require(!isLocked()); + if (!task.isCacheQueuedFifo()) + queue = null; + } + } + return list; + } + + final void remove(AccordTask task, boolean ownsLock) + { + if (queue instanceof Queue) + { + Queue q = (Queue) queue; + boolean remove; + boolean isLocked = isLocked() && q.isLocked(task); + Invariants.require(isLocked == ownsLock); + if (isLocked) + { + // if locked, we've already released unsequenced/pririty/fifo positions unless isLockedHoldingQueue + remove = isLockedHoldingQueue(); + status &= ~LOCKED_MASK; + q.unlock(task); + } + else if (task.isUnsequenced(this)) + { + if (task.isCacheQueued()) + { + if (!q.removeUnsequenced(task)) + releaseUnsequenced(q, task); + } + remove = false; + } + else remove = task.isCacheQueued(); + + if (remove) + { + boolean wasHead = remove && q.removeFifoOrPriority(task, false); + if (isLoaded() && wasHead) + { + unsequenced += q.drainUnsequenced(AccordTask::onChangeHeadStatus, this, NEWLY_RUNNABLE); + onChangedHead(q, q.peek(), null); + } + } + + if (remove || isLocked) + maybeUnwrap(q); + } + else if (queue == task) + { + boolean isLocked = isLocked(); + Invariants.require(isLocked == ownsLock); + if (isLocked) + { + status &= ~LOCKED_MASK; + } + else if (task.isUnsequenced(this)) + { + if (task.isCacheQueued()) --unsequenced; // nothing to release if we hit zero + else Invariants.require(isLoading()); + } + queue = null; + } + else + { + Invariants.require(!ownsLock); + if (task.isUnsequenced(this) && task.isCacheQueued()) + --unsequenced; // nothing to release if we hit zero + } + } + + final boolean isCommandsForKey() + { + return getClass() == AccordSafeCommandsForKey.CommandsForKeyCacheEntry.class; + } + + final RunnableStatus headStatus(AccordTask task) + { + if (queue == task) + return validate(isRunnable(task) ? NEWLY_RUNNABLE : NOT_RUNNABLE); + + Queue q = (Queue) queue; + if (q.peek() != task || !isRunnable(task)) + return NOT_RUNNABLE; + + return validate(q.totalSize() == 1 ? NEWLY_RUNNABLE : NEWLY_BLOCKING_RUNNABLE); + } + + private Queue ensureQueue() + { + if (queue instanceof Queue) + return (Queue) queue; + + AccordTask head = (AccordTask) this.queue; + Queue q = new Queue(); + if (isLocked()) + { + if (isLockedHoldingQueue()) + q.addFifo(head); + q.lock(head); + } + else if (head.isCacheQueuedFifo()) q.addFifo(head); + else q.addPrioritised(head); + this.queue = q; + return q; + } + + final void addWaitingToLoad(AccordTask task) + { + Invariants.require(isLoading()); + if (queue == null) queue = task; + else ensureQueue().addWaitingToLoad(task); + } + + final AccordTask head() + { + if (queue == null) + return null; + + if (queue instanceof Queue) + return ((Queue) queue).peek(); + + if (isLocked() && !isLockedHoldingQueue()) + return null; + + return (AccordTask) queue; + } + + final RunnableStatus addUnsequenced(AccordTask task) + { + Invariants.require(isLoaded()); + + AccordTask head = head(); + if (head != null && head.holdsLocksBetweenRuns()) + { + boolean wait = compare(task, head) > 0 || (unsequenced == 0 && head.hasIncrementalStarted()); + if (wait) + { + if (ensureQueue().addUnsequenced(task) && isLoaded() && unsequenced == 0) + head.onChangeHeadStatus(this, STILL_RUNNABLE_NEWLY_BLOCKING); + return NOT_RUNNABLE; + } + else + { + ++unsequenced; + if (unsequenced == 1 && isLoaded()) + head.onChangeHeadStatus(this, NOT_RUNNABLE); + return NEWLY_RUNNABLE; + } + } + + ++unsequenced; + return NEWLY_RUNNABLE; + } + + final int waitingCount() + { + Invariants.require(isLoading()); + return queue == null ? 0 : queue instanceof Queue + ? ((Queue)queue).sequencedSize() + : isLocked() == isLockedHoldingQueue() ? 1 : 0; + } + + public enum RunnableStatus + { + NOT_RUNNABLE, STILL_RUNNABLE, NEWLY_RUNNABLE, NEWLY_BLOCKING_RUNNABLE, STILL_RUNNABLE_NEWLY_BLOCKING + } + + private boolean isRunnable(AccordTask head) + { + return !head.holdsLocksBetweenRuns() || unsequenced == 0; + } + + private RunnableStatus add(AccordTask task, BiPredicate> add) + { + Object prev = this.queue; + if (prev == null) + { + queue = task; + return validate(isRunnable(task) ? NEWLY_RUNNABLE : NOT_RUNNABLE); + } + + Queue q = ensureQueue(); + if (!add.test(q, task)) + { + if (isLoaded() && q.totalSize() == 2) + { + AccordTask head = q.peek(); + if (isRunnable(head)) + head.onChangeHeadStatus(this, STILL_RUNNABLE_NEWLY_BLOCKING); + } + return NOT_RUNNABLE; + } + + boolean isRunnable = isRunnable(task); + int sequencedSize = q.sequencedSize(); + int unsequencedSize = q.unsequencedSize(); + if (sequencedSize + unsequencedSize == 1) // could have one locked and one waiting + return validate(isRunnable ? NEWLY_RUNNABLE : NOT_RUNNABLE); + + if (isLoaded() && sequencedSize > 1) + onChangedHead(q, null, q.peekBehind()); + + return validate(isRunnable ? NEWLY_BLOCKING_RUNNABLE : NOT_RUNNABLE); + } + + final RunnableStatus addPrioritised(AccordTask task) + { + Invariants.require(!isLoading()); + return add(task, Queue::addPrioritised); + } + + final RunnableStatus addFifo(AccordTask task) + { + return add(task, Queue::addFifo); + } + + private void onChangedHead(Queue q, @Nullable AccordTask notifyNewHead, @Nullable AccordTask notifyPrevHead) + { + if (notifyNewHead != null && isRunnable(notifyNewHead)) + notifyNewHead.onChangeHeadStatus(this, q.totalSize() == 1 ? NEWLY_RUNNABLE : NEWLY_BLOCKING_RUNNABLE); + if (notifyPrevHead != null && isRunnable(notifyPrevHead)) + notifyPrevHead.onChangeHeadStatus(this, NOT_RUNNABLE); + } + + public enum LockMode + { + /** + * Invalid as a parameter to lock methods, but represents the unlocked state + */ + UNLOCKED, + + /** + * If we're sequenced, remove ourselves from the relevant queue so that the next task can queue itself up. + */ + RELEASE_QUEUE, + + /** + * Hold onto our queue position (which we expect to be the head position, as we're queued to execute). + * This is used exclusively for INCR tasks that may hold onto their TxnId for multiple rounds of execution, + * and prevents later tasks from being scheduled when they will be unable to obtain the lock. + */ + HOLD_QUEUE, + + /** + * Skip all queue accounting (sequenced or unsequenced). This mode is used by optimistic referencing via + * tryLockCaches. + */ + UNQUEUED + } + + /** + * On lock we remove ourselves from the priority/fifo queues and notify the new head + */ + public final V lockExclusive(AccordTask owner, LockMode lockMode) + { + Invariants.require(!isLocked()); + Invariants.require(isRunnable(owner) || owner.isUnsequenced(this)); + + if (queue == owner) + { + Invariants.require(lockMode != UNLOCKED); + } + else if (queue == null) + { + queue = owner; + switch (lockMode) + { + default: throw UnhandledEnum.unknown(lockMode); + case UNLOCKED: throw UnhandledEnum.invalid(UNLOCKED); + case HOLD_QUEUE: throw UnhandledEnum.invalid(HOLD_QUEUE, "Must already be head of the queue"); + case RELEASE_QUEUE: + Invariants.require(owner.isUnsequenced(this) && owner.isCacheQueued(), "Must already be head of the queue"); + --unsequenced; + case UNQUEUED: + } + } + else + { + Queue q = ensureQueue(); + switch (lockMode) + { + default: throw UnhandledEnum.unknown(lockMode); + case UNLOCKED: throw UnhandledEnum.invalid(UNLOCKED); + case HOLD_QUEUE: + Invariants.require(!owner.isUnsequenced(this)); + if (q.hasFifo()) Invariants.require(q.peekFifo() == owner); + else + { + boolean wasHead = q.removeIfPriorityHead(owner); + Invariants.require(wasHead); + q.addFifo(owner); + } + q.lock(owner); + break; + case RELEASE_QUEUE: + if (owner.isUnsequenced(this)) releaseUnsequenced(q, owner); + else + { + boolean wasHead = q.removeIfHead(owner); + Invariants.require(wasHead); + if (isLoaded()) + { + unsequenced += q.drainUnsequenced(AccordTask::onChangeHeadStatus, this, NEWLY_RUNNABLE); + onChangedHead(q, q.peek(), null); + } + if (maybeUnwrap(q, owner)) + break; + } + case UNQUEUED: + q.lock(owner); + } + } + + status |= lockMode.ordinal() << LOCKED_SHIFT; + return getExclusive(); + } + + private void releaseUnsequenced(Queue q, AccordTask release) + { + Invariants.require(release.isCacheQueued()); + if (--unsequenced == 0) + { + AccordTask head = q.peek(); + if (head != null && head.holdsLocksBetweenRuns()) + onChangedHead(q, head, null); + } + } + + final boolean hasFifoOrLocked() + { + if (isLocked()) + return true; + + if (queue == null) + return false; + + if (queue instanceof AccordTask) + return ((AccordTask) queue).isCacheQueuedFifo(); + + return ((Queue)queue).hasFifo(); + } + + final void unlink() { remove(); } - boolean isUnqueued() + final boolean isUnqueued() { return isFree(); } - public K key() + public final K key() { return key; } - public int references() + public final int references() { return references; } - public int increment() + public final int increment() { return referencesUpdater.incrementAndGet(this); } - public int decrement() + public final int decrement() { return referencesUpdater.decrementAndGet(this); } - boolean isLoaded() + final boolean isLocked() + { + return (status & LOCKED_MASK) != 0; + } + + final boolean isLockedHoldingQueue() + { + return (status & LOCKED_MASK) == LOCKED_HOLDING_QUEUE; + } + + final boolean isLoaded() { return (status & IS_LOADED) != 0; } - boolean isModified() + final boolean isModified() { return (status & IS_NOT_EVICTED) >= MODIFIED.ordinal(); } - boolean isNested() + final boolean isNested() { Invariants.require(isLoaded()); return (status & IS_NESTED) != 0; } - boolean isShrunk() + final boolean isShrunk() { return (status & SHRUNK) != 0; } - public boolean is(Status status) + public final boolean is(Status status) { return (this.status & STATUS_MASK) == status.ordinal(); } - boolean isLoadingOrWaiting() + final boolean isLoading() { return (status & IS_LOADING_OR_WAITING_MASK) == IS_LOADING_OR_WAITING; } - boolean isSavingOrWaiting() + final boolean isSavingOrWaiting() { return (status & IS_SAVING_OR_WAITING_MASK) == IS_SAVING_OR_WAITING; } - public boolean isComplete() + public final boolean isComplete() { return !is(LOADING) && !is(SAVING); } - int noEvictGeneration() + final int noEvictGeneration() { Invariants.require(isNoEvict()); - return (status >>> 8) & 0xffff; + return (status >>> GENERATION_SHIFT) & GENERATION_MASK; } - int noEvictMaxAge() + final int noEvictMaxAge() { Invariants.require(isNoEvict()); - return status >>> 24; + return status >>> AGE_SHIFT; } - boolean isNoEvict() + final boolean isNoEvict() { return (status & NO_EVICT) != 0; } - int sizeOnHeap() + final int sizeOnHeap() { return sizeOnHeap; } - void updateSize(AccordCache.Type parent) + final void updateSize(AccordCache.Type parent) { // TODO (expected): we aren't weighing the keys int newSizeOnHeap = Ints.saturatedCast(EMPTY_SIZE + estimateOnHeapSize(parent.adapter())); @@ -256,7 +1203,7 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode sizeOnHeap = newSizeOnHeap; } - void initSize(AccordCache.Type parent) + final void initSize(AccordCache.Type parent) { // TODO (expected): we aren't weighing the keys sizeOnHeap = Ints.saturatedCast(EMPTY_SIZE); @@ -265,7 +1212,7 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode } @Override - public String toString() + public final String toString() { return "Node{" + status() + ", key=" + key() + @@ -273,7 +1220,7 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode "}@" + Integer.toHexString(System.identityHashCode(this)); } - public Status status() + public final Status status() { return Status.VALUES[(status & STATUS_MASK)]; } @@ -290,42 +1237,36 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode status |= newStatus.ordinal(); } - public void initialize(V value) + public final void initialize(V value) { Invariants.require(state == null); setStatus(LOADED); state = value; } - public void readyToLoad() + public final void readyToLoad() { Invariants.require(state == null); setStatus(WAITING_TO_LOAD); - state = new WaitingToLoad(); } - public void markNoEvict(int generation, int maxAge) + public final void markNoEvict(int generation, int maxAge) { - Invariants.require((maxAge & ~0xff) == 0); - Invariants.require((generation & ~0xffff) == 0); + Invariants.require((maxAge & ~AGE_MASK) == 0); + Invariants.require((generation & ~GENERATION_MASK) == 0); status |= NO_EVICT; - status |= generation << 8; - status |= maxAge << 24; + status |= generation << GENERATION_SHIFT; + status |= maxAge << AGE_SHIFT; } - public LoadingOrWaiting loadingOrWaiting() - { - return (LoadingOrWaiting)state; - } - - void notifyListeners(BiConsumer, AccordCacheEntry> notify) + final void notifyListeners(BiConsumer, AccordCacheEntry> notify) { owner.notifyListeners(notify, this); } - public interface LoadExecutor + interface LoadExecutor { - Cancellable load(P1 p1, P2 p2, AccordCacheEntry entry); + IOTask load(P1 p1, P2 p2, AccordCacheEntry entry); } // functions as both an identity object, and a register of listeners @@ -355,32 +1296,31 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode } } - public interface SaveExecutor + interface SaveExecutor { - Cancellable save(AccordCacheEntry saving, UniqueSave identity, Runnable save); + Cancellable save(AccordCacheEntry saving, UniqueSave identity, Runnable save); } - public Loading load(LoadExecutor loadExecutor, P1 p1, P2 p2) + final Loading load(LoadExecutor loadExecutor, P1 p1, P2 p2) { Invariants.require(is(WAITING_TO_LOAD), "%s", this); - WaitingToLoad cur = (WaitingToLoad)state; - Loading loading = cur.load(loadExecutor.load(p1, p2, this)); + Loading loading = new Loading(loadExecutor.load(p1, p2, this)); setStatus(LOADING); state = loading; return loading; } - public Loading testLoad() + public final Loading testLoad() { Invariants.require(is(WAITING_TO_LOAD)); - Loading loading = ((WaitingToLoad)state).load(() -> {}); + Loading loading = new Loading(null); setStatus(LOADING); state = loading; return loading; } - public Loading loading() + public final Loading loading() { Invariants.require(is(LOADING), "%s", this); return (Loading) state; @@ -388,7 +1328,7 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode // must own the cache's lock when invoked. this is true of most methods in the class, // but this one is less obvious so named as to draw attention - public V getExclusive() + public final V getExclusive() { Invariants.require(owner == null || owner.commandStore == null || owner.commandStore.executor().isOwningThread()); Invariants.require(isLoaded(), "%s", this); @@ -399,14 +1339,19 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode updateSize(parent); } - return (V)unwrap(); + return (V) maybeUnwrap(); } - public Object getOrShrunkExclusive() + public final void releaseExclusive(S safeState, AccordTask task) + { + owner.release(safeState, task); + } + + public final Object getOrShrunkExclusive() { Invariants.require(owner == null || owner.commandStore == null || owner.commandStore.executor().isOwningThread()); Invariants.require(isLoaded(), "%s", this); - return unwrap(); + return maybeUnwrap(); } public V tryGetExclusive() @@ -414,10 +1359,10 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode Invariants.require(owner == null || owner.commandStore == null || owner.commandStore.executor().isOwningThread()); if (!isLoaded() || isShrunk()) return null; - return (V)unwrap(); + return (V) maybeUnwrap(); } - private Object unwrap() + private Object maybeUnwrap() { return isNested() ? ((Nested)state).state : state; } @@ -475,7 +1420,7 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode if (isShrunk() || state == null) return Shrink.EVICT; - V cur = (V)unwrap(); + V cur = (V) maybeUnwrap(); Shrink shrink = adapter.decideFullShrink(key, cur); if (shrink == Shrink.PERFORM_WITHOUT_LOCK) return Shrink.PERFORM_WITHOUT_LOCK; @@ -489,12 +1434,12 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode V tryGetFull() { - return isShrunk() ? null : (V)unwrap(); + return isShrunk() ? null : (V) maybeUnwrap(); } Object tryGetShrunk() { - return isShrunk() ? unwrap() : null; + return isShrunk() ? maybeUnwrap() : null; } boolean isNull() @@ -583,7 +1528,7 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode return (SavingOrWaitingToSave) state; } - public AccordCacheEntry evicted() + public AccordCacheEntry evicted() { if (isNoEvict()) setStatusUnsafe(EVICTED); @@ -597,12 +1542,12 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode return ((FailedToSave)state).cause; } - void tryApplyShrink(Object cur, Object upd, IntrusiveLinkedList> queue) + void tryApplyShrink(Object cur, Object upd, IntrusiveLinkedList> queue) { if (references() > 0 || !isUnqueued()) return; - if (isLoaded() && unwrap() == cur && upd != cur && upd != null) + if (isLoaded() && maybeUnwrap() == cur && upd != cur && upd != null) applyShrink(owner.parent(), cur, upd); queue.addLast(this); } @@ -632,74 +1577,18 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode private long estimateOnHeapSize(Adapter adapter) { - Object current = unwrap(); + Object current = maybeUnwrap(); if (current == null) return 0; else if (isShrunk()) return adapter.estimateShrunkHeapSize(current); return adapter.estimateHeapSize((V)current); } - public static abstract class LoadingOrWaiting + public static class Loading { - Collection> waiters; + final IOTask loading; - public LoadingOrWaiting() + Loading(IOTask loading) { - } - - public LoadingOrWaiting(Collection> waiters) - { - this.waiters = waiters; - } - - public Collection> waiters() - { - return waiters != null ? waiters : Collections.emptyList(); - } - - public BufferList> copyWaiters() - { - BufferList> list = new BufferList<>(); - if (waiters != null) - list.addAll(waiters); - return list; - } - - public void add(AccordTask waiter) - { - if (waiters == null) - waiters = new ArrayList<>(); - waiters.add(waiter); - } - - public void remove(AccordTask waiter) - { - if (waiters != null) - { - waiters.remove(waiter); - if (waiters.isEmpty()) - waiters = null; - } - } - } - - static class WaitingToLoad extends LoadingOrWaiting - { - public Loading load(Cancellable loading) - { - Invariants.paranoid(waiters == null || !waiters.isEmpty()); - Loading result = new Loading(waiters, loading); - waiters = Collections.emptyList(); - return result; - } - } - - static class Loading extends LoadingOrWaiting - { - public final Cancellable loading; - - public Loading(Collection> waiters, Cancellable loading) - { - super(waiters); this.loading = loading; } } @@ -755,9 +1644,9 @@ public class AccordCacheEntry extends IntrusiveLinkedListNode } } - public static AccordCacheEntry createReadyToLoad(K key, AccordCache.Type.Instance owner) + public static & AccordSafeState> AccordCacheEntry createReadyToLoad(K key, AccordCache.Type.Instance owner) { - AccordCacheEntry node = new AccordCacheEntry<>(key, owner); + AccordCacheEntry node = new AccordCacheEntry<>(key, owner); node.readyToLoad(); return node; } diff --git a/src/java/org/apache/cassandra/service/accord/AccordCommandStore.java b/src/java/org/apache/cassandra/service/accord/AccordCommandStore.java index 60205bf890..aeae9844e5 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordCommandStore.java +++ b/src/java/org/apache/cassandra/service/accord/AccordCommandStore.java @@ -56,7 +56,6 @@ import accord.impl.progresslog.TxnState; import accord.local.Command; import accord.local.CommandStore; import accord.local.CommandStores.RangesForEpoch; -import accord.local.CommandSummaries; import accord.local.ExecutionContext; import accord.local.ExecutionContext.Empty; import accord.local.MaxConflicts; @@ -178,13 +177,16 @@ public class AccordCommandStore extends CommandStore @Override public AccordSafeCommand acquireIfLoaded(TxnId txnId) { - return commands().acquireIfLoaded(txnId); + // note: we must return false if the entry is locked to enforce ordering. + // note importantly that this is also coupled to the safety of synchronously releasing ExclusiveExecutor.owner, + // rather than waiting until the (potentially asynchronous) cleanup of the task completes + return commands().acquireIfLoadedAndPermitted(txnId); } @Override public AccordSafeCommandsForKey acquireIfLoaded(RoutingKey key) { - return commandsForKeys().acquireIfLoaded(key); + return commandsForKeys().acquireIfLoadedAndPermitted(key); } @Override @@ -299,7 +301,7 @@ public class AccordCommandStore extends CommandStore void tryPreSetup(AccordTask task) { if (inStore() && current != null) - task.presetup(current.task); + task.preSetup(current.task); } public final TableId tableId() @@ -432,13 +434,19 @@ public class AccordCommandStore extends CommandStore return taskExecutor().tryExecuteImmediately(run); } - public AccordSafeCommandStore begin(AccordTask operation, @Nullable CommandSummaries commandsForRanges) + public AccordSafeCommandStore begin(AccordSafeCommandStore safeStore) { require(current == null); - current = AccordSafeCommandStore.create(operation, commandsForRanges, this); + current = safeStore; return current; } + public void complete(AccordSafeCommandStore store) + { + require(current == store); + current = null; + } + public boolean hasSafeStore() { return current != null; @@ -454,19 +462,6 @@ public class AccordCommandStore extends CommandStore return progressLog; } - public void complete(AccordSafeCommandStore store) - { - require(current == store); - current.postExecute(); - current = null; - } - - public void abort(AccordSafeCommandStore store) - { - Invariants.require(store == current); - current = null; - } - @Override public void shutdown() { @@ -650,7 +645,7 @@ public class AccordCommandStore extends CommandStore public Ready() { super(1); } @Override public void run() { decrement(); } - void maybeFlush(ExclusiveCaches caches, AccordCacheEntry e) + void maybeFlush(ExclusiveCaches caches, AccordCacheEntry e) { if (e.isModified()) { @@ -665,7 +660,7 @@ public class AccordCommandStore extends CommandStore { if (ranges == null) { - for (AccordCacheEntry e : caches.commandsForKeys()) + for (AccordCacheEntry e : caches.commandsForKeys()) ready.maybeFlush(caches, e); } else @@ -717,7 +712,7 @@ public class AccordCommandStore extends CommandStore if (!maybeShouldReplay(txnId)) return AsyncChains.success(null); - return commandStore.chain(ExecutionContext.contextFor(txnId, "Replay"), safeStore -> { + return commandStore.chain(ExecutionContext.unsequenced(txnId, "Replay"), safeStore -> { Replay replay = shouldReplay(txnId, safeStore.unsafeGet(txnId).current().participants()); if (replay == Replay.NONE) return null; diff --git a/src/java/org/apache/cassandra/service/accord/AccordCommandStores.java b/src/java/org/apache/cassandra/service/accord/AccordCommandStores.java index f82215919e..14accb0fb8 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordCommandStores.java +++ b/src/java/org/apache/cassandra/service/accord/AccordCommandStores.java @@ -55,7 +55,6 @@ import org.apache.cassandra.journal.Descriptor; import org.apache.cassandra.schema.TableId; import org.apache.cassandra.service.accord.AccordCommandStore.DurablyAppliedTo; import org.apache.cassandra.service.accord.AccordExecutor.AccordExecutorFactory; -import org.apache.cassandra.utils.FBUtilities; import static org.apache.cassandra.config.AccordConfig.QueueShardModel.THREAD_PER_SHARD; import static org.apache.cassandra.config.DatabaseDescriptor.getAccord; @@ -73,7 +72,6 @@ public class AccordCommandStores extends CommandStores implements CacheSize, Shu private final int mask; private long cacheSize, workingSetSize; - private int maxQueuedLoads, maxQueuedRangeLoads; private boolean shrinkingOn; AccordCommandStores(NodeCommandStoreService node, Agent agent, DataStore store, RandomSource random, @@ -87,12 +85,9 @@ public class AccordCommandStores extends CommandStores implements CacheSize, Shu cacheSize = DatabaseDescriptor.getAccordCacheSizeInMiB() << 20; workingSetSize = DatabaseDescriptor.getAccordWorkingSetSizeInMiB() << 20; - AccordConfig config = DatabaseDescriptor.getAccord(); - maxQueuedLoads = maxQueuedLoads(config); - maxQueuedRangeLoads = maxQueuedRangeLoads(config); shrinkingOn = DatabaseDescriptor.getAccordCacheShrinkingOn(); refreshCapacities(); - ScheduledExecutors.scheduledFastTasks.scheduleWithFixedDelay(() -> { + ScheduledExecutors.scheduledTasks.scheduleWithFixedDelay(() -> { for (AccordExecutor executor : executors) { executor.executeDirectlyWithLock(() -> { @@ -173,13 +168,6 @@ public class AccordCommandStores extends CommandStores implements CacheSize, Shu refreshCapacities(); } - public synchronized void setMaxQueuedLoads(int total, int range) - { - maxQueuedLoads = total; - maxQueuedRangeLoads = range; - refreshCapacities(); - } - public synchronized void setShrinking(boolean on) { shrinkingOn = on; @@ -213,14 +201,11 @@ public class AccordCommandStores extends CommandStores implements CacheSize, Shu { long capacityPerExecutor = cacheSize / executors.length; long workingSetPerExecutor = workingSetSize < 0 ? Long.MAX_VALUE : workingSetSize / executors.length; - int maxLoadsPerExecutor = Math.max(1, (maxQueuedLoads + executors.length - 1) / executors.length); - int maxRangeLoadsPerExecutor = Math.max(1, (maxQueuedRangeLoads + executors.length - 1) / executors.length); for (AccordExecutor executor : executors) { executor.executeDirectlyWithLock(() -> { executor.setCapacity(capacityPerExecutor); executor.setWorkingSetSize(workingSetPerExecutor); - executor.setMaxQueuedLoads(maxLoadsPerExecutor, maxRangeLoadsPerExecutor); executor.cacheExclusive().setShrinkingOn(shrinkingOn); }); } @@ -341,21 +326,4 @@ public class AccordCommandStores extends CommandStores implements CacheSize, Shu return Math.max(1, config.queue_shard_count.or(DatabaseDescriptor.getAvailableProcessors() / 8)); } } - - private static int threads(AccordConfig config) - { - return config.queue_thread_count.or(2 * FBUtilities.getAvailableProcessors()); - } - - public static int maxQueuedLoads(AccordConfig config) - { - return config.max_queued_loads.or(FBUtilities.getAvailableProcessors()); - } - - public static int maxQueuedRangeLoads(AccordConfig config) - { - return config.max_queued_range_loads.or(maxQueuedLoads(config) / 4); - } - - } diff --git a/src/java/org/apache/cassandra/service/accord/AccordExecutor.java b/src/java/org/apache/cassandra/service/accord/AccordExecutor.java index c600178513..67a4d815c9 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordExecutor.java +++ b/src/java/org/apache/cassandra/service/accord/AccordExecutor.java @@ -26,6 +26,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.LockSupport; @@ -46,6 +47,7 @@ import accord.api.RoutingKey; import accord.impl.AbstractAsyncExecutor; import accord.local.Command; import accord.local.ExecutionContext; +import accord.local.ExecutionContext.ExecutionSequence; import accord.local.cfk.CommandsForKey; import accord.messages.Accept; import accord.messages.Commit; @@ -96,17 +98,20 @@ import org.apache.cassandra.service.accord.AccordCacheEntry.UniqueSave; import org.apache.cassandra.service.accord.AccordExecutor.Task.ExclusiveGroup; import org.apache.cassandra.service.accord.AccordExecutor.Task.GlobalGroup; import org.apache.cassandra.service.accord.AccordExecutor.Task.GroupKind; +import org.apache.cassandra.service.accord.AccordExecutor.Task.State; import org.apache.cassandra.service.accord.debug.DebugExecution.DebugExclusiveExecutor; import org.apache.cassandra.service.accord.debug.DebugExecution.DebugExecutor; import org.apache.cassandra.service.accord.debug.DebugExecution.DebugTask; +import org.apache.cassandra.utils.Clock; import org.apache.cassandra.utils.Closeable; import org.apache.cassandra.utils.WithResources; import org.apache.cassandra.utils.concurrent.AsyncPromise; import org.apache.cassandra.utils.concurrent.Condition; import org.apache.cassandra.utils.concurrent.Future; +import static accord.local.ExecutionContext.ExecutionSequence.BY_PRIORITY; +import static accord.local.ExecutionContext.ExecutionSequence.BY_PRIORITY_ATOMIC; import static accord.primitives.Routable.Domain.Range; -import static accord.utils.Invariants.createIllegalState; import static org.apache.cassandra.config.AccordConfig.QueuePriorityModel.ORIG_HLC_FIFO; import static org.apache.cassandra.service.accord.AccordCache.CommandAdapter.COMMAND_ADAPTER; import static org.apache.cassandra.service.accord.AccordCache.CommandsForKeyAdapter.CFK_ADAPTER; @@ -131,14 +136,17 @@ import static org.apache.cassandra.service.accord.AccordExecutor.Task.GlobalGrou import static org.apache.cassandra.service.accord.AccordExecutor.Task.GlobalGroup.RANGE_SCAN; import static org.apache.cassandra.service.accord.AccordExecutor.Task.GlobalGroup.SAVE; import static org.apache.cassandra.service.accord.AccordExecutor.Task.MAX_TRANCHE; -import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.ASSIGNED; -import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.INITIALIZED; -import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.LOADING; +import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.CANCELLED; +import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.EXECUTED; +import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.FAILED_TO_LOAD; +import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.LOADING_OPTIONAL; +import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.LOADING_REQUIRED; import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.RUNNING; +import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.RUNNING_OR_EXECUTED; import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.SCANNING_RANGES; -import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.WAITING_TO_LOAD; +import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.WAITING_ON_OPTIONAL; +import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.WAITING_ON_REQUIRED; import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.WAITING_TO_RUN; -import static org.apache.cassandra.service.accord.AccordExecutor.Task.State.WAITING_TO_SCAN_RANGES; import static org.apache.cassandra.service.accord.debug.DebugExecution.DEBUG_EXECUTION; import static org.apache.cassandra.utils.Clock.Global.nanoTime; @@ -158,37 +166,39 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor= 0 && FLOW_WIDTH_SHIFT >= 0); switch (BALANCING_MODEL) { default: throw new UnhandledEnum(BALANCING_MODEL); case PRIORITY_ONLY: case BLENDED_PRIORITY_PHASE_FAIR: - case BLENDED_PRIORITY_PHASE_BUDGET_FAIR: - case PRIORITY_BUDGET: BALANCE_BY_POSITION = true; break; case PHASE_ONLY: case PHASE_FAIR: - case PHASE_BUDGET: - case PHASE_BUDGET_FAIR: BALANCE_BY_POSITION = false; } { + // TODO (required): pick default max loads/saves/range loads based on number of threads long global = COUNTER_MASKS, exclusive = COUNTER_MASKS; - global ^= (0x7f ^ 1) << RANGE_SCAN.ordinal(); + global ^= (0x7fL ^ 1) << (RANGE_SCAN.ordinal() * 8); if (config.queue_active_limits != null) { long[] limits = parseEnumParams(config.queue_active_limits, "queue_active_limits"); @@ -199,30 +209,6 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor 1) - { - mins ^= (min ^ 1) << (RANGE_SCAN.ordinal() * 8); // set the default RANGE_SCAN budget to 1 - mins ^= (min ^ 2) << (RANGE_LOAD.ordinal() * 8); // set the default RANGE_LOAD budget to 2 - } - global = selectByOverflowBits(setOverflowWhenLessEqual(limits[0], 0), mins, limits[0]); - } - if (limits[1] != 0) - exclusive = selectByOverflowBits(setOverflowWhenLessEqual(limits[1], 0), minCounterValue(limits[1], 0) * COUNTER_LOWBITS, limits[1]); - } - GLOBAL_QUEUE_BUDGETS = global; - EXCLUSIVE_QUEUE_BUDGETS = exclusive; - } } public static final ShardedDecayingHistograms HISTOGRAMS = new ShardedDecayingHistograms(); @@ -240,15 +226,14 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor> scanningRanges = new StandaloneTaskQueue<>(TinyEnumSet.encode(SCANNING_RANGES)); // never queried, just parked here while scanning - private final StandaloneTaskQueue> waitingToLoadRangeTxns = new StandaloneTaskQueue<>(TinyEnumSet.encode(WAITING_TO_LOAD)); - private final StandaloneTaskQueue> waitingToLoad = new StandaloneTaskQueue<>(TinyEnumSet.encode(WAITING_TO_SCAN_RANGES, SCANNING_RANGES, WAITING_TO_LOAD)); - private final StandaloneTaskQueue> loading = new StandaloneTaskQueue<>(LOADING); - private final RunnableTaskQueue runnable = new RunnableTaskQueue<>(); + final StandaloneTaskQueue> scanningRanges = new StandaloneTaskQueue<>(TinyEnumSet.encode(SCANNING_RANGES)); // never queried, just parked here while scanning + final StandaloneTaskQueue> loading = new StandaloneTaskQueue<>(TinyEnumSet.encode(LOADING_REQUIRED, LOADING_OPTIONAL)); + final StandaloneTaskQueue> waitingOnCacheQueues = new StandaloneTaskQueue<>(TinyEnumSet.encode(WAITING_ON_REQUIRED, WAITING_ON_OPTIONAL)); + final RunnableTaskQueue runnable = new RunnableTaskQueue<>(); private final Tranches tranches = new Tranches(this); @@ -610,7 +583,6 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor waitingForQuiescence; @@ -662,16 +634,17 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor= maxWorkingCapacityInBytes || !runnable.hasWaitingToRun()) + { + AccordSystemMetrics.metrics.pausedExecutorLoading.inc(); + hasPausedLoading = true; + runnable.stop(RANGE_SCAN.ordinal()); + runnable.stop(LOAD.ordinal()); + runnable.stop(RANGE_LOAD.ordinal()); } } @@ -808,142 +798,6 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor> queue = waitingToLoadRangeTxns.isEmpty() || activeRangeLoads >= maxQueuedRangeLoads ? waitingToLoad : waitingToLoadRangeTxns; - AccordTask next = queue.peek(); - if (next == null) - return; - - if (hasPausedLoading || cache.weightedSize() >= maxWorkingCapacityInBytes) - { - // we have too much in memory already, and we have work waiting to run, so let that complete before queueing more - if (!loading.isEmpty() || runnable.hasWaiting()) - { - AccordSystemMetrics.metrics.pausedExecutorLoading.inc(); - hasPausedLoading = true; - return; - } - } - - switch (next.state()) - { - default: - { - failExclusive(next, createIllegalState("Unexpected state: " + next.toDescription())); - break; - } - case WAITING_TO_SCAN_RANGES: - if (activeRangeLoads >= maxQueuedRangeLoads) - { - parkRangeLoad(next); - } - else - { - ++activeRangeLoads; - ++activeLoads; - next.rangeScanner().start(this); - updateQueue(next); - } - break; - - case WAITING_TO_LOAD: - while (true) - { - AccordCacheEntry load = next.peekWaitingToLoad(); - boolean isForRange = isForRange(next, load); - if (isForRange && activeRangeLoads >= maxQueuedRangeLoads) - { - parkRangeLoad(next); - continue outer; - } - - Invariants.require(load != null); - ++activeLoads; - if (isForRange) - ++activeRangeLoads; - - for (AccordTask task : cache.load(this, next, isForRange, load)) - { - if (task == next) continue; - if (task.onLoading(load)) - updateQueue(task); - } - Object prev = next.pollWaitingToLoad(); - Invariants.require(prev == load); - if (next.peekWaitingToLoad() == null) - break; - - Invariants.require(next.state() == WAITING_TO_LOAD, "Invalid state: %s", next); - if (activeLoads >= maxQueuedLoads) - return; - } - Invariants.require(next.state().compareTo(LOADING) >= 0, "Invalid state: %s", next); - updateQueue(next); - } - } - } - - private boolean isForRange(AccordTask task, AccordCacheEntry load) - { - boolean isForRangeTxn = task.isRange(); - if (!isForRangeTxn) - return false; - - for (AccordTask t : load.loadingOrWaiting().waiters()) - { - if (!t.isRange()) - return false; - } - return true; - } - - private void parkRangeLoad(AccordTask task) - { - if (task.queued() != waitingToLoadRangeTxns) - { - task.unqueue(); - waitingToLoadRangeTxns.enqueue(task); - } - } - - private void updateQueue(AccordTask task) - { - task.unqueueIfQueued(); - switch (task.state()) - { - default: throw new AssertionError("Unexpected state: " + task.toDescription()); - case WAITING_TO_SCAN_RANGES: - case WAITING_TO_LOAD: - waitingToLoad.enqueue(task); - break; - case SCANNING_RANGES: - scanningRanges.enqueue(task); - break; - case LOADING: - loading.enqueue(task); - break; - case WAITING_TO_RUN: - waitingToRun(task); - break; - } - } - - private void waitingToRun(AccordTask task) - { - task.onWaitingToRun(); - task.commandStore.exclusiveExecutor.enqueue(task); - } - - private void waitingToRun(Task task, @Nullable ExclusiveExecutor queue) - { - task.onWaitingToRun(); - if (queue == null) runnable.enqueue(task); - else queue.enqueue(task); - } - public ExclusiveExecutor executor() { return new ExclusiveExecutor(this); @@ -968,13 +822,14 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor Cancellable load(AccordTask parent, Boolean isForRange, AccordCacheEntry entry) + public LoadRunnable load(AccordTask parent, Boolean isForRange, AccordCacheEntry entry) { - return submitPlainExclusive(parent, newLoad(entry, isForRange)); + LoadRunnable load = newLoad(entry, isForRange); + return submitPlainExclusive(parent, load); } @Override - public Cancellable save(AccordCacheEntry entry, UniqueSave identity, Runnable save) + public Cancellable save(AccordCacheEntry entry, UniqueSave identity, Runnable save) { return submitPlainExclusive(null, new SaveRunnable(entry, identity, save)); } @@ -1003,15 +858,7 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor void submit(AccordTask operation) { - submit(AccordExecutor::submitExclusive, i -> i, operation); - } - - void submitExclusive(AccordTask task) - { - registerExclusive(task); - task.setupExclusive(); - updateQueue(task); - enqueueLoadsExclusive(); + submit((self, task) -> task.submitExclusive(self), i -> i, operation); } public void submitExclusive(Runnable runnable) @@ -1028,7 +875,10 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor T submitPlainExclusive(Task parent, T task) { Invariants.require(isOwningThread()); + task.setStateExclusive(WAITING_TO_RUN); if (parent == null) registerExclusive(task); else registerConsequenceExclusive(parent, task); - task.onWaitingToRun(); - runnable.enqueue(task); + task.onLoaded(); + runnable.enqueue(task, true); return task; } @@ -1052,10 +903,10 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor task) { - AccordTask.State state = task.state(); + State state = task.state(); switch (state) { default: throw new UnhandledEnum(state); case SCANNING_RANGES: - case LOADING: - case WAITING_TO_LOAD: - case WAITING_TO_SCAN_RANGES: + case LOADING_REQUIRED: + case LOADING_OPTIONAL: + case WAITING_ON_REQUIRED: + case WAITING_ON_OPTIONAL: case WAITING_TO_RUN: - task.unqueueIfQueued(); - try { task.cancelExclusive(); } - finally { completeTaskExclusive(task); } - break; - - case INITIALIZED: // TODO (expected): preferable to be able to cancel at this stage, even if unlikely to trigger at this phase - case ASSIGNED: + if (!task.hasIncrementalStarted()) + { + task.unqueueIfQueued(); + try { task.cancelExclusive(); } + finally { cleanupTaskExclusive(task, false); } + break; + } + case UNINITIALIZED: // TODO (expected): preferable to be able to cancel at this stage, even if unlikely to trigger at this phase case RUNNING: - case PERSISTING: - case FINISHED: + case INCOMPLETE: + case EXECUTED: case CANCELLED: - case FAILED: + case FAILED_TO_LOAD: // cannot safely cancel } } void onScannedRangesExclusive(AccordTask task, Throwable fail) { - --activeLoads; - --activeRangeLoads; // the task may have already been cancelled, in which case we don't need to fail it - if (!task.state().isExecuted()) - { - if (fail != null) - { - failExclusive(task, fail); - } - else - { - task.rangeScanner().scannedExclusive(); - updateQueue(task); - } - } - enqueueLoadsExclusive(); + if (task.state().isExecuted()) + return; + + if (fail != null) failExclusive(task, fail, FAILED_TO_LOAD); + else task.rangeScanner().scannedExclusive(); } - private void failExclusive(AccordTask task, Throwable fail) + private void failExclusive(AccordTask task, Throwable fail, State newState) { if (task.state().isExecuted()) return; - try { task.failExclusive(fail); } + try { task.failExclusive(fail, newState); } catch (Throwable t) { agent.onException(t); } finally { task.unqueueIfQueued(); - completeTaskExclusive(task); + cleanupTaskExclusive(task, false); } } - private void onSavedExclusive(AccordCacheEntry state, Object identity, Throwable fail) + private void onSavedExclusive(AccordCacheEntry state, Object identity, Throwable fail) { cache.saved(state, identity, fail); } - private void onLoadedExclusive(AccordCacheEntry loaded, V value, Throwable fail, boolean isForRange) + private void onLoadedExclusive(AccordCacheEntry loaded, V value, Throwable fail) { - --activeLoads; - if (isForRange) - --activeRangeLoads; + if (loaded.status() == EVICTED) + return; - if (loaded.status() != EVICTED) + try (ArrayBuffers.BufferList> tasks = loaded.drainWaitingToLoad()) { - try (ArrayBuffers.BufferList> tasks = loaded.loading().copyWaiters()) + if (fail != null) { - if (fail != null) - { - for (AccordTask task : tasks) - failExclusive(task, fail); - cache.failedToLoad(loaded); - } - else - { - cache.loaded(loaded, value); - for (AccordTask task : tasks) - { - if (task.onLoad(loaded)) - { - Invariants.require(task.queued() == loading); - task.unqueue(); - waitingToRun(task); - } - } - } + for (AccordTask task : tasks) + failExclusive(task, fail, FAILED_TO_LOAD); + cache.failedToLoad(loaded); + } + else + { + cache.loaded(loaded, value); + for (AccordTask task : tasks) + task.onLoadOneExclusive(loaded); } } - enqueueLoadsExclusive(); + maybePauseLoading(); } private Task inherit() @@ -1264,7 +1100,7 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor= 1, "Must permit at least one load"); - Invariants.requireArgument(range >= 1, "Must permit at least one range load"); - maxQueuedLoads = total; - maxQueuedRangeLoads = range; - } - @Override public long capacity() { @@ -1332,24 +1159,39 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor= 0; + return this.compareTo(EXECUTED) >= 0; } - boolean isComplete() + boolean hasStarted() { - return this.compareTo(FINISHED) >= 0; + return this.compareTo(RUNNING) >= 0; } static State forOrdinal(int ordinal) @@ -1384,27 +1231,25 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor next < prev ? prev + 1 : next); ExclusiveGroup group = ExclusiveGroup.OTHER; TxnId txnId = context.primaryTxnId(); if (txnId != null) @@ -1511,7 +1373,7 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor queued() + { + return queued; + } + + final void unqueueIfQueued() + { + if (queued != null) + { + queued.unqueue(this); + queued = null; + } + } + + final void unqueue(TaskQueue expected) + { + Invariants.require(queued == expected, "%s != %s", queued, expected); + queued.unqueue(this); + queued = null; + } + + final void unsetQueue(TaskQueue expected) + { + Invariants.require(queued == expected, "%s != %s", queued, expected); + queued = null; + } + + final void setQueue(TaskQueue queue) + { + Invariants.require(queued == null); + Invariants.require(isCompatible(queue)); + queued = queue; } final void onRunning() @@ -1609,71 +1562,31 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor describeState() + { + State state = state(); + if (state == RUNNING || state == EXECUTED) { - Task next = cur.next; - cur.next = prev; - prev = cur; - cur = next; + RunState runState = runState(); + if (runState == RunState.NONE) + return state; + return runState; } - return prev; - } - - public DebuggableTask debuggable() { return null; } - - abstract String toDescription(); - - abstract void submitExclusive(AccordExecutor owner); - - /** - * Prepare to run while holding the state cache lock - */ - abstract protected void preRunExclusive(); - - /** - * Run the command; the state cache lock may or may not be held depending on the executor implementation - */ - protected abstract void run(); - - /** - * Fail the command; the state cache lock may or may not be held depending on the executor implementation - */ - abstract protected void fail(Throwable fail); - - abstract protected boolean isNewWork(); - - /** - * Cleanup the command while holding the state cache lock - */ - protected void cleanupExclusive(AccordExecutor executor) - { - executor.unregisterExclusive(this); - cleanupAt = nanoTime(); - if (runningAt != 0) - { - if (waitingToRunAt == 0) - waitingToRunAt = runningAt; - executor.elapsedWaitingToRun.increment(runningAt - waitingToRunAt, runningAt); - executor.elapsedPreparingToRun.increment(waitingToRunAt - createdAt, runningAt); - executor.elapsedRunning.increment(cleanupAt - runningAt, cleanupAt); - executor.elapsed.increment(cleanupAt - createdAt, cleanupAt); - } - if (DEBUG_EXECUTION) DebugTask.get(this).onCompleted(executor.debug); - } - - void cancelExclusive(AccordExecutor owner) {} - - public final State state() - { return State.forOrdinal(stateOrdinal()); } @@ -1687,14 +1600,24 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor>> EXCLUSIVE_GROUP_SHIFT) & GROUP_MASK; } - @Nullable - final TaskQueue queued() - { - return queued; - } - - final void unqueueIfQueued() - { - if (queued != null) - unqueue(); - } - - final void unqueue() - { - Invariants.require(queued != null); - queued.unqueue(this); - queued = null; - } - - final void unsetQueue(TaskQueue queue) - { - Invariants.require(queued == queue); - queued = null; - } - - final void setQueue(TaskQueue queue) - { - Invariants.require(queued == null); - Invariants.require(isCompatible(queue)); - queued = queue; - } - private boolean isCompatible(TaskQueue queue) { int self = stateOrdinal(); return TinyEnumSet.contains(queue.states, self); } - private static int init(GlobalGroup global, ExclusiveGroup exclusive, State state) + final boolean isSync() { - return global.bits | exclusive.bits | state.ordinal(); + return 0 == (info & NONSYNC_BIT); } - final void setTranche(int tranche) + final boolean isNonSync() { - Invariants.require(tranche <= MAX_TRANCHE); - info = info | (tranche << TRANCHE_SHIFT) | HAS_TRANCHE_BIT; + return !isSync(); } - final void setInheritedTranche(int tranche) + final void setNonSyncExclusive() { - Invariants.require(tranche <= MAX_TRANCHE); - info = info | (tranche << TRANCHE_SHIFT) | HAS_TRANCHE_BIT | HAS_INHERITED_BIT; + info |= NONSYNC_BIT; + } + + final boolean isIncremental() + { + return 0 != (info & INCREMENTAL_MASK); + } + + final void setIncrementalExclusive() + { + info |= INCREMENTAL | NONSYNC_BIT; + } + + final boolean hasIncrementalStarted() + { + return (info & INCREMENTAL_MASK) >= INCREMENTAL_STARTED; + } + + final void setIncrementalStartedExclusive() + { + Invariants.require(isIncremental()); + if (!isIncrementalFinishing()) + info = (info & ~INCREMENTAL_MASK) | INCREMENTAL_STARTED; + } + + final boolean isIncrementalFinishing() + { + return (info & INCREMENTAL_MASK) >= INCREMENTAL_FINISHING; + } + + final void setIncrementalFinishingExclusive() + { + Invariants.require(isIncremental()); + info |= INCREMENTAL_FINISHING; + } + + final void setSequencedExclusive(ExecutionSequence sequence) + { + Invariants.require(isUnsequenced()); + info |= sequence.ordinal() << SEQUENCED_SHIFT; + } + + final boolean isUnsequenced() + { + return (info & SEQUENCED_MASK) == 0; + } + + final boolean isSequencedByPriority() + { + return (info & SEQUENCED_MASK) == SEQUENCED_PRIORITY; + } + + final boolean isSequencedByPriorityAtomic() + { + return (info & SEQUENCED_MASK) >= SEQUENCED_ATOMIC; + } + + final boolean isCacheQueuedFifo() + { + return (info & SEQUENCED_MASK) == SEQUENCED_ATOMIC_AND_QUEUED; + } + + final boolean isCacheQueued() + { + return 0 != (info & CACHE_QUEUED_BIT); + } + + // supersedes priority, in whichever order they're called + final void setCacheQueuedFifoExclusive() + { + Invariants.require(isSequencedByPriorityAtomic()); + info |= SEQUENCED_ATOMIC_AND_QUEUED | CACHE_QUEUED_BIT; + } + + final void setCacheQueuedExclusive() + { + info |= CACHE_QUEUED_BIT; } final int tranche() @@ -1789,10 +1760,51 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor>> TRANCHE_SHIFT; } + final void setTranche(int tranche) + { + Invariants.require(tranche <= MAX_TRANCHE); + info = info | (tranche << TRANCHE_SHIFT) | HAS_TRANCHE_BIT; + } + + final void setInheritedWithTranche(int tranche) + { + Invariants.require(tranche <= MAX_TRANCHE); + info = info | (tranche << TRANCHE_SHIFT) | HAS_TRANCHE_BIT | HAS_INHERITED_BIT; + } + final boolean hasInherited() { return (info & HAS_INHERITED_BIT) != 0; } + + final void setInheritedRangeScan() + { + info = info | HAS_INHERITED_RANGE_SCAN_BIT; + } + + final boolean hasInheritedRangeScan() + { + return (info & HAS_INHERITED_RANGE_SCAN_BIT) != 0; + } + + static int init(GlobalGroup global, ExclusiveGroup exclusive) + { + return (global.ordinal() << GLOBAL_GROUP_SHIFT) | (exclusive.ordinal() << EXCLUSIVE_GROUP_SHIFT); + } + + static Task reverse(Task unqueued) + { + Task prev = null; + Task cur = unqueued; + while (cur != null) + { + Task next = cur.next; + cur.next = prev; + prev = cur; + cur = next; + } + return prev; + } } // run the task even on a stopped commandStore @@ -1811,7 +1823,7 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor) task).executionContext())); + task.reportFailure(new RejectedExecutionException(commandStoreId + " is terminated. Cannot execute " + ((AccordTask) task).executionContext())); else task.run(); - // NOTE: cannot safely release owner here, in case an immediate-execution runs before we can release our references and store their changes to the cache + + // NOTE: we can ONLY safely release owner here due to AccordCacheEntry locking, which remains in place until AccordTask.releaseResourcesExclusive + // this also relies on AccordSafeCommandStore$ExclusiveCaches.acquireIfLoaded returning false when the entry is locked + owner = null; } private boolean reject(Task task) @@ -1928,45 +1943,49 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor extends TaskQueue @@ -2218,7 +2243,7 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor[] queues; final long[] positions; final byte groupShift; - final long baseBudget, limits; + final long limits; + /** sets overflow bits for a queue that has been stopped */ + long stopped; /** sets overflow bits for each counter when it needs its position updated */ long dirty; /** sets overflow bits for each counter when there's associated work */ long hasWork; /** Stores recent dequeue counts for up to 8 sub queues. */ long dispatches; + // TODO (required): increment arrivals based on internal queue for ExclusiveExecutors + // also: experiment with decaying on arrival schedule rather than poll schedule, since this should respond to work growth more accurately /** Stores recent enqueue counts for up to 8 sub queues. */ long arrivals; /** Stores currently-active counts for up to 8 sub queues. We can use this to impose limits on specific queues. */ long active; - /** Stores a biased budget for each task type, so that we may prefer to serve one type of task over another */ - long budget; /** deficit-round-robin credits for the two PRIORITY_FAIR strategies (flow/age). */ int creditFlow, creditAge; int waitingCount; - MultiTaskQueue(int waitingStates, GroupKind groups, long budget, long limits) + MultiTaskQueue(int waitingStates, GroupKind groups, long limits) { super(waitingStates); - this.baseBudget = budget; this.limits = limits; int queueCount = groups.count; Invariants.require(queueCount <= 8); @@ -2364,6 +2390,16 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor>> groupShift) & Task.GROUP_MASK; } + void stop(int group) + { + stopped |= overflowBit(group); + } + + void restart(int group) + { + stopped &= ~overflowBit(group); + } + final TaskQueue queue(Task task) { int group = group(task); @@ -2392,12 +2428,8 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor>>= 7; - int bitIndex = Long.numberOfTrailingZeros(visit); - return bitIndex / 8; - } - private int pollGroupByPhaseFair() { return minCounterIndex(recentFlowImbalances()); } - private int pollGroupByPhaseBudgetFair() - { - return minCounterIndex(recentFlowImbalances(), saturatedOrWithoutWorkOrWithoutBudget()); - } - // PRIORITY_FAIR selection: a deficit round-robin blend of two strategies, chosen per poll: // flow -> minCounterIndex(recent - arrivals + bias) (least fairly serviced) // age -> minGroupByPriority() (earliest-queued work) @@ -2516,11 +2526,6 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor queue = queues[group]; T head = queue.pollSingle(); @@ -2695,7 +2669,7 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor queue = queue(group); int result = queue.enqueueSingle(task); - incrementArrivals(group); + if (incrementArrivals) + incrementArrivals(group); if (result < 0) setHasWork(group); if (result != 0) setDirty(group); } @@ -2790,13 +2765,6 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor>> 7) & lowBit; } + final void incrementArrivals(Task task) + { + int group = group(task); + if (group >= 0) + incrementArrivals(group); + } + final void incrementArrivals(int group) { int shift = group * 8; @@ -2842,9 +2817,9 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor 0; + return unsaturatedWithWork() != 0; } final boolean isWaiting(T task) @@ -2870,13 +2845,13 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor extends MultiTaskQueue { - static final int RUNNABLE = TinyEnumSet.encode(WAITING_TO_RUN, ASSIGNED, RUNNING); + static final int RUNNABLE = TinyEnumSet.encode(WAITING_TO_RUN, RUNNING); final TaskQueue assigned; RunnableTaskQueue() { - super(RUNNABLE, GroupKind.GLOBAL, GLOBAL_QUEUE_BUDGETS, GLOBAL_QUEUE_LIMITS); + super(RUNNABLE, GroupKind.GLOBAL, GLOBAL_QUEUE_LIMITS); this.assigned = new TaskQueue<>(0); } @@ -2886,15 +2861,13 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor LoadRunnable newLoad(AccordCacheEntry entry, boolean isForRange) + LoadRunnable newLoad(AccordCacheEntry entry, boolean isForRange) { return new LoadRunnable<>(entry, isForRange ? RANGE_LOAD : LOAD); } - class LoadRunnable extends IOTask + public class LoadRunnable extends IOTask { - final AccordCacheEntry entry; + final AccordCacheEntry entry; Object result = FailureHolder.NOT_STARTED; - LoadRunnable(AccordCacheEntry entry, GlobalGroup group) + LoadRunnable(AccordCacheEntry entry, GlobalGroup group) { super(group); Invariants.require(group == LOAD || group == RANGE_LOAD); this.entry = entry; } - boolean isForRange() { return is(RANGE_LOAD); } - void postRunExclusive() { - if (!(result instanceof FailureHolder)) onLoadedExclusive(entry, (V)result, null, isForRange()); - else onLoadedExclusive(entry, null, ((FailureHolder)result).fail, isForRange()); + if (!(result instanceof FailureHolder)) onLoadedExclusive(entry, (V)result, null); + else onLoadedExclusive(entry, null, ((FailureHolder)result).fail); } @Override @@ -3194,7 +3163,7 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor entry; + final AccordCacheEntry entry; final UniqueSave identity; final Runnable run; Throwable failure = NOT_STARTED; - SaveRunnable(AccordCacheEntry entry, UniqueSave identity, Runnable run) + SaveRunnable(AccordCacheEntry entry, UniqueSave identity, Runnable run) { super(SAVE); this.entry = entry; @@ -3301,7 +3270,7 @@ public abstract class AccordExecutor implements CacheSize, LoadExecutor readyToRun = new ConcurrentLinkedQueue<>(); - private Task pendingSequentialHead, pendingSequentialTail; - private Task pendingCleanupHead, pendingCleanupTail; + private Task pendingExecutedHead, pendingExecutedTail; private Task pendingNewHead, pendingNewTail; private int pendingCount; @@ -105,14 +106,17 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop { updatePendingUnqueued(); Task requeue = null, requeueTail = null; + Task submit = null, submitTail = null; Task cur = pendingNewHead; while (cur != null) { Task next = cur.next; + cur.next = null; if (cur.isNewWork()) { - cur.next = null; - cur.submitExclusive(this); + if (submit == null) submit = cur; + else submitTail.next = cur; + submitTail = cur; --pendingCount; } else @@ -126,6 +130,13 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop pendingNewHead = requeue; pendingNewTail = requeueTail; + while (submit != null) + { + Task next = submit.next; + submit.next = null; + submit.submitExclusive(this); + submit = next; + } } private boolean enqueueOnePending() @@ -133,25 +144,23 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop if (pendingCount == 0) return false; - if (pendingSequentialHead != null) + --pendingCount; + if (pendingExecutedHead != null) { - pendingSequentialHead = enqueueOneCleanup(pendingSequentialHead); - if (pendingSequentialHead == null) - pendingSequentialTail = null; - } - else if (pendingCleanupHead != null) - { - pendingCleanupHead = enqueueOneCleanup(pendingCleanupHead); - if (pendingCleanupHead == null) - pendingCleanupTail = null; + Task executed = pendingExecutedHead; + pendingExecutedHead = destructiveNext(executed); + if (pendingExecutedHead == null) + pendingExecutedTail = null; + cleanupTaskExclusive(executed, true); } else { - pendingNewHead = enqueueOneSubmit(pendingNewHead); + Task submit = pendingNewHead; + pendingNewHead = destructiveNext(submit); if (pendingNewHead == null) pendingNewTail = null; + submit.submitExclusive(this); } - --pendingCount; return true; } @@ -178,22 +187,29 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop } boolean hasDrainedSignal = false; + int loops = 0; while (true) { long state = lock.state(); int signals = SignalLock.asyncSignalCount(state); int waiters = SignalLock.waitingEnabledThreadCount(state); - if (signals >= readyToRunTarget) + if (signals > 0) { - if (enqueueOnePending() || (updatePendingUnqueued() && enqueueOnePending())) continue; - else if (hasDrainedSignal) - lock.signalLockWorkExclusive(); - return; - } - else if (waiters > 0 && signals > 1 && SignalLock.activeEnabledThreadCount(state) == 1) - { - // ensure at least one other thread is running if there's enough work for it; it will spin up other threads if necessary - lock.propagateAsyncWorkSignals(1); + if (++loops > MAX_LOOPS) + return; + + if (signals >= readyToRunTarget) + { + if (enqueueOnePending() || (updatePendingUnqueued() && enqueueOnePending())) continue; + else if (hasDrainedSignal) + lock.signalLockWorkExclusive(); + return; + } + else if (waiters > 0 && signals > 1 && SignalLock.activeEnabledThreadCount(state) == 1) + { + // ensure at least one other thread is running if there's enough work for it; it will spin up other threads if necessary + lock.propagateAsyncWorkSignals(1); + } } Task task = pollAlreadyWaitingToRunExclusive(); @@ -216,9 +232,9 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop try { task.preRunExclusive(); } catch (Throwable t) { - try { task.fail(t); } + try { task.failExclusive(t, Task.State.FAILED_OTHER); } catch (Throwable t2) { try { t.addSuppressed(t2); } catch (Throwable t3) {} } - try { completeTaskExclusive(task); } + try { cleanupTaskExclusive(task, false); } catch (Throwable t2) { try { t.addSuppressed(t2); } catch (Throwable t3) {} } continue; } @@ -237,28 +253,22 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop return false; int count = 0; - Task addSequentialHead = null, addSequentialTail = null; - Task addCleanupHead = null, addCleanupTail = null; + Task addExecutedHead = null, addExecutedTail = null; Task addNewHead = null, addNewTail = null; { Task cur = Task.reverse(acquireUnqueuedExclusive()); while (cur != null) { Task next = cur.next; - if (!cur.isReadyToCleanup()) + if (cur.is(UNINITIALIZED)) { if (addNewHead == null) addNewHead = addNewTail = setNextNull(cur); else addNewHead = reverseOne(addNewHead, cur); } - else if (cur instanceof ExclusiveExecutorTask) - { - if (addSequentialHead == null) addSequentialHead = addSequentialTail = setNextNull(cur); - else addSequentialHead = reverseOne(addSequentialHead, cur); - } else { - if (addCleanupHead == null) addCleanupHead = addCleanupTail = setNextNull(cur); - else addCleanupHead = reverseOne(addCleanupHead, cur); + if (addExecutedHead == null) addExecutedHead = addExecutedTail = setNextNull(cur); + else addExecutedHead = reverseOne(addExecutedHead, cur); } ++count; cur = next; @@ -266,17 +276,11 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop } pendingCount += count; - if (addSequentialHead != null) + if (addExecutedHead != null) { - if (pendingSequentialHead == null) pendingSequentialHead = addSequentialHead; - else pendingSequentialTail.next = addSequentialHead; - pendingSequentialTail = addSequentialTail; - } - if (addCleanupHead != null) - { - if (pendingCleanupHead == null) pendingCleanupHead = addCleanupHead; - else pendingCleanupTail.next = addCleanupHead; - pendingCleanupTail = addCleanupTail; + if (pendingExecutedHead == null) pendingExecutedHead = addExecutedHead; + else pendingExecutedTail.next = addExecutedHead; + pendingExecutedTail = addExecutedTail; } if (addNewHead != null) { @@ -349,22 +353,22 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop } } - private Task cleanupAndMaybeGetWork(AccordTaskRunner self, @Nullable Task cleanup) + private Task executedAndMaybeGetWork(AccordTaskRunner self, @Nullable Task executed) { if (lock.tryAcquireAsyncWork()) { if (shutdown) throw new ShutdownException(); - return pushCleanupAndReturn(cleanup, nonNull(pollReadyToRun())); + return pushExecutedAndReturn(executed, nonNull(pollReadyToRun())); } if (!tryLock(self)) - return pushCleanupAndReturn(cleanup, null); + return pushExecutedAndReturn(executed, null); try { - completeTaskExclusive(cleanup); + cleanupTaskExclusive(executed, true); fetchWorkExclusive(); } catch (Throwable t) @@ -383,10 +387,9 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop return null; } - private Task pushCleanupAndReturn(Task cleanup, Task result) + private Task pushExecutedAndReturn(Task complete, Task result) { - cleanup.setReadyToCleanup(); - if (push(cleanup) == null) + if (push(complete) == null) lock.signalLockWork(); return result; } @@ -401,7 +404,7 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop final boolean unlockAndAcquire(AccordTaskRunner self) { - self.clearAccordLockedExecutor(); + self.exitAccordLockedExecutor(); if (DEBUG_EXECUTION) debug.onExitLock(); return lock.unlockAndAcquireAsyncWork(); } @@ -422,7 +425,7 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop { if (task != null) { - try { task = cleanupAndMaybeGetWork(self, task); } + try { task = executedAndMaybeGetWork(self, task); } catch (Throwable t) { task = null; throw t; } } if (task == null) @@ -435,7 +438,7 @@ public class AccordExecutorSignalLoop extends AccordExecutorAbstractLoop } catch (Throwable t) { - try { task.fail(t); } + try { task.failExecution(t); } catch (Throwable t2) { try diff --git a/src/java/org/apache/cassandra/service/accord/AccordExecutorSimple.java b/src/java/org/apache/cassandra/service/accord/AccordExecutorSimple.java index ee37744d8d..cadd7a766e 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordExecutorSimple.java +++ b/src/java/org/apache/cassandra/service/accord/AccordExecutorSimple.java @@ -82,7 +82,8 @@ class AccordExecutorSimple extends AccordExecutor protected void run() { - Thread self = Thread.currentThread(); + AccordTaskRunner self = AccordTaskRunner.get(); + self.setAccordActiveExecutor(AccordExecutorSimple.this); lock.lock(); try { @@ -95,11 +96,24 @@ class AccordExecutorSimple extends AccordExecutor return; } - try { task.preRunExclusive(); task.run(); } - catch (Throwable t) { task.fail(t); } + // TODO (expected): dedup with AbstractLockLoop, and cleanup (executed flag is a bit ugly) + self.setAccordActiveTask(task); + boolean executed = false; + try + { + task.preRunExclusive(); + executed = true; + task.run(); + } + catch (Throwable t) + { + executed = false; + task.failExecution(t); + } finally { - completeTaskExclusive(task); + cleanupTaskExclusive(task, executed); + self.setAccordActiveTask(null); } } } diff --git a/src/java/org/apache/cassandra/service/accord/AccordSafeCommand.java b/src/java/org/apache/cassandra/service/accord/AccordSafeCommand.java index 488ec61cb0..1263a3e1ec 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordSafeCommand.java +++ b/src/java/org/apache/cassandra/service/accord/AccordSafeCommand.java @@ -20,51 +20,22 @@ package org.apache.cassandra.service.accord; import java.util.Objects; -import com.google.common.annotations.VisibleForTesting; - import accord.api.Journal; import accord.local.Command; import accord.local.SafeCommand; import accord.primitives.TxnId; -import org.apache.cassandra.utils.concurrent.Ref; +import org.apache.cassandra.service.accord.AccordCacheEntry.LockMode; -public class AccordSafeCommand extends SafeCommand implements AccordSafeState +public class AccordSafeCommand extends SafeCommand implements AccordSafeState { - public static class DebugAccordSafeCommand extends AccordSafeCommand - { - final Ref selfRef; - public DebugAccordSafeCommand(AccordCacheEntry global) - { - super(global); - selfRef = new Ref<>(this, null); - selfRef.debug(global.key().toString()); - } - - @Override - public void markUnsafe() - { - super.markUnsafe(); - selfRef.release(); - } - - public static void trace(AccordSafeCommand safeCommand, String message) - { - ((DebugAccordSafeCommand)safeCommand).selfRef.debug(message); - } - } - - private boolean unsafe; - private final AccordCacheEntry global; + private final AccordCacheEntry global; private Command original; - private Command current; - public AccordSafeCommand(AccordCacheEntry global) + public AccordSafeCommand(AccordCacheEntry global) { super(global.key()); this.global = global; - this.original = null; - this.current = null; } @Override @@ -73,7 +44,7 @@ public class AccordSafeCommand extends SafeCommand implements AccordSafeState extends Task implements Function, Cancellable, DebuggableTask +public final class AccordTask extends Task implements Cancellable, DebuggableTask { private static final Logger logger = LoggerFactory.getLogger(AccordTask.class); private static final NoSpamLogger noSpamLogger = NoSpamLogger.getLogger(logger, 1, TimeUnit.MINUTES); - static class ForFunction extends AccordTask - { - private final Function function; - - public ForFunction(AccordCommandStore commandStore, ExecutionContext context, Function function) - { - super(commandStore, context); - this.function = function; - } - - @Override - public R apply(SafeCommandStore commandStore) - { - return function.apply(commandStore); - } - } - - // TODO (desired): these anonymous ops are somewhat tricky to debug. We may want to at least give them names. - static class ForConsumer extends AccordTask - { - private final Consumer consumer; - - private ForConsumer(AccordCommandStore commandStore, ExecutionContext context, Consumer consumer) - { - super(commandStore, context); - this.consumer = consumer; - } - - @Override - public Void apply(SafeCommandStore commandStore) - { - consumer.accept(commandStore); - return null; - } - } - - public static AccordTask create(CommandStore commandStore, ExecutionContext context, Function function) - { - return new ForFunction<>((AccordCommandStore) commandStore, context, function); - } - public static AccordTask create(CommandStore commandStore, ExecutionContext context, Consumer consumer) { - return new ForConsumer((AccordCommandStore) commandStore, context, consumer); + return new AccordTask<>((AccordCommandStore) commandStore, context, safeStore -> { + consumer.accept(safeStore); + return null; + }); + } + + public static AccordTask create(CommandStore commandStore, ExecutionContext context, Function function) + { + return new AccordTask<>((AccordCommandStore) commandStore, context, function); + } + + final class NonSyncState extends ExecutionContext.Wrapped implements ExecutionContext + { + RoutingKeys active; + ObjectHashSet notBlocking; + ObjectHashSet blocking; // cache entries on which we're blocking work + int loaded, processed; + int ready; + + public NonSyncState() + { + super(executionContext); + } + + @Override + public Unseekables keys() + { + return active; + } + + void addLoaded() + { + ++loaded; + } + + void onNotHead(AccordCacheEntry entry) + { + if ((notBlocking == null || !notBlocking.remove((RoutingKey) entry.key())) && blocking != null) + blocking.remove((RoutingKey) entry.key()); + } + + void onNewHead(AccordCacheEntry entry) + { + ensureNotBlocking().add((RoutingKey) entry.key()); + } + + void onNewBlockingHead(AccordCacheEntry entry) + { + ensureBlocking().add((RoutingKey) entry.key()); + } + + void onStillHeadNewBlocking(AccordCacheEntry entry) + { + notBlocking.remove((RoutingKey) entry.key()); + ensureBlocking().add((RoutingKey) entry.key()); + } + + private ObjectHashSet ensureBlocking() + { + if (blocking == null) + blocking = new ObjectHashSet<>(); + return blocking; + } + + private ObjectHashSet ensureNotBlocking() + { + if (notBlocking == null) + notBlocking = new ObjectHashSet<>(); + return notBlocking; + } + + private int readyCount() + { + return (blocking == null ? 0 : blocking.size()) + (notBlocking == null ? 0 : notBlocking.size()); + } + + boolean isLoaded() + { + return loaded >= Math.min(keys, NONSYNC_MIN_BATCH_SIZE); + } + + boolean isWaitReady() + { + if (readyCount() >= Math.min(keys - processed, NONSYNC_MIN_BATCH_SIZE)) + return true; + + return blocking != null && blocking.size() >= NONSYNC_BLOCKED_LIMIT; + } + + void preRunExclusive() + { + try (BufferList keys = new BufferList<>()) + { + if ((blocking == null || !populate(keys, blocking)) && notBlocking != null) + populate(keys, notBlocking); + + keys.forEach(key -> preExecute(refs.get(key), AccordTask.this, RELEASE_QUEUE)); + keys.sort(RoutingKey::compareTo); + active = RoutingKeys.of(keys); + } + processed += active.size(); + if (processed == keys && isIncremental()) + setIncrementalFinishingExclusive(); + } + + private boolean populate(List keys, ObjectHashSet from) + { + if (keys.size() + from.size() <= AccordExecutor.NONSYNC_MAX_BATCH_SIZE) + { + keys.addAll(from); + from.clear(); + return keys.size() == AccordExecutor.NONSYNC_MAX_BATCH_SIZE; + } + + Iterator iterator = from.iterator(); + while (iterator.hasNext()) + { + if (keys.size() == AccordExecutor.NONSYNC_MAX_BATCH_SIZE) + return true; + + RoutingKey key = iterator.next(); + keys.add(key); + iterator.remove(); + } + + return false; + } + + void postRunExclusive() + { + if (active != null) + { + for (RoutingKey key : active) + postExecute(refs.remove(key), AccordTask.this); + active = null; + } + ready = readyCount(); + } } final AccordCommandStore commandStore; private final ExecutionContext executionContext; - private volatile String loggingId; - private static final AtomicLong nextLoggingId = new AtomicLong(Clock.Global.currentTimeMillis()); - private static final AtomicReferenceFieldUpdater loggingIdUpdater = AtomicReferenceFieldUpdater.newUpdater(AccordTask.class, String.class, "loggingId"); + private final Function function; - // TODO (desired): merge all of these maps into one - @Nullable Object2ObjectHashMap commands; - @Nullable Object2ObjectHashMap commandsForKey; - @Nullable Object2ObjectHashMap> loading; + // TODO (expected): simple custom map that allows (at least): + // - efficient putIfAbsent + // - efficient small collections (2-4 entries) + // - forEach with parameters to avoid boxing lambdas + // - destructive forEach + // - forEach over specific SafeState types + Object2ObjectHashMap> refs = new Object2ObjectHashMap<>(); + + /** + * if is(LOADING), this is the number of cache entries we're waiting to complete loading before we can transition to WAITING_ON_CACHE_QUEUES; + * if is(WAITING_ON_CACHE_QUEUES), it's the number we're waiting to be head of before we can run + *

+ * if isNonSync(), this counts only txnId; otherwise it counts keys and txnId + */ + int waitingForState; + + /** + * Only set when isNonSync() + *

+ * if is(LOADING), this is the cache entries that have finished loading + * otherwise it's the cache entries for which we're at the head of the queue and are ready to run with + */ + @Nullable NonSyncState nonSync; + + int keys; // TODO (expected): not counting keys we add during execution LogLinearDecayingHistograms.Buffer histogramBuffer; - // TODO (desired): collection supporting faster deletes but still fast poll (e.g. some ordered collection) - @Nullable ArrayDeque> waitingToLoad; @Nullable RangeTxnScanner rangeScanner; @Nullable CommandSummaries commandsForRanges; + byte runState; private BiConsumer callback; - public AccordTask(@Nonnull AccordCommandStore commandStore, ExecutionContext executionContext) + public AccordTask(@Nonnull AccordCommandStore commandStore, ExecutionContext executionContext, Function function) { - super(executionContext); + super(executionContext, commandStore.executor().uniqueCreatedAt); this.commandStore = commandStore; this.executionContext = executionContext; - this.loggingId = "0x" + Long.toHexString(nextLoggingId.incrementAndGet()); - + this.function = function; if (logger.isTraceEnabled()) logger.trace("Created {} on {}", this, commandStore); } - private String loggingId() + String loggingId() { - String id = loggingId; - if (id == null) - { - id = "0x" + Long.toHexString(nextLoggingId.incrementAndGet()); - if (!loggingIdUpdater.compareAndSet(this, null, id)) - id = loggingId; - } - return id; + return executor().executorId + "/" + Long.toHexString(createdAt); } @Override public String toString() { - return executionContext.describe() + ' ' + toBriefString(); + return "@[" + commandStore.id() + ',' + commandStore.node().id() + "] " + executionContext.describe() + ' ' + toBriefString(); } public String toBriefString() { - return '{' + loggingId() + ',' + state() + '}'; + return '{' + loggingId() + ',' + describeState() + '}'; } public String toDescription() { return toBriefString() + ": " - + (queued() == null ? "unqueued" : state()) + + (queued() == null ? "unqueued" : describeState()) + ", primaryTxnId: " + executionContext.primaryTxnId() - + ", waitingToLoad: " + summarise(waitingToLoad) - + ", loading:" + summarise(loading, AccordSafeState::global) - + ", cfks:" + summarise(commandsForKey, AccordSafeState::global) - + ", txns:" + summarise(commands, AccordSafeState::global); + + ", state: " + summarise(refs, AccordSafeState::global); } @@ -252,11 +373,6 @@ public abstract class AccordTask extends Task implements Function keys() - { - return executionContext.keys(); - } - // TODO (expected): try to execute immediately BUT consider ordering requirements // esp. with deferred actions on e.g. CommandsForKey (not yet supported but also important for performance) public AsyncChain chain() @@ -267,7 +383,7 @@ public abstract class AccordTask extends Task implements Function callback) { preSetup(callback); - commandStore.executor().submit(AccordTask.this); + executor().submit(AccordTask.this); return AccordTask.this; } }; @@ -281,86 +397,147 @@ public abstract class AccordTask extends Task implements Function parent) + public void preSetup(AccordTask parent) { this.position = parent.position; + setInheritedWithTranche(parent.tranche()); + // note we use the caches "unsafely" here deliberately, as we only reference commands we already have references to // so we do not mutate anything, except the atomic counter of references - if (parent.commands != null) + + LoadKeys loadKeys = loadKeys(executionContext); + if (loadKeys != NONE) { - for (TxnId txnId : executionContext.txnIds()) - presetupExclusive(txnId, AccordTask::ensureCommands, parent.commands, commandStore.cachesUnsafe().commands()); + Unseekables parentKeysOrRanges = parent.executionContext.keys(); + Unseekables keysOrRanges = executionContext.keys(); + + boolean isKeySubset = parent.isIncremental() ? parent.nonSync.active.containsAll(keysOrRanges) : parentKeysOrRanges.containsAll(keysOrRanges); + if (isKeySubset) + setInheritedRangeScan(); + + setSequencedExclusive(executionContext.executionSequence()); + if (isSequencedByPriorityAtomic()) + { + boolean isTxnIdSubset = executionContext.isTxnIdSubsetOf(parent.executionContext); + Invariants.require(isKeySubset, "Must start ATOMIC tasks from a task declaring a superset of the required keys (for ASYNC/INCR tasks this means the keys active for the batch in question)"); + Invariants.require(isTxnIdSubset, "Must start ATOMIC tasks from a task declaring a superset of the required TxnIds"); + // TODO (required): we're appending to the fifo queue - does this maintain correct order? + setCacheQueuedFifoExclusive(); + } + + if (loadKeys != SYNC) + { + setNonSyncExclusive(); + nonSync = new NonSyncState(); + if (loadKeys == INCR) + { + setIncrementalExclusive(); + // forbid BY_PRIORITY sequencing to avoid priority inversion deadlocks on INCR tasks that lock a TxnId but await some key that has a higher priority task (that is waiting on our locked TxnId) - solvable in future if necessary + Invariants.require(isSequencedByPriorityAtomic() || isUnsequenced(), "INCR tasks may currently only be ATOMIC or UNSEQUENCED"); + } + } + + if (keysOrRanges.equals(parentKeysOrRanges)) + { + // TODO (desired): custom map we can more cheaply fork/copy + parent.refs.forEach((key, val) -> { + if (val instanceof AccordSafeCommandsForKey) + preSetup((RoutingKey) key, parent.refs, commandStore.cachesUnsafe().commandsForKeys()); + }); + } + else + { + switch (keysOrRanges.domain()) + { + case Key: + for (RoutingKey key : (AbstractUnseekableKeys) keysOrRanges) + preSetup(key, parent.refs, commandStore.cachesUnsafe().commandsForKeys()); + break; + + case Range: + AbstractRanges ranges = (AbstractRanges) keysOrRanges; + parent.refs.forEach((key, val) -> { + if (val instanceof AccordSafeCommandsForKey && ranges.contains((RoutingKey) key)) + preSetup((RoutingKey) key, parent.refs, commandStore.cachesUnsafe().commandsForKeys()); + }); + break; + } + } } - if (parent.commandsForKey == null) return; - if (executionContext.keys().domain() != Key) return; - switch (executionContext.loadKeys()) - { - default: throw new UnhandledEnum(executionContext.loadKeys()); - case NONE: - break; - - case ASYNC: - case INCR: - case SYNC: - for (RoutingKey key : (AbstractUnseekableKeys) executionContext.keys()) - presetupExclusive(key, AccordTask::ensureCommandsForKey, parent.commandsForKey, commandStore.cachesUnsafe().commandsForKeys()); - break; - } + for (TxnId txnId : executionContext.txnIds()) + preSetup(txnId, parent.refs, commandStore.cachesUnsafe().commands()); } @Override void submitExclusive(AccordExecutor owner) { - owner.submitExclusive(this); - } - - public void setupExclusive() - { + owner.registerExclusive(this); setupInternal(commandStore.cachesExclusive()); - setState(rangeScanner != null ? WAITING_TO_SCAN_RANGES - : waitingToLoad != null ? WAITING_TO_LOAD - : loading != null ? LOADING : WAITING_TO_RUN); } private void setupInternal(Caches caches) { + boolean hasPreSetup = hasInherited(); + LoadKeys loadKeys = loadKeys(executionContext); + if (loadKeys != NONE) { - boolean hasPreSetup = commands != null; - for (TxnId txnId : executionContext.txnIds()) + if (loadKeys != SYNC && !hasPreSetup) { - if (hasPreSetup && completePresetupExclusive(txnId, commands, caches.commands())) - continue; - setupExclusive(txnId, AccordTask::ensureCommands, caches.commands()); + setNonSyncExclusive(); + nonSync = new NonSyncState(); + if (loadKeys == INCR) + setIncrementalExclusive(); + } + + Unseekables keysOrRanges = executionContext.keys(); + switch (keysOrRanges.domain()) + { + case Range: + if (!hasInheritedRangeScan()) setupRangeLoadsExclusive(caches); + else refs.forEach((k, v) -> { + if (v instanceof AccordSafeCommandsForKey) + completePresetupExclusive((AccordSafeCommandsForKey)v); + }); + break; + case Key: + setupKeyLoadsExclusive(hasPreSetup, caches, (AbstractUnseekableKeys) keysOrRanges, hasInheritedRangeScan()); + break; } } - if (executionContext.keys().isEmpty()) + for (TxnId txnId : executionContext.txnIds()) + { + if (hasPreSetup && completePresetupExclusive(txnId)) + continue; + setupExclusive(txnId, caches.commands(), 1); + } + + if (is(SCANNING_RANGES)) return; - switch (executionContext.keys().domain()) - { - case Key: setupKeyLoadsExclusive(caches, (AbstractUnseekableKeys) executionContext.keys(), false); break; - case Range: setupRangeLoadsExclusive(caches); - } + onSetupOrScannedExclusive(); } - private void setupKeyLoadsExclusive(Caches caches, Iterable keys, boolean isToCompleteRangeScan) + private void setupKeyLoadsExclusive(boolean hasPreSetup, Caches caches, Iterable setupKeys, boolean doNotScanRanges) { - if (executionContext.loadKeys() == LoadKeys.NONE) + if (executionContext.loadKeys() == NONE) return; - if (!isToCompleteRangeScan && executionContext.loadKeysFor() == RECOVERY) + if (!doNotScanRanges && executionContext.loadKeysFor() == RECOVERY) { Invariants.require(rangeScanner == null); rangeScanner = new RangeTxnScanner(); + rangeScanner.start(); } - boolean hasPreSetup = commandsForKey != null; - for (RoutingKey key : keys) + int waitsForIncrement = isSync() ? 1 : 0; + for (RoutingKey setupKey : setupKeys) { - if (hasPreSetup && completePresetupExclusive(key, commandsForKey, caches.commandsForKeys())) continue; - setupExclusive(key, AccordTask::ensureCommandsForKey, caches.commandsForKeys()); + if (hasPreSetup && completePresetupExclusive(setupKey)) + continue; + + setupExclusive(setupKey, caches.commandsForKeys(), waitsForIncrement); } } @@ -369,123 +546,365 @@ public abstract class AccordTask extends Task implements Function> void presetupExclusive(K k, Function, Map> loaded, Map parentMap, AccordCache.Type.Instance cache) + private & AccordSafeState> void preSetup(K k, Map> parentMap, AccordCache.Type.Instance cache) { - AccordSafeState ref = parentMap.get(k); + S ref = (S) parentMap.get(k); if (ref == null) return; - AccordCacheEntry node = ref.global(); + AccordCacheEntry node = ref.global(); int refs = node.increment(); Invariants.require(refs > 1); - loaded.apply(this).put(k, cache.parent().adapter().safeRef(node)); + S safeState = cache.parent().adapter().safeRef(node); + this.refs.put(k, safeState); + if (cache.isCommandsForKey()) + keys++; } - // expects to hold lock - private > boolean completePresetupExclusive(K k, Map map, AccordCache.Type.Instance cache) + private & AccordSafeState> boolean completePresetupExclusive(K k) { - AccordSafeState preacquired = map.get(k); + S preacquired = (S) refs.get(k); if (preacquired != null) { - cache.recordPreAcquired(preacquired); + completePresetupExclusive(preacquired); return true; } return false; } + private & AccordSafeState> void completePresetupExclusive(S preacquired) + { + AccordCacheEntry entry = preacquired.global(); + if (entry.isLoaded()) completeSetupOfLoaded(entry); + else completeSetupOfLoading(entry, true); + } + // expects to hold lock - private > void setupExclusive(K k, Function, Map> loaded, AccordCache.Type.Instance cache) + private & AccordSafeState> void setupExclusive(K k, AccordCache.Type.Instance cache, int waitForIncrement) { S safeRef = cache.acquire(k); - Status entryStatus = safeRef.global().status(); - Map map; + AccordCacheEntry entry = safeRef.global(); + Status entryStatus = entry.status(); + boolean submitLoad = false; + boolean isLoaded; switch (entryStatus) { default: throw new UnhandledEnum(entryStatus); case WAITING_TO_LOAD: + submitLoad = true; case LOADING: - map = ensureLoading(); + isLoaded = false; + waitingForState += waitForIncrement; break; case WAITING_TO_SAVE: case SAVING: case LOADED: case MODIFIED: case FAILED_TO_SAVE: - map = loaded.apply(this); + isLoaded = true; } - Object prev = map.putIfAbsent(k, safeRef); + Object prev = refs.putIfAbsent(k, safeRef); if (prev != null) { - noSpamLogger.warn("PreLoadContext {} contained key {} more than once", map, k); + noSpamLogger.warn("ExecutionContext {} contained key {} more than once", refs, k); cache.release(safeRef, this); + waitingForState -= waitForIncrement; } - else if (map == loading) - { - if (entryStatus == Status.WAITING_TO_LOAD) - ensureWaitingToLoad().add(safeRef.global()); - safeRef.global().loadingOrWaiting().add(this); - Invariants.paranoid(safeRef.global().loadingOrWaiting().waiters().size() == safeRef.global().references()); - } - } - - // expects to hold lock - public boolean onLoad(AccordCacheEntry state) - { - AccordSafeState safeRef = loading == null ? null : loading.remove(state.key()); - Invariants.require(safeRef != null && safeRef.global() == state, "Expected to find %s loading; found %s", state, this, AccordTask::toDescription); - if (safeRef.getClass() == AccordSafeCommand.class) - ensureCommands().put((TxnId)state.key(), (AccordSafeCommand) safeRef); else - ensureCommandsForKey().put((RoutingKey) state.key(), (AccordSafeCommandsForKey) safeRef); + { + if (entry.isCommandsForKey()) + keys++; - if (!loading.isEmpty()) - return false; - - loading = null; - if (compareTo(WAITING_TO_LOAD) < 0) - return false; - - Invariants.require(waitingToLoad == null, "Invalid state: %s", this, AccordTask::toDescription); - setState(WAITING_TO_RUN); - return true; + if (isLoaded) completeSetupOfLoaded(entry); + else + { + if (submitLoad) executor().cacheUnsafe().load(executor(), this, is(ExclusiveGroup.RANGE), entry); + completeSetupOfLoading(entry, !submitLoad); + } + } } - // expects to hold lock - public boolean onLoading(AccordCacheEntry state) + private void completeSetupOfLoaded(AccordCacheEntry entry) { - boolean removed = waitingToLoad != null && waitingToLoad.remove(state); - Invariants.require(removed, "%s not found in waitingToLoad %s", state, this, AccordTask::toDescription); - if (!waitingToLoad.isEmpty()) - return false; - - return onEmptyWaitingToLoad(); + if (isOptional(entry)) + { + nonSync.addLoaded(); + if (isCacheQueuedFifo()) + addQueuedOptionalKey(entry, entry.addFifo(this)); + } + else if (isCacheQueuedFifo()) + { + entry.addFifo(this); + } } - private boolean onEmptyWaitingToLoad() + private void completeSetupOfLoading(AccordCacheEntry entry, boolean alreadyLoading) { - waitingToLoad = null; - if (compareTo(WAITING_TO_LOAD) < 0) - return false; + if (alreadyLoading) + { + Loading loading = entry.loading(); + if (loading.loading != null && loading.loading.is(RANGE_LOAD) && loading.loading.is(WAITING_TO_RUN) && !is(ExclusiveGroup.RANGE)) + { + // requeue anything setup as a range load that's now needed for a key-based operation, so it can use the correct the queue limits + loading.loading.unqueue(executor().runnable); + loading.loading.override(LOAD); + executor().runnable.enqueue(loading.loading, false); + } + } - setState(loading == null ? WAITING_TO_RUN : LOADING); - return true; + if (isCacheQueuedFifo()) entry.addFifo(this); + else entry.addWaitingToLoad(this); + Invariants.paranoid(entry.waitingCount() == entry.references()); + } + + private void onSetupOrScannedExclusive() + { + if (waitingForState > 0) + { + setStateExclusive(LOADING_REQUIRED); + executor().loading.enqueue(this); + } + else onLoadedRequiredExclusive(); + } + + private void onLoadedRequiredExclusive() + { + if (isSync() || nonSync.isLoaded()) + { + waitOnCacheQueuesExclusive(); + } + else + { + setStateExclusive(LOADING_OPTIONAL); + executor().loading.enqueue(this); + } + } + + boolean isUnsequenced(AccordCacheEntry entry) + { + return isUnsequenced() && (entry.isCommandsForKey() || !isIncremental()); + } + + boolean isOptional(AccordCacheEntry entry) + { + return isNonSync() && entry.isCommandsForKey(); + } + + boolean holdsLocksBetweenRuns() + { // TODO (desired): encode as a state bit + return isIncremental() && executionContext.primaryTxnId() != null; + } + + private void waitOnCacheQueuesExclusive() + { + Invariants.require(waitingForState == 0); + onLoaded(); + executor().runnable.incrementArrivals(this); + commandStore.exclusiveExecutor.incrementArrivals(this); + + this.refs.forEach((key, safeState) -> { + AccordCacheEntry entry = global(safeState); + boolean optional = isOptional(entry); + if (entry.isLoaded()) + { + RunnableStatus status = addToCacheQueue(entry, false); + if (optional) addQueuedOptionalKey(entry, status); + else if (status == NOT_RUNNABLE) + ++waitingForState; + } + else Invariants.require(optional); + }); + + // TODO (desired): exception-safe rollback for addUnsequenced + setCacheQueuedExclusive(); + if (waitingForState == 0) waitOnOptionalCacheQueuesExclusive(); + else + { + setStateExclusive(WAITING_ON_REQUIRED); + executor().waitingOnCacheQueues.enqueue(this); + } + } + + private void waitOnOptionalCacheQueuesExclusive() + { + if (isSync() || nonSync.isWaitReady()) waitToRunExclusive(); + else + { + setStateExclusive(WAITING_ON_OPTIONAL); + executor().waitingOnCacheQueues.enqueue(this); + } + } + + RunnableStatus addToCacheQueue(AccordCacheEntry loaded, boolean addIfFifo) + { + if (isCacheQueuedFifo()) return addIfFifo ? loaded.addFifo(this) : loaded.headStatus(this); + else if (isUnsequenced(loaded)) return loaded.addUnsequenced(this); + else return loaded.addPrioritised(this); + } + + void onLoadOneExclusive(AccordCacheEntry loaded) + { + if (isOptional(loaded)) + { + // if we're incremental/async we don't block on keys loading, so we don't need to decrement anything + // however, if we're in fifo mode this loaded key might be ready for us to run with + State state = state(); + switch (state) + { + default: throw new UnhandledEnum(state); + case WAITING_ON_REQUIRED: + case WAITING_ON_OPTIONAL: + case WAITING_TO_RUN: + case RUNNING: + RunnableStatus status = addToCacheQueue(loaded, false); + if (status != NOT_RUNNABLE) + addQueuedOptionalKey(loaded, status); + // fall-through + case LOADING_REQUIRED: + nonSync.addLoaded(); + break; + + case LOADING_OPTIONAL: + addLoadedOptionalKey(); + break; + } + } + else + { + if (--waitingForState == 0) + { + if (is(LOADING_REQUIRED)) + { + unqueue(executor().loading); + onLoadedRequiredExclusive(); + } + else Invariants.require(is(SCANNING_RANGES)); + } + } + } + + void addLoadedOptionalKey() + { + nonSync.addLoaded(); + if (is(LOADING_OPTIONAL) && nonSync.isLoaded()) + { + unqueue(executor().loading); + waitOnCacheQueuesExclusive(); + } + } + + // TODO (expected): add vs setup vs onChange; some callers don't need to try + void addQueuedOptionalKey(AccordCacheEntry loaded, RunnableStatus status) + { + switch (status) + { + default: throw UnhandledEnum.unknown(status); + case NOT_RUNNABLE: break; + case STILL_RUNNABLE: + case NEWLY_RUNNABLE: + nonSync.onNewHead(loaded); + break; + case STILL_RUNNABLE_NEWLY_BLOCKING: + case NEWLY_BLOCKING_RUNNABLE: + nonSync.onNewBlockingHead(loaded); + break; + } + + if (is(WAITING_ON_OPTIONAL) && nonSync.isWaitReady()) + { + unqueue(executor().waitingOnCacheQueues); + waitToRunExclusive(); + } + } + + void onChangeHeadStatus(AccordCacheEntry entry, RunnableStatus status) + { + if (isSync() || !entry.isCommandsForKey()) onChangeRequiredHeadStatus(entry, status); + if (isNonSync() && entry.isCommandsForKey()) onChangeOptionalHeadStatus(entry, status); + } + + private void incrementWaitingWhileAlreadyWaiting() + { + Invariants.require(isState(WAITING)); + if (waitingForState == 0) + { + if (is(WAITING_ON_OPTIONAL)) setStateExclusive(WAITING_ON_REQUIRED); + else + { + // TODO (expected): this is potentially costly; maybe we don't want to swap these in and out (but harder to maintain invariants) + unqueue(commandStore.exclusiveExecutor); + setStateExclusive(WAITING_ON_REQUIRED); + executor().waitingOnCacheQueues.enqueue(this); + } + } + Invariants.require(waitingForState < refs.size()); + ++waitingForState; + } + + void onChangeRequiredHeadStatus(AccordCacheEntry entry, RunnableStatus newStatus) + { + if (newStatus == NOT_RUNNABLE) + { + incrementWaitingWhileAlreadyWaiting(); + } + else if (newStatus != STILL_RUNNABLE_NEWLY_BLOCKING) + { + Invariants.require(is(WAITING_ON_REQUIRED)); + if (--waitingForState == 0) + { + unqueue(executor().waitingOnCacheQueues); + waitOnOptionalCacheQueuesExclusive(); + } + } + } + + void onChangeOptionalHeadStatus(AccordCacheEntry entry, RunnableStatus status) + { + Invariants.require(isState(WAITING_OR_RUNNING)); + switch (status) + { + default: throw UnhandledEnum.unknown(status); + case STILL_RUNNABLE: throw UnhandledEnum.invalid(STILL_RUNNABLE); // onChange -> changed (but this means no change) + case NOT_RUNNABLE: + nonSync.onNotHead(entry); + if (is(WAITING_TO_RUN) && !nonSync.isWaitReady()) + { + unqueue(commandStore.exclusiveExecutor); + setStateExclusive(WAITING_ON_OPTIONAL); + executor().waitingOnCacheQueues.enqueue(this); + } + return; + + case STILL_RUNNABLE_NEWLY_BLOCKING: + nonSync.onStillHeadNewBlocking(entry); + break; + + case NEWLY_RUNNABLE: + nonSync.onNewHead(entry); + break; + + case NEWLY_BLOCKING_RUNNABLE: + nonSync.onNewBlockingHead(entry); + break; + } + + if (is(WAITING_ON_OPTIONAL) && nonSync.isWaitReady()) + { + unqueue(executor().waitingOnCacheQueues); + waitToRunExclusive(); + } + } + + void waitToRunExclusive() + { + setStateExclusive(WAITING_TO_RUN); + commandStore.exclusiveExecutor.enqueue(this, false); } public ExecutionContext executionContext() @@ -493,70 +912,132 @@ public abstract class AccordTask extends Task implements Function commands() + @Override + protected void preRunExclusive() { - return commands; - } - - public Map ensureCommands() - { - if (commands == null) - commands = new Object2ObjectHashMap<>(); - return commands; - } - - public Map commandsForKey() - { - return commandsForKey; - } - - public Map ensureCommandsForKey() - { - if (commandsForKey == null) - commandsForKey = new Object2ObjectHashMap<>(); - return commandsForKey; - } - - private Map> ensureLoading() - { - if (loading == null) - loading = new Object2ObjectHashMap<>(); - return loading; - } - - private ArrayDeque> ensureWaitingToLoad() - { - Invariants.require(compareTo(WAITING_TO_LOAD) <= 0, "Expected status to be on or before WAITING_TO_LOAD; found %s", this, AccordTask::toDescription); - if (waitingToLoad == null) - waitingToLoad = new ArrayDeque<>(); - return waitingToLoad; - } - - public AccordCacheEntry pollWaitingToLoad() - { - Invariants.require(is(WAITING_TO_LOAD), "Expected status to be WAITING_TO_LOAD; found %s", this, AccordTask::toDescription); - if (waitingToLoad == null) - return null; - - AccordCacheEntry next = waitingToLoad.poll(); - if (waitingToLoad.isEmpty()) - onEmptyWaitingToLoad(); - return next; - } - - public AccordCacheEntry peekWaitingToLoad() - { - return waitingToLoad == null ? null : waitingToLoad.peek(); - } - - private void maybeSanityCheck(AccordSafeCommand safeCommand) - { - if (SANITY_CHECK) + super.preRunExclusive(); + if (rangeScanner != null) { - DebugTask debug = DebugTask.get(this); - if (debug.sanityCheck == null) - debug.sanityCheck = new ArrayList<>(commands.size()); - debug.sanityCheck.add(safeCommand.current()); + commandsForRanges = rangeScanner.finish(commandStore.cachesExclusive()); + rangeScanner = null; + } + + if (isSync()) + { + refs.forEach((k, v) -> { + preExecute(v, this, RELEASE_QUEUE); + }); + } + else + { + if (!hasIncrementalStarted()) + { + TxnId primaryTxnId = executionContext.primaryTxnId(); + if (primaryTxnId != null) + { + LockMode lockMode = holdsLocksBetweenRuns() ? HOLD_QUEUE : RELEASE_QUEUE; + preExecute(refs.get(primaryTxnId), this, lockMode); + TxnId additionalTxnId = executionContext.additionalTxnId(); + if (additionalTxnId != null) + preExecute(refs.get(additionalTxnId), this, lockMode); + } + + if (isIncremental() && isSequencedByPriorityAtomic() && !isCacheQueuedFifo()) + { + setCacheQueuedFifoExclusive(); + refs.forEach((key, safeState) -> { + AccordCacheEntry entry = global(safeState); + RunnableStatus status = entry.moveToFifo(this); + if (entry.isLoaded()) + { + switch (status) + { + default: throw UnhandledEnum.unknown(status); + case NOT_RUNNABLE: + case STILL_RUNNABLE: + case STILL_RUNNABLE_NEWLY_BLOCKING: + break; + case NEWLY_RUNNABLE: + nonSync.onNewHead(entry); + break; + case NEWLY_BLOCKING_RUNNABLE: + nonSync.onNewBlockingHead(entry); + break; + } + } + }); + } + + if (isIncremental()) + setIncrementalStartedExclusive(); + } + nonSync.preRunExclusive(); + } + } + + @Override + public void run() + { + onRunning(); + AccordSafeCommandStore safeStore = null; + try (Closeable close = resources.get()) + { + if (Tracing.isTracing()) + Tracing.trace(executionContext.describe()); + + commandStore.begin(safeStore = new AccordSafeCommandStore(this, isSync() ? executionContext : nonSync)); + + R result = function.apply(safeStore); + + boolean finished = !isIncremental() || isIncrementalFinishing(); + if (finished) + { + List changes = new ArrayList<>(); + // TODO (expected): save any TxnId we add so that we don't need to iterate all of refs + refs.forEach((key, value) -> { + if (value instanceof AccordSafeCommand) + { + AccordSafeCommand safeCommand = (AccordSafeCommand) value; + Journal.CommandUpdate diff = safeCommand.update(); + if (diff != null) + { + changes.add(diff); + maybeSanityCheck(safeCommand); + } + } + }); + + boolean flush = !changes.isEmpty() || safeStore.fieldUpdates() != null; + if (flush) + { + setRunState(PERSISTING); + Runnable onFlush = () -> finish(result, null); + safeStore.persistFieldUpdatesInternal(changes.isEmpty() ? onFlush : null); + if (!changes.isEmpty()) + save(changes, onFlush); + finished = false; + } + } + + // TODO (required): exception handling here needs improving + safeStore.postExecute(); + commandStore.complete(safeStore); + safeStore = null; + + if (finished) + finish(result, null); + } + catch (Throwable t) + { + if (safeStore != null) + refs.forEach((k, v) -> v.setAbandoned()); + throw t; + } + finally + { + if (safeStore != null) + commandStore.complete(safeStore); + onRunComplete(); } } @@ -579,147 +1060,71 @@ public abstract class AccordTask extends Task implements Function v.preExecute()); - if (commandsForKey != null) - commandsForKey.forEach((k, v) -> v.preExecute()); - } - - @Override - public void run() - { - onRunning(); - AccordSafeCommandStore safeStore = null; - try (Closeable close = resources.get()) - { - if (Tracing.isTracing()) - Tracing.trace(executionContext.describe()); - - setState(RUNNING); - - safeStore = commandStore.begin(this, commandsForRanges); - R result = apply(safeStore); - - List changes = null; - if (commands != null) - { - for (AccordSafeCommand safeCommand : commands.values()) - { - if (safeCommand.txnId().is(EphemeralRead)) - continue; - - Journal.CommandUpdate diff = safeCommand.update(); - if (diff == null) - continue; - - if (changes == null) - changes = new ArrayList<>(commands.size()); - changes.add(diff); - - maybeSanityCheck(safeCommand); - } - } - - boolean flush = changes != null || safeStore.fieldUpdates() != null; - if (flush) - { - setState(PERSISTING); - Runnable onFlush = () -> finish(result, null); - safeStore.persistFieldUpdatesInternal(changes == null ? onFlush : null); - if (changes != null) save(changes, onFlush); - } - - commandStore.complete(safeStore); - safeStore = null; - onRunComplete(); - if (!flush) - finish(result, null); - } - catch (Throwable t) - { - if (safeStore != null) - { - revert(); - commandStore.abort(safeStore); - } - throw t; + DebugTask debug = DebugTask.get(this); + if (debug.sanityCheck == null) + debug.sanityCheck = new ArrayList<>(2); + debug.sanityCheck.add(safeCommand.current()); } } - public void fail(Throwable throwable) + public void reportFailure(Throwable throwable) { - if (state().isComplete()) - return; - try - { - setState(FAILED); - commandStore.agent().onException(throwable); - } - finally { if (callback != null) callback.accept(null, throwable); } - } - - @Override - protected boolean isNewWork() - { - return true; - } - - public void failExclusive(Throwable throwable) - { - fail(throwable); - } - - @Override - protected void cleanupExclusive(AccordExecutor executor) - { - Invariants.expect(state().isExecuted()); - releaseResources(commandStore.cachesExclusive()); - super.cleanupExclusive(executor); - executor.keys.increment(commandsForKey == null ? 0 : commandsForKey.size(), runningAt); - if (histogramBuffer != null) + finally { - histogramBuffer.flush(cleanupAt); - histogramBuffer = null; + commandStore.agent().onException(throwable); } } - @Nullable - public RangeTxnScanner rangeScanner() + @Override + protected void cleanupExclusive(AccordExecutor executor, boolean executed) { - return rangeScanner; + if (is(RUNNING) && isNonSync()) + { + nonSync.postRunExclusive(); + if (isIncremental() && !isIncrementalFinishing()) + { + setStateExclusive(INCOMPLETE); + waitOnOptionalCacheQueuesExclusive(); + return; + } + } + + executor.keys.increment(keys, runningAt); + releaseResourcesExclusive(commandStore.cachesExclusive()); + super.cleanupExclusive(executor, executed); + if (histogramBuffer != null) + { + histogramBuffer.flush(completeAt); + histogramBuffer = null; + } } @Override public void cancel() { - if (!state().isComplete()) - commandStore.executor().cancel(this); + if (!state().hasStarted()) + executor().cancel(this); } void cancelExclusive() { logger.info("Cancelling {}", executionContext); - setState(CANCELLED); + setStateExclusive(CANCELLED); if (rangeScanner != null) rangeScanner.cancelled = true; if (callback != null) { - if (commandStore.executor().isInLoop()) callback.accept(null, new CancellationException()); - else commandStore.executor().submit(() -> callback.accept(null, new CancellationException())); + if (executor().isInLoop()) callback.accept(null, new CancellationException()); + else executor().submit(() -> callback.accept(null, new CancellationException())); } } @@ -730,113 +1135,54 @@ public abstract class AccordTask extends Task implements Function caches.commands().release(v, this)); - commands.clear(); - commands = null; - if (DEBUG_EXECUTION) DebugTask.get(this).onReleasedCommands(); - } - if (commandsForKey != null) - { - commandsForKey.forEach((k, v) -> caches.commandsForKeys().release(v, this)); - commandsForKey.clear(); - commandsForKey = null; - if (DEBUG_EXECUTION) DebugTask.get(this).onReleasedCommandsForKeys(); - } - if (waitingToLoad != null) - { - while (!waitingToLoad.isEmpty()) - waitingToLoad.poll().loadingOrWaiting().remove(this); - waitingToLoad = null; - } - if (loading != null) - { - loading.forEach((k, v) -> caches.global().release(v, this)); - loading.clear(); - loading = null; - } + + refs.forEach((key, safeState) -> { + AccordSafeState.postExecute(safeState, this); + }); + if (DEBUG_EXECUTION) DebugTask.get(this).onReleasedState(); } catch (Throwable t) { - releaseResourcesSlow(caches, t); + releaseResourcesSlowExclusive(t); commandStore.agent().onException(t); } + finally + { + refs = null; + } } - private void releaseResourcesSlow(Caches caches, Throwable suppressedBy) + private void releaseResourcesSlowExclusive(Throwable suppressedBy) { - if (commands != null) - { - safeRelease(commands, caches.commands(), suppressedBy); - commands.clear(); - commands = null; - } - if (commandsForKey != null) - { - safeRelease(commandsForKey, caches.commandsForKeys(), suppressedBy); - commandsForKey.clear(); - commandsForKey = null; - } - if (waitingToLoad != null) - { - while (!waitingToLoad.isEmpty()) + if (refs == null) + return; + + refs.forEach((k, safeState) -> { + if (!safeState.isReleased()) { - try { waitingToLoad.poll().loadingOrWaiting().remove(this); } + try { AccordSafeState.postExecute(safeState, this); } catch (Throwable t) { suppressedBy.addSuppressed(t); } } - waitingToLoad = null; - } - if (loading != null) - { - safeRelease(loading, caches.global(), suppressedBy); - loading.clear(); - loading = null; - } - } - - private void safeRelease(Map> map, AccordCache.Type.Instance cache, Throwable suppressedBy) - { - for (AccordSafeState safeState : map.values()) - { - if (safeState.isUnsafe()) continue; - try { cache.release(safeState, this); } - catch (Throwable t) { suppressedBy.addSuppressed(t); } - } - } - - private void safeRelease(Map> map, AccordCache cache, Throwable suppressedBy) - { - for (AccordSafeState safeState : map.values()) - { - if (safeState.isUnsafe()) continue; - try { cache.release(safeState, this); } - catch (Throwable t) { suppressedBy.addSuppressed(t); } - } - } - - void revert() - { - if (commands != null) - commands.forEach((k, v) -> v.revert()); - if (commandsForKey != null) - commandsForKey.forEach((k, v) -> v.revert()); + }); + refs = null; } public class RangeTxnAndKeyScanner extends RangeTxnScanner @@ -844,17 +1190,17 @@ public abstract class AccordTask extends Task implements Function { @Override - public void onUpdate(AccordCacheEntry state) + public void onUpdate(AccordCacheEntry state) { if (ranges.contains(state.key())) - reference(state); + reference((AccordCacheEntry) state); } } final Set intersectingKeys = new ObjectHashSet<>(); - final KeyWatcher keyWatcher = new KeyWatcher(); final Ranges ranges = ((AbstractRanges) executionContext.keys()).toRanges(); final AccordCache.Type.Instance commandsForKeyCache; + KeyWatcher keyWatcher = new KeyWatcher(); public RangeTxnAndKeyScanner(AccordCache.Type.Instance commandsForKeyCache) { @@ -877,11 +1223,8 @@ public abstract class AccordTask extends Task implements Function entry) + private void reference(AccordCacheEntry entry) { - if (loading != null && loading.containsKey(entry.key())) - return; - switch (entry.status()) { default: throw new AssertionError("Unhandled Status: " + entry.status()); @@ -894,7 +1237,7 @@ public abstract class AccordTask extends Task implements Function extends Task implements Function extends Task implements Function extends Task implements Function extends Task implements Function extends Task implements Function extends Task implements Function byId = new TreeMap<>(summaries); return (CommandSummaries.ByTxnIdSnapshot) () -> byId; } @@ -1061,4 +1418,29 @@ public abstract class AccordTask extends Task implements Function entry = caches.commands().getUnsafe(txnId); + AccordCacheEntry entry = caches.commands().getUnsafe(txnId); if (entry == null) { loadFromDisk.add(txnId); @@ -115,7 +115,6 @@ public class InMemoryRangeIndex extends InMemoryRangeSummaryIndex implements Ran public void finish(Map into) { - cleanupExclusive(null); owner.search(this, into::put, null); } diff --git a/src/java/org/apache/cassandra/service/accord/RangeIndex.java b/src/java/org/apache/cassandra/service/accord/RangeIndex.java index a31179a4eb..228aad3781 100644 --- a/src/java/org/apache/cassandra/service/accord/RangeIndex.java +++ b/src/java/org/apache/cassandra/service/accord/RangeIndex.java @@ -82,7 +82,7 @@ public interface RangeIndex return null; } - public CommandSummaries.Summary ifRelevant(AccordCacheEntry state) + public CommandSummaries.Summary ifRelevant(AccordCacheEntry state) { if (state.key().domain() != Routable.Domain.Range) return null; diff --git a/src/java/org/apache/cassandra/service/accord/debug/DebugBlockedTxns.java b/src/java/org/apache/cassandra/service/accord/debug/DebugBlockedTxns.java index 15cbd81ff2..e5ff023c80 100644 --- a/src/java/org/apache/cassandra/service/accord/debug/DebugBlockedTxns.java +++ b/src/java/org/apache/cassandra/service/accord/debug/DebugBlockedTxns.java @@ -178,7 +178,7 @@ public class DebugBlockedTxns private AsyncChain visitRootTxnAsync(CommandStore commandStore, TxnId txnId) { - return commandStore.chain(ExecutionContext.contextFor(txnId, "Populate txn_blocked_by"), safeStore -> { + return commandStore.chain(ExecutionContext.unsequenced(txnId, "Populate txn_blocked_by"), safeStore -> { Command command = safeStore.unsafeGetNoCleanup(txnId).current(); if (command == null || command.saveStatus() == SaveStatus.Uninitialised) return null; @@ -188,7 +188,7 @@ public class DebugBlockedTxns private AsyncChain visitTxnAsync(CommandStore commandStore, TxnId txnId, Timestamp rootExecuteAt, @Nullable TokenKey byKey, int depth, boolean recurse) { - return commandStore.chain(ExecutionContext.contextFor(txnId, "Populate txn_blocked_by"), safeStore -> { + return commandStore.chain(ExecutionContext.unsequenced(txnId, "Populate txn_blocked_by"), safeStore -> { Command command = safeStore.unsafeGetNoCleanup(txnId).current(); if (command == null || command.saveStatus() == SaveStatus.Uninitialised) return null; @@ -231,7 +231,7 @@ public class DebugBlockedTxns private AsyncChain visitKeysAsync(CommandStore commandStore, TokenKey key, Timestamp rootExecuteAt, int depth) { - return commandStore.chain(ExecutionContext.contextFor(RoutingKeys.of(key.toUnseekable()), SYNC, READ_WRITE, "Populate txn_blocked_by"), safeStore -> { + return commandStore.chain(ExecutionContext.unsequencedReadWrite(RoutingKeys.of(key.toUnseekable()), "Populate txn_blocked_by"), safeStore -> { visitKeysSync(safeStore, key, rootExecuteAt, depth); }); } diff --git a/src/java/org/apache/cassandra/service/accord/debug/DebugExecution.java b/src/java/org/apache/cassandra/service/accord/debug/DebugExecution.java index e01f877d14..037a09f1c4 100644 --- a/src/java/org/apache/cassandra/service/accord/debug/DebugExecution.java +++ b/src/java/org/apache/cassandra/service/accord/debug/DebugExecution.java @@ -40,10 +40,10 @@ public class DebugExecution { private static final Logger logger = LoggerFactory.getLogger(DebugExecution.class); public static final boolean DEBUG_EXECUTION = CassandraRelevantProperties.ACCORD_DEBUG_EXECUTION.getBoolean(false); - private static final long REPORT_MIN_LATENCY_MICROS = 20_000; + private static final long REPORT_MIN_LATENCY_MICROS = 50_000; private static final long REPORT_CPU_RATIO = 2; - private static final long REPORT_MAX_LATENCY_MICROS = 50_000; - private static final long REPORT_CPU_MICROS = 10_000; + private static final long REPORT_MAX_LATENCY_MICROS = 100_000; + private static final long REPORT_CPU_MICROS = 50_000; // TODO (expected): use sharded histogram so we can report global stats public static class DebugExecutor @@ -94,43 +94,16 @@ public class DebugExecution long lockedForCpuMicros = (unlockedAtCpu - lockedAtCpu)/1000; if (lockedForMicros >= REPORT_MAX_LATENCY_MICROS) { - report("Held lock for {}us (cpu:{}us)\n", lockedForMicros, lockedForCpuMicros); + report("Held lock for {}us (cpu:{}us)", lockedForMicros, lockedForCpuMicros); } else if (lockedForMicros >= REPORT_MIN_LATENCY_MICROS && (lockedForMicros / lockedForCpuMicros) >= REPORT_CPU_RATIO) { - report("Held lock for {}us with cpu time only {}us\n", lockedForMicros, lockedForCpuMicros); + report("Held lock for {}us with cpu time only {}us", lockedForMicros, lockedForCpuMicros); } locked.increment(lockedForMicros); } } - public static class DebugExecutorLoop - { - final DebugExecutor owner; - long lockAt; - - public DebugExecutorLoop(DebugExecutor owner) - { - this.owner = owner; - } - - public void onLock() - { - lockAt = nanoTime(); - } - - public void onEnterLock() - { - owner.onEnterLock(lockAt); - lockAt = 0; - } - - public void onExitLock() - { - owner.onExitLock(); - } - } - public static class DebugExclusiveExecutor { public static DebugExclusiveExecutor maybeDebug(DebugExecutor owner, int commandStoreId) @@ -200,7 +173,7 @@ public class DebugExecution public List sanityCheck; // for AccordTask only long polledAt, preRunAt, runCompleteAt, completedAt; - long releasedRangeScannerAt, releasedCommandsAt, releasedCommandsForKeyAt; + long releasedRangeScannerAt, releasedStateAt; long runningAtCpu, runCompleteAtCpu; Thread thread; @@ -231,14 +204,9 @@ public class DebugExecution releasedRangeScannerAt = nanoTime(); } - public void onReleasedCommands() + public void onReleasedState() { - releasedCommandsAt = nanoTime(); - } - - public void onReleasedCommandsForKeys() - { - releasedCommandsForKeyAt = nanoTime(); + releasedStateAt = nanoTime(); } public void onCompleted(DebugExecutor owner) @@ -254,9 +222,9 @@ public class DebugExecution runningMicros = (runCompleteAt - task.runningAt) / 1000; owner.running.increment(runningMicros); } - long runToCleanMicros = (task.cleanupAt - runCompleteAt)/1000; + long runToCleanMicros = (task.completeAt - runCompleteAt) / 1000; owner.runToCleanup.increment(runToCleanMicros); - long cleanupMicros = (completedAt - task.cleanupAt)/1000; + long cleanupMicros = (completedAt - task.completeAt) / 1000; owner.cleanup.increment(cleanupMicros); long totalMicros = (completedAt - polledAt)/1000; owner.taskTotal.increment(totalMicros); @@ -266,7 +234,7 @@ public class DebugExecution String reason = ""; if (totalMicros > REPORT_MAX_LATENCY_MICROS) reason += "LONG TIME "; if (totalCpu > REPORT_CPU_MICROS) reason += "HIGH CPU "; - if ((totalMicros > REPORT_MIN_LATENCY_MICROS && (totalMicros/totalCpu) >= REPORT_CPU_RATIO)) reason += "LOW RATIO "; + if ((totalMicros > REPORT_MIN_LATENCY_MICROS && (totalCpu == 0 || (totalMicros/totalCpu) >= REPORT_CPU_RATIO))) reason += "LOW RATIO "; report("{}{}: total {}us cpu:{}us ({}), pollToRun {}us, running {}us, runToClean {}us, cleanup {}us", reason, task, totalMicros, totalCpu, thread, pollToRunMicros, runningMicros, runToCleanMicros, cleanupMicros); } diff --git a/src/java/org/apache/cassandra/service/accord/debug/DebugTxnGraph.java b/src/java/org/apache/cassandra/service/accord/debug/DebugTxnGraph.java index a821db6340..7807eff9bc 100644 --- a/src/java/org/apache/cassandra/service/accord/debug/DebugTxnGraph.java +++ b/src/java/org/apache/cassandra/service/accord/debug/DebugTxnGraph.java @@ -221,7 +221,7 @@ public abstract class DebugTxnGraph private AsyncChain> submitRoot(CommandStore commandStore, TxnId txnId) { - return commandStore.chain(ExecutionContext.contextFor(txnId, "Populate txn_graph"), safeStore -> { + return commandStore.chain(ExecutionContext.unsequenced(txnId, "Populate txn_graph"), safeStore -> { Command command = safeStore.unsafeGetNoCleanup(txnId).current(); if (command == null || command.saveStatus() == SaveStatus.Uninitialised) return AsyncChains.>success(null); @@ -232,7 +232,7 @@ public abstract class DebugTxnGraph private AsyncChain> submitParent(CommandStore commandStore, TxnId txnId, P param, Map infos, Set visitedParent, int depth) { - return commandStore.chain(ExecutionContext.contextFor(txnId, "Populate txn_graph"), safeStore -> { + return commandStore.chain(ExecutionContext.unsequenced(txnId, "Populate txn_graph"), safeStore -> { Command command = safeStore.unsafeGetNoCleanup(txnId).current(); if (command == null || command.saveStatus() == SaveStatus.Uninitialised) return AsyncChains.>success(null); @@ -344,7 +344,7 @@ public abstract class DebugTxnGraph private AsyncChain populateTxnAsync(CommandStore commandStore, TxnId txnId, Map visited) { - return commandStore.chain(ExecutionContext.contextFor(txnId, "Populate txn_graph"), safeStore -> { + return commandStore.chain(ExecutionContext.unsequenced(txnId, "Populate txn_graph"), safeStore -> { Command command = safeStore.unsafeGetNoCleanup(txnId).current(); visited.putIfAbsent(txnId, command == null || command.saveStatus() == SaveStatus.Uninitialised ? SaveInfo.NONE : new SaveInfo(command.saveStatus(), command.executeAtIfKnown())); }); diff --git a/src/java/org/apache/cassandra/service/accord/journal/JournalRangeIndex.java b/src/java/org/apache/cassandra/service/accord/journal/JournalRangeIndex.java index eba2e534b4..c3be7c87a0 100644 --- a/src/java/org/apache/cassandra/service/accord/journal/JournalRangeIndex.java +++ b/src/java/org/apache/cassandra/service/accord/journal/JournalRangeIndex.java @@ -107,7 +107,7 @@ public class JournalRangeIndex extends SemiSyncIntervalTree implements } @Override - public void onUpdate(AccordCacheEntry state) + public void onUpdate(AccordCacheEntry state) { Summary summary = loader.ifRelevant(state); if (summary != null) @@ -162,7 +162,7 @@ public class JournalRangeIndex extends SemiSyncIntervalTree implements if (isMaybeRelevant(i)) { TxnId txnId = i.txnId; - AccordCacheEntry entry = c.getUnsafe(txnId); + AccordCacheEntry entry = c.getUnsafe(txnId); Invariants.expect(entry != null, "%s found interval %s but no matching transaction in cache", owner.commandStore, i); if (entry != null) { @@ -267,7 +267,7 @@ public class JournalRangeIndex extends SemiSyncIntervalTree implements } @Override - public void onUpdate(AccordCacheEntry state) + public void onUpdate(AccordCacheEntry state) { TxnId txnId = state.key(); if (txnId.is(Routable.Domain.Range)) @@ -334,7 +334,7 @@ public class JournalRangeIndex extends SemiSyncIntervalTree implements } @Override - public void onEvict(AccordCacheEntry state) + public void onEvict(AccordCacheEntry state) { TxnId txnId = state.key(); if (txnId.is(Routable.Domain.Range)) diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordDropTableBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordDropTableBase.java index 04db462483..edff67507a 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordDropTableBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordDropTableBase.java @@ -22,12 +22,12 @@ import com.google.common.base.Throwables; import org.assertj.core.api.Assertions; -import accord.api.RoutingKey; import accord.local.CommandStores; import accord.local.ExecutionContext; import accord.local.LoadKeys; import accord.local.Node; import accord.local.cfk.CommandsForKey; +import accord.local.cfk.SafeCommandsForKey; import accord.primitives.Ranges; import accord.primitives.Routable; import accord.primitives.Txn; @@ -43,7 +43,6 @@ import org.apache.cassandra.net.Verb; import org.apache.cassandra.schema.TableId; import org.apache.cassandra.service.accord.AccordCommandStore; import org.apache.cassandra.service.accord.AccordSafeCommandStore; -import org.apache.cassandra.service.accord.AccordSafeCommandsForKey; import org.apache.cassandra.service.accord.AccordService; import org.apache.cassandra.service.accord.TokenRange; @@ -135,19 +134,16 @@ public class AccordDropTableBase extends TestBaseImpl TableId tableId = TableId.fromString(s); AccordService accord = (AccordService) AccordService.instance(); TxnId syntheticTxnId = new TxnId(TxnId.MAX_EPOCH, 0, Txn.Kind.ExclusiveSyncPoint, Routable.Domain.Range, new Node.Id(1)); - ExecutionContext ctx = ExecutionContext.contextFor(syntheticTxnId, Ranges.single(TokenRange.fullRange(tableId, getPartitioner())), LoadKeys.SYNC, READ_WRITE, "Test"); + ExecutionContext ctx = ExecutionContext.unsequencedReadWrite(syntheticTxnId, Ranges.single(TokenRange.fullRange(tableId, getPartitioner())), "Test"); CommandStores stores = accord.node().commandStores(); for (int storeId : stores.ids()) { AccordCommandStore store = (AccordCommandStore) stores.forId(storeId); getBlocking(store.chain(ctx, input -> { AccordSafeCommandStore safe = (AccordSafeCommandStore) input; - for (RoutingKey key : safe.commandsForKeysKeys()) + for (SafeCommandsForKey safeCfk : safe.safeCommandsForKeys()) { - AccordSafeCommandsForKey safeCFK = (AccordSafeCommandsForKey) safe.ifLoadedAndInitialised(key); - if (safeCFK == null) // we read and found a key, but its null at load time... so ignore it - continue; - CommandsForKey cfk = safeCFK.current(); + CommandsForKey cfk = safeCfk.current(); CommandsForKey.TxnInfo minUndecided = cfk.minUndecidedManaged(); if (minUndecided != null) throw new AssertionError("Undecided txn: " + minUndecided); diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordLoadTest.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordLoadTest.java index 8270da184e..f7e52f108a 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordLoadTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordLoadTest.java @@ -355,6 +355,12 @@ public class AccordLoadTest extends AccordTestBase return builder.setArtificialLatencies(LATENCIES); } + private static SettingsBuilder populate(SettingsBuilder builder, int keyCount) + { + return builder.setKeySelector(roundrobin(keyCount)) + .setReadRatio(0f); + } + private static SettingsBuilder ycsbA(SettingsBuilder builder, int keyCount) { return builder.setKeySelector(ycsbZipfian(keyCount)) @@ -832,7 +838,7 @@ public class AccordLoadTest extends AccordTestBase if (storeId.get() >= 0) { CommandStore commandStore = service.node().commandStores().forId(storeId.get()); - List> result = AccordService.getBlocking(commandStore.submit(ExecutionContext.contextFor(candidate, "LoadTest"), safeStore -> { + List> result = AccordService.getBlocking(commandStore.submit(ExecutionContext.unsequenced(candidate, "LoadTest"), safeStore -> { SafeCommand safeCommand = safeStore.unsafeGet(candidate); PartialDeps deps = safeCommand.current().partialDeps(); if (deps == null) @@ -855,7 +861,7 @@ public class AccordLoadTest extends AccordTestBase for (List info : result) { TxnId txnId = TxnId.parse(info.get(0)); - AccordService.getBlocking(commandStore.execute(ExecutionContext.contextFor(txnId, "LoadTest"), safeStore -> { + AccordService.getBlocking(commandStore.execute(ExecutionContext.unsequenced(txnId, "LoadTest"), safeStore -> { SafeCommand safeCommand = safeStore.unsafeGet(txnId); if (safeCommand.current().executeAt != null) info.add(safeCommand.current().executeAt.toString()); @@ -915,7 +921,7 @@ public class AccordLoadTest extends AccordTestBase cluster.forEach(() -> { refresh(AccordExecutorMetrics.INSTANCE.elapsedRunning); refresh(AccordExecutorMetrics.INSTANCE.elapsed); - System.out.printf("%tT.%tL (%d %d %d %d %d %d)ms (%d %d %d %d %d %d)ms (%d %d %d %d %.0f, %d %d %d)us %d %d %d\n", nowMillis, nowMillis, + System.out.printf("%tT.%tL (%d %d %d %d %d %d)ms (%d %d %d %d %d %d)ms (%d %d %d)us %d %.0f (%d %d %d)us %d %d %d\n", nowMillis, nowMillis, getLatency(AccordCoordinatorMetrics.readMetrics.preacceptLatency, 0.5), getLatency(AccordCoordinatorMetrics.readMetrics.executeLatency, 0.5), getLatency(AccordCoordinatorMetrics.readMetrics.applyLatency, 0.5), @@ -1092,13 +1098,13 @@ public class AccordLoadTest extends AccordTestBase try { test.setup(); - test.testLoad(withArtificialLatencies(ycsbA(new SettingsBuilder(), 100_000) + test.testLoad(populate(new SettingsBuilder(), 1_000_000) // .setRatePerSecond(400).setMinRatePerSecond(200) // .setRatePerSecond(800).setMinRatePerSecond(200) .setRatePerSecond(1600).setMinRatePerSecond(200) .setIncreaseRatePerSecondInterval(5000) // .setTraceLast(5000) - ).build()); + .build()); } finally { diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/journal/AccordJournalConsistentExpungeTest.java b/test/distributed/org/apache/cassandra/distributed/test/accord/journal/AccordJournalConsistentExpungeTest.java index 8770df01c9..1b864aea1b 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/journal/AccordJournalConsistentExpungeTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/journal/AccordJournalConsistentExpungeTest.java @@ -38,6 +38,7 @@ import org.apache.cassandra.schema.Schema; import org.apache.cassandra.schema.TableId; import org.apache.cassandra.service.accord.AccordCacheEntry; import org.apache.cassandra.service.accord.AccordCommandStore; +import org.apache.cassandra.service.accord.AccordSafeCommand; import org.apache.cassandra.service.accord.AccordService; import org.apache.cassandra.service.accord.api.PartitionKey; import org.apache.cassandra.utils.ByteBufferUtil; @@ -86,7 +87,7 @@ public class AccordJournalConsistentExpungeTest extends TestBaseImpl Node node = service.node(); AccordCommandStore commandStore = (AccordCommandStore) node.commandStores().unsafeForKey(key.toUnseekable()); - Iterator> iterator = commandStore.cachesUnsafe().commands().iterator(); + Iterator> iterator = commandStore.cachesUnsafe().commands().iterator(); TxnId txnId = TxnId.NONE; diff --git a/test/simulator/test/org/apache/cassandra/simulator/test/AccordExecutorAndCacheTest.java b/test/simulator/test/org/apache/cassandra/simulator/test/AccordExecutorAndCacheTest.java new file mode 100644 index 0000000000..6bf1f46dfd --- /dev/null +++ b/test/simulator/test/org/apache/cassandra/simulator/test/AccordExecutorAndCacheTest.java @@ -0,0 +1,329 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.simulator.test; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; +import java.util.concurrent.Future; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.LockSupport; +import java.util.function.BooleanSupplier; +import java.util.function.Consumer; + +import javax.annotation.Nullable; + +import org.junit.Test; + +import accord.api.AsyncExecutor; +import accord.api.ExclusiveAsyncExecutor; +import accord.utils.async.Cancellable; + +import org.apache.cassandra.concurrent.ExecutorPlus; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.distributed.api.IIsolatedExecutor.SerializableSupplier; +import org.apache.cassandra.service.accord.AccordExecutor; +import org.apache.cassandra.service.accord.AccordExecutorAsyncSubmit; +import org.apache.cassandra.service.accord.AccordExecutorSignalLoop; +import org.apache.cassandra.service.accord.api.AccordAgent; +import org.apache.cassandra.utils.concurrent.AsyncPromise; +import org.apache.cassandra.utils.concurrent.SignalLock; + +import static org.apache.cassandra.concurrent.ExecutorFactory.Global.executorFactory; +import static org.apache.cassandra.service.accord.AccordExecutor.Mode.RUN_WITHOUT_LOCK; + +// TODO (required): have simulator intercept ReentantLock so we can test SyncSubmit and SemiSyncSubmit +public class AccordExecutorAndCacheTest extends SimulationTestBase +{ + @Test + public void signalLoopTest() + { + executorTest(() -> new AccordExecutorSignalLoop(1, RUN_WITHOUT_LOCK, SignalLock.MAX_THREADS, -1, -1, TimeUnit.MICROSECONDS, i -> "Loop" + i, new AccordAgent()), + 16); + } + + static class Submitted + { + final AtomicInteger nextId = new AtomicInteger(); + final AtomicInteger doneBefore = new AtomicInteger(); + final ConcurrentHashMap>> consequences = new ConcurrentHashMap<>(); + + boolean isDone() + { + return isDoneBetween(0, nextId.get()); + } + + boolean isDoneBefore(int before) + { + if (!isDoneBetween(doneBefore.get(), before)) + return false; + doneBefore.accumulateAndGet(before, Integer::max); + return true; + } + + boolean isDoneBetween(int from, int before) + { + for (int id = from ; id < before ; ++id) + { + for (Future future : consequences.get(id)) + { + if (!future.isDone()) + return false; + } + } + return true; + } + + Collection> start() + { + ConcurrentLinkedQueue> result = new ConcurrentLinkedQueue<>(); + + while (true) + { + int id = nextId.get(); + Object prev = consequences.putIfAbsent(id, result); + nextId.compareAndSet(id, id + 1); + if (prev == null) + return result; + } + } + } + + static class Control extends ConcurrentLinkedQueue + { + final Submitted submitted; + final AtomicInteger count = new AtomicInteger(); + final float cancelChance; + float processChance; + + Control(float cancelChance, Submitted submitted) + { + this(submitted, cancelChance, ThreadLocalRandom.current().nextFloat() * 0.5f); + } + + Control(Submitted submitted, float cancelChance, float processChance) + { + this.submitted = submitted; + this.cancelChance = cancelChance; + this.processChance = processChance; + } + + void submit(AsyncExecutor executor, Collection> consequences, Consumer>> run) + { + AsyncPromise future = new AsyncPromise<>(); + Cancellable cancel = executor.chain(() -> run.accept(consequences)).begin((success, fail) -> { + if (fail == null) future.trySuccess(null); + else + { + future.tryFailure(fail); + if (fail instanceof CancellationException) + run.accept(submitted.start()); + } + }); + consequences.add(future); + + if (cancel != null && ThreadLocalRandom.current().nextFloat() <= cancelChance) + { + add(cancel); + count.incrementAndGet(); + } + + if (count.get() > 0 && ThreadLocalRandom.current().nextFloat() <= processChance) + { + int cancelCount = 0; + do + { + ++cancelCount; + + float delta = ThreadLocalRandom.current().nextFloat() - 0.5f; + if (delta < 0) processChance /= delta; + else processChance *= -delta; + if (processChance < 0.001f || processChance > 0.999f) + processChance = cancelChance; + } while (count.decrementAndGet() > 0 && ThreadLocalRandom.current().nextFloat() <= processChance); + + // do outside of loop to avoid reentry + while (cancelCount-- > 0) + remove().cancel(); + } + } + } + + public void executorTest(SerializableSupplier supplier, int submissionThreads) + { + simulate(arr(() -> { + try + { + DatabaseDescriptor.daemonInitialization(); + ExecutorPlus submit = executorFactory().pooled("submit-test", submissionThreads); + AccordExecutor executor = supplier.get(); + Lock lock = executor.unsafeLock(); + ExclusiveAsyncExecutor sequentialExecutor = executor.newExclusiveExecutor(); + Executor lockExecutor = executorFactory().sequential("lock"); + + for (float sleepChance : new float[] { 0f, 0.01f, 0.1f }) + { + for (float lockChance : new float[] { 0f, 0.01f, 0.1f }) + { + for (float cancelChance : new float[] { 0f, 0.01f, 0.1f }) + { + System.out.println(String.format("sleepChance %.2f, lockChance %.2f, cancelChance %.2f", sleepChance, lockChance, cancelChance)); + List> done = new ArrayList<>(); + Submitted submitted = new Submitted(); + for (int i = 0; i < submissionThreads; ++i) + { + int id = i; + done.add(submit.submit(() -> { + try + { + submitLoop(id, lock, executor, sequentialExecutor, lockExecutor, 20, 10, sleepChance, lockChance, new Control(cancelChance, submitted), submitted); + } + catch (ExecutionException | InterruptedException e) + { + throw new RuntimeException(e); + } + })); + } + for (Future f : done) + f.get(); + + if (!submitted.isDone()) + throw new AssertionError(); + } + } + } + } + catch (Throwable t) + { + throw new RuntimeException(t); + } + }), + () -> {}, 1L); + } + + private static void submitLoop(int id, Lock lock, AccordExecutor executor, ExclusiveAsyncExecutor sequentialExecutor, Executor lockExecutor, int outerLoop, int innerLoop, float sleepChance, float lockChance, Control control, Submitted submitted) throws ExecutionException, InterruptedException + { + ConcurrentLinkedQueue> awaitConsequences = new ConcurrentLinkedQueue<>(); + while (outerLoop-- > 0) + { + List>> allAwaitSubmitted = new ArrayList<>(); + for (int i = 0; i < innerLoop; ++i) + { + Collection> awaitSubmitted = submitted.start(); + allAwaitSubmitted.add(awaitSubmitted); + submitRecursive(lock, executor, sequentialExecutor, 1 + i, awaitSubmitted, awaitConsequences, submitted, sleepChance, lockChance, control); + } + + AtomicBoolean done = new AtomicBoolean(); + submitUntil(lock, lockExecutor, sleepChance, done::get); + for (Collection> awaitSubmitted : allAwaitSubmitted) + await(awaitSubmitted, CancellationException.class); + await(awaitConsequences, null); + done.set(true); + System.out.println("Loop " + id + '.' + (1 + outerLoop)); + } + } + + private static void submitRecursive(Lock lock, AccordExecutor executor, ExclusiveAsyncExecutor sequentialExecutor, int count, Collection> consequences, Collection> awaitConsequences, Submitted submitted, float sleepChance, float lockChance, Control control) + { + AsyncExecutor submitTo = ThreadLocalRandom.current().nextBoolean() ? executor : sequentialExecutor; + + control.submit(submitTo, consequences, nextConsequences -> { + ThreadLocalRandom rnd = ThreadLocalRandom.current(); + boolean locked = false; + if (rnd.nextFloat() < lockChance) + { + if (rnd.nextBoolean()) locked = lock.tryLock(); + else { locked = true; lock.lock(); } + } + if (ThreadLocalRandom.current().nextFloat() < 0.01f) + { + int expectDoneBefore = submitted.nextId.get(); + AsyncPromise afterConsequences = new AsyncPromise<>(); + executor.afterSubmittedAndConsequences(() -> { + if (!submitted.isDoneBefore(expectDoneBefore)) + throw new AssertionError(); + afterConsequences.setSuccess(null); + }); + awaitConsequences.add(afterConsequences); + } + try + { + if (count > 1) + submitRecursive(lock, executor, sequentialExecutor, count -1, nextConsequences, awaitConsequences, submitted, sleepChance, lockChance, control); + if (rnd.nextFloat() < sleepChance) + LockSupport.parkNanos(rnd.nextInt(10000, 100000)); + } + finally + { + if (locked) + lock.unlock(); + } + }); + } + + private static void submitUntil(Lock lock, Executor executor, float sleepChance, BooleanSupplier done) + { + if (done.getAsBoolean()) + return; + + executor.execute(() -> { + + ThreadLocalRandom rnd = ThreadLocalRandom.current(); + boolean tryLock = rnd.nextBoolean(); + boolean locked = !tryLock; + if (tryLock) locked = lock.tryLock(); + else lock.lock(); + try + { + if (rnd.nextFloat() < sleepChance) + LockSupport.parkNanos(rnd.nextInt(10000, 100000)); + + submitUntil(lock, executor, sleepChance, done); + } + finally + { + if (locked) + lock.unlock(); + } + }); + } + + private static void await(Collection> await, @Nullable Class ignore) throws InterruptedException, ExecutionException + { + for (Future future : await) + { + try { future.get(); } + catch (ExecutionException e) + { + if (ignore == null || !(ignore.isInstance(e.getCause()))) + throw e; + } + } + } +} diff --git a/test/unit/org/apache/cassandra/db/compaction/CompactionAccordIteratorsTest.java b/test/unit/org/apache/cassandra/db/compaction/CompactionAccordIteratorsTest.java index 5d6408f676..69f3722bd0 100644 --- a/test/unit/org/apache/cassandra/db/compaction/CompactionAccordIteratorsTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/CompactionAccordIteratorsTest.java @@ -91,6 +91,7 @@ import org.apache.cassandra.service.accord.api.TokenKey; import org.apache.cassandra.utils.FBUtilities; import static accord.local.ExecutionContext.contextFor; +import static accord.local.ExecutionContext.unsequencedReadWrite; import static accord.local.LoadKeys.SYNC; import static accord.local.LoadKeysFor.READ_WRITE; import static accord.local.RedundantStatus.SomeStatus.GC_BEFORE_AND_LOCALLY_DURABLE; @@ -322,28 +323,28 @@ public class CompactionAccordIteratorsTest PartialDeps partialDeps = Deps.NONE.intersecting(AccordTestUtils.fullRange(txn)); PartialTxn partialTxn = txn.slice(commandStore.unsafeGetRangesForEpoch().currentRanges(), true); Route partialRoute = route.overlapping(commandStore.unsafeGetRangesForEpoch().currentRanges()); - getBlocking(commandStore.execute(contextFor(txnId, route, SYNC, READ_WRITE, "Test"), safe -> { + getBlocking(commandStore.execute(unsequencedReadWrite(txnId, route, "Test"), safe -> { CheckedCommands.preaccept(safe, txnId, partialTxn, route, (a, b) -> {}); })); flush(commandStore); - getBlocking(commandStore.execute(contextFor(txnId, route, SYNC, READ_WRITE, "Test"), safe -> { + getBlocking(commandStore.execute(unsequencedReadWrite(txnId, route, "Test"), safe -> { CheckedCommands.accept(safe, txnId, Ballot.ZERO, partialRoute, txnId, partialDeps, (a, b) -> {}); })); flush(commandStore); - getBlocking(commandStore.execute(contextFor(txnId, route, SYNC, READ_WRITE, "Test"), safe -> { + getBlocking(commandStore.execute(unsequencedReadWrite(txnId, route, "Test"), safe -> { CheckedCommands.commit(safe, SaveStatus.Stable, Ballot.ZERO, txnId, route, partialTxn, txnId, partialDeps, (a, b) -> {}); })); flush(commandStore); - getBlocking(commandStore.chain(contextFor(txnId, route, SYNC, READ_WRITE, "Test"), safe -> { + getBlocking(commandStore.chain(unsequencedReadWrite(txnId, route, "Test"), safe -> { return AccordTestUtils.processTxnResultDirect(safe, txnId, partialTxn, txnId); - }).flatMap(i -> i).flatMap(result -> commandStore.chain(contextFor(txnId, route, SYNC, READ_WRITE, "Test"), safe -> { + }).flatMap(i -> i).flatMap(result -> commandStore.chain(unsequencedReadWrite(txnId, route, "Test"), safe -> { CheckedCommands.apply(safe, txnId, route, txnId, partialDeps, partialTxn, result.left, result.right, (a, b) -> {}); }))); flush(commandStore); // The apply chain is asychronous, so it is easiest to just spin until it is applied // in order to have the updated state in the system table spinAssertEquals(true, 5, () -> { - return getBlocking(commandStore.submit(contextFor(txnId, route, SYNC, READ_WRITE, "Test"), safe -> { + return getBlocking(commandStore.submit(unsequencedReadWrite(txnId, route, "Test"), safe -> { StoreParticipants participants = StoreParticipants.all(route); Command command = safe.get(txnId, participants).current(); return command.hasBeen(Status.Applied); diff --git a/test/unit/org/apache/cassandra/service/accord/AccordCacheEntryTest.java b/test/unit/org/apache/cassandra/service/accord/AccordCacheEntryTest.java index a91c082039..4c174a37b7 100644 --- a/test/unit/org/apache/cassandra/service/accord/AccordCacheEntryTest.java +++ b/test/unit/org/apache/cassandra/service/accord/AccordCacheEntryTest.java @@ -20,14 +20,24 @@ package org.apache.cassandra.service.accord; import org.junit.Assert; import org.junit.Test; +import accord.local.SafeState; + import org.apache.cassandra.service.accord.AccordCache.Type; +import org.apache.cassandra.service.accord.AccordCacheEntry.LockMode; import org.apache.cassandra.service.accord.AccordCacheEntry.Status; public class AccordCacheEntryTest { - static class CacheEntry extends AccordCacheEntry + static class TestSafeState extends SafeState implements AccordSafeState { - public CacheEntry(String key, Type.Instance instance) + @Override public AccordCacheEntry global() { return null; } + @Override public void preExecute(AccordTask owner, LockMode lockMode) {} + @Override public void postExecute(AccordTask owner) {} + } + + static class CacheEntry extends AccordCacheEntry + { + public CacheEntry(String key, Type.Instance instance) { super(key, instance); } diff --git a/test/unit/org/apache/cassandra/service/accord/AccordCacheTest.java b/test/unit/org/apache/cassandra/service/accord/AccordCacheTest.java index 5cb919a66e..aedf100df0 100644 --- a/test/unit/org/apache/cassandra/service/accord/AccordCacheTest.java +++ b/test/unit/org/apache/cassandra/service/accord/AccordCacheTest.java @@ -23,13 +23,18 @@ import org.agrona.concurrent.NoOpLock; import org.junit.Assert; import org.junit.Test; +import accord.local.ExecutionContext; +import accord.local.SafeState; + import org.apache.cassandra.cache.CacheSize; import org.apache.cassandra.concurrent.ExecutorPlus; import org.apache.cassandra.concurrent.ManualExecutor; import org.apache.cassandra.metrics.AccordCacheMetrics; +import org.apache.cassandra.service.accord.AccordCacheEntry.LockMode; import org.apache.cassandra.service.accord.AccordCacheEntry.SaveExecutor; import org.apache.cassandra.service.accord.AccordCacheEntry.Status; +import static org.apache.cassandra.service.accord.AccordCacheEntry.LockMode.RELEASE_QUEUE; import static org.apache.cassandra.service.accord.AccordTestUtils.testLoad; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -40,85 +45,56 @@ public class AccordCacheTest { private static final long DEFAULT_NODE_SIZE = nodeSize(0); - private static abstract class TestSafeState implements AccordSafeState + private static abstract class TestSafeState & AccordSafeState> extends SafeState implements AccordSafeState { - protected boolean invalidated = false; - protected final AccordCacheEntry global; - private T original = null; + protected final AccordCacheEntry global; - public TestSafeState(AccordCacheEntry global) + public TestSafeState(AccordCacheEntry global) { this.global = global; } - public AccordCacheEntry global() + public AccordCacheEntry global() { return global; } - @Override - public T key() - { - return global.key(); - } + public final T key() { return global.key(); } - @Override - public T current() + public void preExecute(AccordTask owner, LockMode lockMode) { - return global.getExclusive(); - } - - @Override - public void set(T update) - { - global.setExclusive(update); - } - - @Override - public T original() - { - return original; - } - - @Override - public void preExecute() - { - original = global.getExclusive(); - } - - @Override - public Throwable failure() - { - return global.failure(); - } - - @Override - public void markUnsafe() - { - invalidated = true; - } - - @Override - public boolean isUnsafe() - { - return invalidated; + requireUninitialised(); + current = global.lockExclusive(owner, lockMode); + setSafe(); } } - private static class SafeString extends TestSafeState + private static class SafeString extends TestSafeState { - public SafeString(AccordCacheEntry global) + public SafeString(AccordCacheEntry global) { super(global); } + + @Override + public void postExecute(AccordTask owner) + { + global.releaseExclusive(this, owner); + } } - private static class SafeInt extends TestSafeState + private static class SafeInt extends TestSafeState { - public SafeInt(AccordCacheEntry global) + public SafeInt(AccordCacheEntry global) { super(global); } + + @Override + public void postExecute(AccordTask owner) + { + global.releaseExclusive(this, owner); + } } private static long emptyNodeSize() @@ -264,6 +240,7 @@ public class AccordCacheTest assertCacheMetrics(cacheMetrics, 0, 3, 3, 3); SafeString safeString = instance.acquire("1"); + safeString.preExecute(new AccordTask<>(null, (ExecutionContext.Empty)() -> "Test", null), RELEASE_QUEUE); Assert.assertEquals(Status.LOADED, safeString.global.status()); assertCacheState(cache, 1, 3, nodeSize(1) * 3); @@ -392,6 +369,7 @@ public class AccordCacheTest assertCacheState(cache, 1, 1, nodeSize(1)); SafeString safeString2 = instance.acquire("0"); + safeString2.preExecute(new AccordTask<>(null, (ExecutionContext.Empty)() -> "Test", null), RELEASE_QUEUE); Assert.assertEquals("0", safeString2.current()); Assert.assertEquals(Status.LOADED, safeString1.global.status()); Assert.assertEquals(2, instance.references("0", SafeString.class)); diff --git a/test/unit/org/apache/cassandra/service/accord/AccordCommandStoreTest.java b/test/unit/org/apache/cassandra/service/accord/AccordCommandStoreTest.java index fc691f60fc..a8bbec8fdf 100644 --- a/test/unit/org/apache/cassandra/service/accord/AccordCommandStoreTest.java +++ b/test/unit/org/apache/cassandra/service/accord/AccordCommandStoreTest.java @@ -18,8 +18,6 @@ package org.apache.cassandra.service.accord; -import java.util.NavigableMap; -import java.util.TreeMap; import java.util.concurrent.atomic.AtomicLong; import org.junit.Assert; @@ -73,6 +71,7 @@ import org.apache.cassandra.utils.Pair; import static accord.primitives.Status.Durability.AllQuorums; import static com.google.common.collect.Iterables.getOnlyElement; import static org.apache.cassandra.cql3.statements.schema.CreateTableStatement.parse; +import static org.apache.cassandra.service.accord.AccordCacheEntry.LockMode.RELEASE_QUEUE; import static org.apache.cassandra.service.accord.AccordService.getBlocking; import static org.apache.cassandra.service.accord.AccordTestUtils.Commands.preaccepted; import static org.apache.cassandra.service.accord.AccordTestUtils.ballot; @@ -140,6 +139,7 @@ public class AccordCommandStoreTest promised, executeAt, txn, dependencies, accepted, waitingOn, result.left, TxnDataResult.PERSISTABLE); AccordSafeCommand safeCommand = new AccordSafeCommand(loaded(txnId, null)); + safeCommand.preExecute(new AccordTask<>(null, ExecutionContext.unsequenced(txnId, "Test"), null), RELEASE_QUEUE); safeCommand.set(expected); // In practice we should never need to save it with the condition boolean set // Not sure why this test does that @@ -168,10 +168,10 @@ public class AccordCommandStoreTest Command command2 = preaccepted(txnId2, txn, timestamp(1, clock.incrementAndGet(), 1)); AccordSafeCommandsForKey cfk = new AccordSafeCommandsForKey(loaded(key, null)); - cfk.initialize(); + cfk.preExecute(new AccordTask<>(null, ExecutionContext.unsequenced(txnId1, "Test"), null), RELEASE_QUEUE); - cfk.set(cfk.current().update(new TestSafeCommandStore(ExecutionContext.contextFor(command1.txnId(), "Test")), command1).cfk()); - cfk.set(cfk.current().update(new TestSafeCommandStore(ExecutionContext.contextFor(command1.txnId(), "Test")), command2).cfk()); + cfk.set(cfk.current().update(new TestSafeCommandStore(ExecutionContext.unsequenced(command1.txnId(), "Test")), command1).cfk()); + cfk.set(cfk.current().update(new TestSafeCommandStore(ExecutionContext.unsequenced(command1.txnId(), "Test")), command2).cfk()); CommandsForKeyAccessor.systemTableUpdater(commandStore.id(), (TokenKey)cfk.key(), cfk.current(), null, commandStore.nextSystemTimestampMicros()).run(); logger.info("E: {}", cfk); @@ -180,11 +180,4 @@ public class AccordCommandStoreTest Assert.assertEquals(cfk.current(), actual); } - - private static > NavigableMap toNavigableMap(V safeState) - { - TreeMap map = new TreeMap<>(); - map.put(safeState.key(), safeState); - return map; - } } diff --git a/test/unit/org/apache/cassandra/service/accord/AccordCommandTest.java b/test/unit/org/apache/cassandra/service/accord/AccordCommandTest.java index 3509c2e506..189b08ed0d 100644 --- a/test/unit/org/apache/cassandra/service/accord/AccordCommandTest.java +++ b/test/unit/org/apache/cassandra/service/accord/AccordCommandTest.java @@ -174,7 +174,7 @@ public class AccordCommandTest Commit commit = Commit.SerializerSupport.create(txnId, route, 1, 1, Commit.Kind.StableWithTxnAndDeps, Ballot.ZERO, executeAt, partialTxn, deps, fullRoute); getBlocking(commandStore.execute(commit, commit::apply)); - getBlocking(commandStore.execute(ExecutionContext.contextFor(txnId, Keys.of(key).toParticipants(), LoadKeys.SYNC, READ_WRITE, "Test"), safeStore -> { + getBlocking(commandStore.execute(ExecutionContext.unsequencedReadWrite(txnId, Keys.of(key).toParticipants(), "Test"), safeStore -> { Command before = safeStore.ifInitialised(txnId).current(); Assert.assertEquals(commit.executeAt, before.executeAt()); Assert.assertTrue(before.hasBeen(Status.Committed)); diff --git a/test/unit/org/apache/cassandra/service/accord/AccordTaskTest.java b/test/unit/org/apache/cassandra/service/accord/AccordTaskTest.java index 54dee77c04..9ca2b11678 100644 --- a/test/unit/org/apache/cassandra/service/accord/AccordTaskTest.java +++ b/test/unit/org/apache/cassandra/service/accord/AccordTaskTest.java @@ -86,6 +86,7 @@ import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.concurrent.Condition; import static accord.local.ExecutionContext.contextFor; +import static accord.local.ExecutionContext.unsequencedReadWrite; import static accord.local.LoadKeys.SYNC; import static accord.local.LoadKeysFor.READ_WRITE; import static accord.utils.Property.qt; @@ -127,7 +128,7 @@ public class AccordTaskTest AccordCommandStore commandStore = createAccordCommandStore(clock::incrementAndGet, "ks", "tbl"); TxnId txnId = txnId(1, clock.incrementAndGet(), 1); - getBlocking(commandStore.execute(ExecutionContext.contextFor(txnId, "Test"), instance -> { + getBlocking(commandStore.execute(ExecutionContext.unsequenced(txnId, "Test"), instance -> { // TODO review: This change to `ifInitialized` was done in a lot of places and it doesn't preserve this property // I fixed this reference to point to `ifLoadedAndInitialised` and but didn't update other places Assert.assertNull(instance.ifInitialised(txnId)); @@ -141,7 +142,7 @@ public class AccordTaskTest AccordCommandStore commandStore = createAccordCommandStore(clock::incrementAndGet, "ks", "tbl"); TxnId txnId = txnId(1, clock.incrementAndGet(), 1); - getBlocking(commandStore.execute(ExecutionContext.contextFor(txnId, "Test"), safe -> { + getBlocking(commandStore.execute(ExecutionContext.unsequenced(txnId, "Test"), safe -> { StoreParticipants participants = StoreParticipants.empty(txnId); SafeCommand command = safe.get(txnId, participants); Assert.assertNotNull(command); @@ -198,7 +199,7 @@ public class AccordTaskTest route.overlapping(ranges); PartialDeps deps = PartialDeps.builder(ranges, true).build(); - Command command = getBlocking(commandStore.submit(contextFor(txnId, route, SYNC, READ_WRITE, "Test"), safe -> { + Command command = getBlocking(commandStore.submit(unsequencedReadWrite(txnId, route, "Test"), safe -> { CheckedCommands.preaccept(safe, txnId, partialTxn, route, appendDiffToLog(commandStore)); CheckedCommands.commit(safe, SaveStatus.Stable, Ballot.ZERO, txnId, route, partialTxn, executeAt, deps, appendDiffToLog(commandStore)); return safe.ifInitialised(txnId).current(); @@ -246,7 +247,7 @@ public class AccordTaskTest Route partialRoute = route.overlapping(ranges); PartialDeps deps = PartialDeps.builder(ranges, true).build(); - Command command = getBlocking(commandStore.submit(contextFor(txnId, route, SYNC, READ_WRITE, "Test"), safe -> { + Command command = getBlocking(commandStore.submit(unsequencedReadWrite(txnId, route, "Test"), safe -> { CheckedCommands.preaccept(safe, txnId, partialTxn, route, appendDiffToLog(commandStore)); CheckedCommands.accept(safe, txnId, Ballot.ZERO, partialRoute, executeAt, deps, appendDiffToLog(commandStore)); CheckedCommands.commit(safe, SaveStatus.Committed, Ballot.ZERO, txnId, route, partialTxn, executeAt, deps, appendDiffToLog(commandStore)); @@ -441,7 +442,7 @@ public class AccordTaskTest AssertionError error = null; for (T key : keys) { - AccordCacheEntry node = cache.getUnsafe(key); + AccordCacheEntry node = cache.getUnsafe(key); if (node == null) continue; try { @@ -476,7 +477,7 @@ public class AccordTaskTest { for (T key : keys) { - AccordCacheEntry node = cache.getUnsafe(key); + AccordCacheEntry node = cache.getUnsafe(key); if (node == null) continue; Awaitility.await("For node " + node.key() + " to complete") .atMost(Duration.ofMinutes(1)) diff --git a/test/unit/org/apache/cassandra/service/accord/AccordTestUtils.java b/test/unit/org/apache/cassandra/service/accord/AccordTestUtils.java index ae43676851..297d13a79d 100644 --- a/test/unit/org/apache/cassandra/service/accord/AccordTestUtils.java +++ b/test/unit/org/apache/cassandra/service/accord/AccordTestUtils.java @@ -57,6 +57,7 @@ import accord.local.Node; import accord.local.Node.Id; import accord.local.NodeCommandStoreService; import accord.local.SafeCommandStore; +import accord.local.SafeState; import accord.local.StoreParticipants; import accord.local.TimeService; import accord.local.durability.DurabilityService; @@ -82,7 +83,6 @@ import accord.topology.TopologyManager; import accord.utils.SortedArrays.SortedArrayList; import accord.utils.async.AsyncChain; import accord.utils.async.AsyncChains; -import accord.utils.async.Cancellable; import org.apache.cassandra.ServerTestUtils; import org.apache.cassandra.concurrent.ExecutorPlus; @@ -104,6 +104,7 @@ import org.apache.cassandra.schema.TableId; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.accord.AccordCacheEntry.LoadExecutor; +import org.apache.cassandra.service.accord.AccordExecutor.IOTask; import org.apache.cassandra.service.accord.api.AccordAgent; import org.apache.cassandra.service.accord.api.PartitionKey; import org.apache.cassandra.service.accord.journal.AccordJournal; @@ -114,7 +115,6 @@ import org.apache.cassandra.service.accord.txn.TxnQuery; import org.apache.cassandra.service.accord.txn.TxnRead; import org.apache.cassandra.utils.Pair; import org.apache.cassandra.utils.concurrent.Condition; -import org.apache.cassandra.utils.concurrent.Future; import org.apache.cassandra.utils.concurrent.UncheckedInterruptedException; import static accord.primitives.Routable.Domain.Key; @@ -123,6 +123,7 @@ import static accord.primitives.SaveStatus.PreAccepted; import static accord.primitives.Status.Durability.NotDurable; import static accord.primitives.Txn.Kind.Write; import static java.lang.String.format; +import static org.apache.cassandra.service.accord.AccordCacheEntry.LockMode.RELEASE_QUEUE; import static org.apache.cassandra.service.accord.AccordExecutor.Mode.RUN_WITH_LOCK; import static org.apache.cassandra.service.accord.AccordService.getBlocking; @@ -162,16 +163,16 @@ public class AccordTestUtils } } - public static AccordCacheEntry loaded(K key, V value) + public static & AccordSafeState> AccordCacheEntry loaded(K key, V value) { - AccordCacheEntry global = new AccordCacheEntry<>(key, null); + AccordCacheEntry global = new AccordCacheEntry<>(key, null); global.initialize(value); return global; } public static AccordSafeCommand safeCommand(Command command) { - AccordCacheEntry global = loaded(command.txnId(), command); + AccordCacheEntry global = loaded(command.txnId(), command); return new AccordSafeCommand(global); } @@ -188,9 +189,9 @@ public class AccordTestUtils return new LoadExecutor<>() { @Override - public Cancellable load(P1 p1, P2 p2, AccordCacheEntry entry) + public IOTask load(P1 p1, P2 p2, AccordCacheEntry entry) { - Future future = executor.submit(() -> { + executor.submit(() -> { V v; try { v = entry.owner.parent().adapter().load(entry.owner.commandStore, entry.key()); } catch (Throwable t) @@ -200,19 +201,19 @@ public class AccordTestUtils } entry.loaded(v); }); - return () -> future.cancel(true); + return null; } }; } - public static void testLoad(ManualExecutor executor, AccordSafeState safeState, V val) + public static & AccordSafeState> void testLoad(ManualExecutor executor, S safeState, V val) { Assert.assertEquals(AccordCacheEntry.Status.WAITING_TO_LOAD, safeState.global().status()); safeState.global().load(loadExecutor(executor), null, null); Assert.assertEquals(AccordCacheEntry.Status.LOADING, safeState.global().status()); executor.runOne(); Assert.assertEquals(AccordCacheEntry.Status.LOADED, safeState.global().status()); - safeState.preExecute(); + safeState.preExecute(new AccordTask<>(null, (ExecutionContext.Empty)() -> "Test", null), RELEASE_QUEUE); Assert.assertEquals(val, safeState.current()); } diff --git a/test/unit/org/apache/cassandra/service/accord/SimpleSimulatedAccordCommandStoreTest.java b/test/unit/org/apache/cassandra/service/accord/SimpleSimulatedAccordCommandStoreTest.java index fae6c912fa..f62a784f1e 100644 --- a/test/unit/org/apache/cassandra/service/accord/SimpleSimulatedAccordCommandStoreTest.java +++ b/test/unit/org/apache/cassandra/service/accord/SimpleSimulatedAccordCommandStoreTest.java @@ -41,7 +41,7 @@ public class SimpleSimulatedAccordCommandStoreTest extends SimulatedAccordComman for (int i = 0, examples = 100; i < examples; i++) { TxnId id = AccordGens.txnIds().next(rs); - instance.process(ExecutionContext.contextFor(id, "Test"), (safe) -> { + instance.process(ExecutionContext.unsequenced(id, "Test"), (safe) -> { var safeCommand = safe.get(id, StoreParticipants.empty(id)); var command = safeCommand.current(); Assertions.assertThat(command.saveStatus()).isEqualTo(SaveStatus.Uninitialised); diff --git a/test/unit/org/apache/cassandra/service/accord/SimulatedAccordTaskTest.java b/test/unit/org/apache/cassandra/service/accord/SimulatedAccordTaskTest.java index ce061f40bf..fcd7ccdde6 100644 --- a/test/unit/org/apache/cassandra/service/accord/SimulatedAccordTaskTest.java +++ b/test/unit/org/apache/cassandra/service/accord/SimulatedAccordTaskTest.java @@ -23,6 +23,7 @@ import java.util.concurrent.locks.LockSupport; import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.BooleanSupplier; +import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Supplier; @@ -180,7 +181,10 @@ public class SimulatedAccordTaskTest extends SimulatedAccordCommandStoreTestBase private static AccordTask operation(SimulatedAccordCommandStore instance, ExecutionContext ctx, Action action, BooleanSupplier delay) { - return new SimulatedOperation(instance.commandStore, ctx, action == Action.FAILURE ? SimulatedOperation.Action.FAILURE : SimulatedOperation.Action.SUCCESS); + + Function function = action == Action.FAILURE ? safeStore -> { throw new SimulatedFault("Operation failed for keys " + ctx.keys()); } + : safeStore -> null; + return AccordTask.create(instance.commandStore, ctx, function); } private static class Counter implements BiConsumer @@ -196,26 +200,6 @@ public class SimulatedAccordTaskTest extends SimulatedAccordCommandStoreTestBase } } - private static class SimulatedOperation extends AccordTask - { - enum Action { SUCCESS, FAILURE} - private final Action action; - - public SimulatedOperation(AccordCommandStore commandStore, ExecutionContext executionContext, Action action) - { - super(commandStore, executionContext); - this.action = action; - } - - @Override - public Void apply(SafeCommandStore safe) - { - if (action == Action.FAILURE) - throw new SimulatedFault("Operation failed for keys " + keys()); - return null; - } - } - private static class SimulatedLoadFunctionWrapper implements FunctionWrapper { final Supplier actions; diff --git a/test/unit/org/apache/cassandra/service/accord/serializers/CommandsForKeySerializerTest.java b/test/unit/org/apache/cassandra/service/accord/serializers/CommandsForKeySerializerTest.java index 9975fd6b2b..448583ac00 100644 --- a/test/unit/org/apache/cassandra/service/accord/serializers/CommandsForKeySerializerTest.java +++ b/test/unit/org/apache/cassandra/service/accord/serializers/CommandsForKeySerializerTest.java @@ -505,8 +505,8 @@ public class CommandsForKeySerializerTest { int next = source.nextInt(commands.size()); Command command = commands.get(next); - if (command.txnId.isSyncPoint()) cfk = cfk.registerUnmanaged(new TestSafeCommandStore(ExecutionContext.contextFor(command.txnId(), "Test")), new TestSafeCommand(command), REGISTER).cfk(); - else cfk = cfk.update(new TestSafeCommandStore(ExecutionContext.contextFor(command.txnId(), "Test")), command).cfk(); + if (command.txnId.isSyncPoint()) cfk = cfk.registerUnmanaged(new TestSafeCommandStore(ExecutionContext.unsequenced(command.txnId(), "Test")), new TestSafeCommand(command), REGISTER).cfk(); + else cfk = cfk.update(new TestSafeCommandStore(ExecutionContext.unsequenced(command.txnId(), "Test")), command).cfk(); commands.set(next, commands.get(commands.size() - 1)); commands.remove(commands.size() - 1); } @@ -547,33 +547,10 @@ public class CommandsForKeySerializerTest static class TestSafeCommand extends SafeCommand { - final Command command; TestSafeCommand(Command command) { super(command.txnId); - this.command = command; - } - - @Override - public Command current() - { - return command; - } - - @Override - public void markUnsafe() - { - } - - @Override - public boolean isUnsafe() - { - return false; - } - - @Override - protected void set(Command command) - { + current = command; } } @@ -704,7 +681,7 @@ public class CommandsForKeySerializerTest { public TestSafeCommandStore(ExecutionContext context) { - super(context, TestCommandStore.INSTANCE); + super(context); } @Override protected CommandStoreCaches tryGetCaches() { return null; } @@ -712,6 +689,7 @@ public class CommandsForKeySerializerTest @Override protected SafeCommandsForKey add(SafeCommandsForKey safeCfk, CommandStoreCaches caches) { return null; } @Override protected SafeCommand getInternal(TxnId txnId) { return null; } @Override protected SafeCommandsForKey getInternal(RoutingKey key) { return null; } + @Override public CommandStore commandStore() { return TestCommandStore.INSTANCE; } @Override public DataStore dataStore() { return null; } @Override public Agent agent() { return null; } @Override public ProgressLog progressLog() { return null; }