ifesdjeen commented on code in PR #162:
URL: https://github.com/apache/cassandra-accord/pull/162#discussion_r1934505914
##########
accord-core/src/main/java/accord/primitives/Timestamp.java:
##########
@@ -277,6 +277,18 @@ public int compareTo(@Nonnull Timestamp that)
return c;
}
+ public boolean isAtLeastByEpochAndHlc(@Nonnull Timestamp that)
+ {
+ if (this == that) return true;
+ int cmpEpoch = Long.compare(this.epoch(), that.epoch());
+ int cmpHlc = Long.compare(this.hlc(), that.hlc());
+ if (cmpEpoch < 0 || cmpHlc < 0) return false;
+ if (cmpEpoch > 0 || cmpHlc > 0) return true;
Review Comment:
Can we be in a situation where epoch is ahead and HLC is behind?
##########
accord-core/src/main/java/accord/messages/RecoverAwait.java:
##########
@@ -18,24 +18,31 @@
package accord.messages;
+import java.util.EnumMap;
+
import accord.api.ProgressLog;
+import accord.coordinate.Recover;
+import accord.coordinate.Recover.InferredFastPath;
import accord.local.Command;
import accord.local.Node;
import accord.local.SafeCommand;
import accord.local.SafeCommandStore;
+import accord.primitives.Ballot;
import accord.primitives.Known.KnownDeps;
import accord.primitives.Participants;
import accord.primitives.Txn;
import accord.primitives.TxnId;
import accord.topology.Topologies;
+import static accord.api.ProgressLog.BlockedUntil.CommittedOrNotFastPathCommit;
import static accord.primitives.Routables.Slice.Minimal;
import static accord.primitives.Timestamp.Flag.HLC_BOUND;
public class RecoverAwait extends Await
{
final TxnId recoverId;
private transient boolean rejects;
+ private transient boolean cannotAccept;
Review Comment:
Nit: unused constructor on L47
##########
accord-core/src/main/java/accord/impl/progresslog/DefaultProgressLog.java:
##########
@@ -301,23 +303,40 @@ public List<TxnId> activeBefore(TxnId before)
}
@Override
- public void clearBefore(TxnId clearWaitingBefore, TxnId clearAnyBefore)
+ public void clearBefore(SafeCommandStore safeStore, TxnId
clearWaitingBefore, TxnId clearAllBefore)
{
+ if (clearAllBefore.compareTo(clearWaitingBefore) >= 0)
+ clearWaitingBefore = clearAllBefore;
+
int index = 0;
while (index < BTree.size(stateMap))
{
TxnState state = BTree.findByIndex(stateMap, index);
- if (state.txnId.compareTo(clearAnyBefore) < 0)
+ if (state.txnId.compareTo(clearWaitingBefore) >= 0)
+ return;
+
+ boolean notify = state.waiting().blockedUntil() != NotBlocked
+ && state.waiting().homeSatisfies() == NotBlocked;
+
+ if (notify)
+ {
+ // the command might be invalidated, which should be
established on load, so simply load the command
+ TxnId txnId = state.txnId;
+
safeStore.commandStore().execute(PreLoadContext.contextFor(txnId), safeStore0
-> {
Review Comment:
Maybe a silly optimization, and definitely outside scope of this patch, but
should we maybe make txnid implement preloadcontext for itself? This seems like
a rather common pattern.
##########
accord-core/src/main/java/accord/coordinate/FetchData.java:
##########
@@ -79,6 +79,18 @@ private static class FetchRequest
final BiConsumer<? super FetchResult, Throwable> callback;
public FetchRequest(Known fetch, TxnId txnId, InvalidIf invalidIf,
@Nullable Timestamp executeAt, Participants<?> fetchKeys, EpochSupplier
lowEpoch, EpochSupplier highEpoch, BiConsumer<? super FetchResult, Throwable>
callback)
+ {
Review Comment:
nit: unused constructor; but could be used in C*?
##########
accord-core/src/main/java/accord/utils/ArrayBuffers.java:
##########
@@ -169,6 +174,73 @@ default int[] completeAndDiscard(int[] buffer, int
usedSize)
boolean forceDiscard(int[] buffer);
}
+ public interface LongBufferAllocator
+ {
+ /**
+ * Return a {@code long[]} of size at least {@code minSize}, possibly
from a pool.
+ * This array may not be zero initialized, and its contents should be
treated as random.
+ */
+ long[] getLongs(int minSize);
+ }
+
+ public interface LongBuffers extends LongBufferAllocator
+ {
+ /**
+ * Return an {@code int[]} of size at least {@code minSize}, possibly
from a pool,
Review Comment:
nit: probably `long` here
##########
accord-core/src/main/java/accord/utils/ArrayBuffers.java:
##########
@@ -169,6 +174,73 @@ default int[] completeAndDiscard(int[] buffer, int
usedSize)
boolean forceDiscard(int[] buffer);
}
+ public interface LongBufferAllocator
+ {
+ /**
+ * Return a {@code long[]} of size at least {@code minSize}, possibly
from a pool.
+ * This array may not be zero initialized, and its contents should be
treated as random.
+ */
+ long[] getLongs(int minSize);
+ }
+
+ public interface LongBuffers extends LongBufferAllocator
+ {
+ /**
+ * Return an {@code int[]} of size at least {@code minSize}, possibly
from a pool,
+ * and copy the contents of {@code copyAndDiscard} into it.
+ *
+ * The remainder of the array may not be zero-initialized, and should
be assumed to contain random data.
+ *
+ * The parameter will be returned to the pool, if eligible.
+ */
+ default long[] resize(long[] copyAndDiscard, int usedSize, int minSize)
+ {
+ long[] newBuf = getLongs(minSize);
+ System.arraycopy(copyAndDiscard, 0, newBuf, 0, usedSize);
+ forceDiscard(copyAndDiscard);
+ return newBuf;
+ }
+
+ /**
+ * To be invoked on the result buffer with the number of elements
contained;
+ * either the buffer will be returned and the size optionally
captured, or else the result may be
+ * shrunk to the size of the contents, depending on implementation.
+ */
+ long[] complete(long[] buffer, int usedSize);
+
+ /**
+ * The buffer is no longer needed by the caller, which is discarding
the array;
+ * if {@link #complete(long[], int)} returned the buffer as its result
this buffer should NOT be
+ * returned to any pool.
+ *
+ * Note that this method assumes {@link #complete(long[], int)} was
invoked on this buffer previously.
+ * However, it is guaranteed that a failure to do so does not leak
memory or pool space, only produces some
+ * additional garbage.
+ *
+ * @return true if the buffer is discarded (and discard-able), false
if it was retained or is believed to be in use
+ */
+ boolean discard(long[] buffer, int usedSize);
+
+ /**
+ * Equivalent to
+ * int[] result = complete(buffer, usedSize);
Review Comment:
nit: probably `long[]` here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
