ifesdjeen commented on code in PR #150:
URL: https://github.com/apache/cassandra-accord/pull/150#discussion_r1884292590
##########
accord-core/src/main/java/accord/impl/AbstractConfigurationService.java:
##########
@@ -289,13 +289,16 @@ public void fetchTopologyForEpoch(long epoch)
fetchTopologyInternal(epoch);
}
+ // TODO (expected): rename, sync is too ambiguous
Review Comment:
Oh yes please. I was looking into whether we need to move this into Listener
because of this name, too.
##########
accord-core/src/main/java/accord/local/RedundantBefore.java:
##########
@@ -712,37 +857,95 @@ public Ranges removePreBootstrap(TxnId txnId, Ranges
ranges)
{
if (maxBootstrap.compareTo(txnId) <= 0)
return ranges;
- return foldl(ranges, Entry::removePreBootstrap, ranges, txnId, null, r
-> false);
+ return foldl(ranges, Entry::withoutPreBootstrap, ranges, txnId, null,
r -> false);
}
/**
- * Subtract any ranges we consider stale or pre-bootstrap
+ * Subtract anything we don't need to coordinate (because they are known
to be shard durable),
+ * and we don't execute locally, i.e. are pre-bootstrap or stale (or for
RX are on ranges that are already retired)
*/
- public Ranges expectToExecute(TxnId txnId, @Nonnull Timestamp executeAt,
Ranges ranges)
+ public Participants<?> expectToOwn(TxnId txnId, @Nullable EpochSupplier
executeAt, Participants<?> participants)
{
- Invariants.checkArgument(executeAt != null, "executeAt must not be
null");
- if (maxBootstrap.compareTo(txnId) <= 0 && (staleRanges == null ||
!staleRanges.intersects(ranges)))
- return ranges;
- return foldl(ranges, Entry::expectToExecute, ranges, txnId, executeAt,
r -> false);
+ if (txnId.is(ExclusiveSyncPoint))
+ {
+ if (!mayFilterStaleOrPreBootstrapOrRetired(txnId, participants))
+ return participants;
+
+ return foldl(participants,
Entry::withoutRedundantAnd_StaleOrPreBootstrapOrRetired, participants, txnId, i
-> false);
+ }
+ else
+ {
+ if (!mayFilterStaleOrPreBootstrap(txnId, participants))
+ return participants;
+
+ return foldl(participants,
Entry::withoutRedundantAnd_StaleOrPreBootstrap, participants, txnId, executeAt,
r -> false);
+ }
}
/**
- * Subtract any ranges we consider stale or pre-bootstrap at any point
+ * Subtract anything we won't execute locally, i.e. are pre-bootstrap or
stale (or for RX are on ranges that are already retired)
*/
- public Ranges everExpectToExecute(TxnId txnId, Ranges ranges)
+ public Participants<?> expectToExecute(TxnId txnId, @Nullable
EpochSupplier executeAt, Participants<?> participants)
{
- if (maxBootstrap.compareTo(txnId) <= 0 && (staleRanges == null ||
!staleRanges.intersects(ranges)))
- return ranges;
- return foldl(ranges, Entry::expectToExecute, ranges, txnId, null, r ->
false);
+ if (txnId.is(ExclusiveSyncPoint))
+ {
+ if (!mayFilterStaleOrPreBootstrapOrRetired(txnId, participants))
+ return participants;
+
+ return foldl(participants,
Entry::withoutStaleOrPreBootstrapOrLocallyRetired, participants, txnId, i ->
false);
+ }
+ else
+ {
+ if (!mayFilterStaleOrPreBootstrap(txnId, participants))
+ return participants;
+
+ return foldl(participants,
Entry::participantsWithoutStaleOrPreBootstrap, participants, txnId, executeAt,
r -> false);
+ }
+ }
+
+ public boolean mayFilter(TxnId txnId, Participants<?> participants)
Review Comment:
nit: why a shortcut name for this one?
##########
accord-core/src/main/java/accord/local/cfk/Pruning.java:
##########
@@ -393,23 +393,23 @@ private static Long2ObjectHashMap<TxnInfo>
buildEpochPrunedBefores(TxnInfo[] byI
return null;
Long2ObjectHashMap<TxnInfo> epochPrunedBefores = new
Long2ObjectHashMap<>();
- for (long epoch = newPrunedBefore.epoch() ; epoch <=
newPrunedBefore.executeAt.epoch(); ++epoch)
- epochPrunedBefores.put(epoch, newPrunedBefore);
+ epochPrunedBefores.put(newPrunedBefore.epoch(), newPrunedBefore);
- int maxi = Arrays.binarySearch(committedByExecuteAt, newPrunedBefore,
TxnInfo::compareExecuteAt);
+ int maxi = -1 - Arrays.binarySearch(committedByExecuteAt,
newPrunedBefore, (a, b) -> a.compareExecuteAtEpoch(b) >= 0 ? 1 : -1);
Review Comment:
do we need to check for < 0 here?
##########
accord-core/src/main/java/accord/coordinate/tracking/QuorumIdTracker.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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 accord.coordinate.tracking;
+
+import java.util.Set;
+
+import accord.local.Node;
+import accord.topology.Shard;
+import accord.topology.Topologies;
+import org.agrona.collections.ObjectHashSet;
+
+import static accord.coordinate.tracking.AbstractTracker.ShardOutcomes.Fail;
+import static
accord.coordinate.tracking.AbstractTracker.ShardOutcomes.NoChange;
+import static accord.coordinate.tracking.AbstractTracker.ShardOutcomes.Success;
+
+public class QuorumIdTracker extends
SimpleTracker<QuorumIdTracker.QuorumIdShardTracker> implements ResponseTracker
Review Comment:
Should we maybe always use `QuorumIdTracker`? I have been in a situation
recently, where I had to reconstruct IDs of successes and failures.
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -91,7 +102,7 @@ private Commands()
{
}
- public enum AcceptOutcome { Success, Redundant, RejectedBallot, Truncated }
+ public enum AcceptOutcome { Success, Redundant, RejectedBallot, Retired,
Truncated }
Review Comment:
Is this a correctness issue or optimizaiton? In either case would be great
to document.
##########
accord-core/src/main/java/accord/local/RedundantBefore.java:
##########
@@ -161,8 +185,7 @@ public Entry(Range range, long startOwnershipEpoch, long
endOwnershipEpoch, @Non
this.bootstrappedAt = bootstrappedAt;
this.staleUntilAtLeast = staleUntilAtLeast;
checkNoneOrRX(locallyWitnessedOrInvalidatedBefore,
locallyAppliedOrInvalidatedBefore, locallyDecidedAndAppliedOrInvalidatedBefore,
- shardAppliedOrInvalidatedBefore, gcBefore);
- checkPartiallyOrdered(locallyDecidedAndAppliedOrInvalidatedBefore,
locallyAppliedOrInvalidatedBefore);
+ shardAppliedOrInvalidatedBefore,
gcBefore);checkPartiallyOrdered(locallyDecidedAndAppliedOrInvalidatedBefore,
locallyAppliedOrInvalidatedBefore);
Review Comment:
nit: line break here?
##########
accord-core/src/main/java/accord/impl/InMemoryCommandStore.java:
##########
@@ -331,6 +332,17 @@ protected void updatedRedundantBefore(SafeCommandStore
safeStore, TxnId syncId,
}
});
});
+ TxnId clearProgressLogBefore =
unsafeGetRedundantBefore().minShardRedundantBefore();
+ List<TxnId> clearing = ((DefaultProgressLog)
progressLog).activeBefore(clearProgressLogBefore);
+ for (TxnId txnId : clearing)
+ {
+ GlobalCommand globalCommand = commands.get(txnId);
+; Invariants.checkState(globalCommand != null &&
!globalCommand.isEmpty());
Review Comment:
nit: `;`
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -718,7 +706,7 @@ else if (safeStore.isFullyPreBootstrapOrStale(dependency,
waiting.partialDeps().
static void updateDependencyAndMaybeExecute(SafeCommandStore safeStore,
SafeCommand safeCommand, SafeCommand predecessor, boolean notifyWaitingOn)
{
Command.Committed command = safeCommand.current().asCommitted();
- if (command.hasBeen(Applied))
+ if (command.hasBeen(Status.Applied))
Review Comment:
nit: `Status` added by accident?
##########
accord-core/src/main/java/accord/local/CommandStore.java:
##########
@@ -479,11 +479,12 @@ final Supplier<EpochReady> bootstrapper(Node node, Ranges
newRanges, long epoch)
Bootstrap bootstrap = new Bootstrap(node, this, epoch,
newRanges);
bootstraps.add(bootstrap);
bootstrap.start(safeStore);
- return new EpochReady(epoch, null, bootstrap.coordination,
bootstrap.data, bootstrap.reads);
+ return new EpochReady(epoch, null, null, bootstrap.data,
bootstrap.reads);
Review Comment:
I might be missing something, but if the node can process reads, it should
be ready for coordination, too or?
##########
accord-core/src/main/java/accord/coordinate/ExecuteTxn.java:
##########
@@ -104,7 +106,7 @@ private Commit.Kind commitKind()
@Override
public void contact(Id to)
{
- if (SEND_MINIMUM_STABLE_MESSAGES) Commit.stableAndRead(to, node,
allTopologies, commitKind(), txnId, txn, route, readScope, executeAt,
stableDeps, this, SEND_MINIMUM_STABLE_MESSAGES);
+ if (SEND_MINIMUM_STABLE_MESSAGES && path != RECOVER)
Commit.stableAndRead(to, node, allTopologies, commitKind(), txnId, txn, route,
readScope, executeAt, stableDeps, this, false);
Review Comment:
Is it intended that we e passing `false` here? Above we seem to be passing
`true` under same condition (send minimum and path not recover).
##########
accord-core/src/main/java/accord/coordinate/CollectLatestDeps.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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 accord.coordinate;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+import accord.api.RoutingKey;
+import accord.coordinate.tracking.QuorumTracker;
+import accord.local.CommandStore;
+import accord.local.Node;
+import accord.local.Node.Id;
+import accord.messages.Callback;
+import accord.messages.GetLatestDeps;
+import accord.messages.GetLatestDeps.GetLatestDepsOk;
+import accord.primitives.FullRoute;
+import accord.primitives.LatestDeps;
+import accord.primitives.Timestamp;
+import accord.primitives.TxnId;
+import accord.primitives.Unseekables;
+import accord.topology.Topologies;
+import accord.utils.SortedArrays.SortedArrayList;
+import accord.utils.SortedListMap;
+
+import static accord.coordinate.tracking.RequestStatus.Failed;
+import static accord.coordinate.tracking.RequestStatus.Success;
+
+public class CollectLatestDeps implements Callback<GetLatestDepsOk>
+{
+ final Node node;
+ final TxnId txnId;
+ final RoutingKey homeKey;
+ final Timestamp executeAt;
+
+ private final SortedListMap<Id, GetLatestDepsOk> oks;
Review Comment:
could you elaborate why it is important for this to be sorted?
##########
accord-core/src/main/java/accord/local/CommandStore.java:
##########
@@ -498,15 +499,21 @@ final Supplier<EpochReady> bootstrapper(Node node, Ranges
newRanges, long epoch)
protected Supplier<EpochReady> sync(Node node, Ranges ranges, long epoch)
{
return () -> {
- if (redundantBefore.min(ranges,
RedundantBefore.Entry::locallyWitnessedBefore).epoch() >= epoch)
- return new EpochReady(epoch, DONE, DONE, DONE, DONE);
-
- AsyncResults.SettableResult<Void> whenDone = new
AsyncResults.SettableResult<>();
- waitingOnSync.put(epoch, new WaitingOnSync(whenDone, ranges));
- return new EpochReady(epoch, DONE, whenDone, DONE, DONE);
+ AsyncResult<Void> readyToCoordinate = readyToCoordinate(ranges,
epoch);
+ return new EpochReady(epoch, DONE, readyToCoordinate, DONE, DONE);
Review Comment:
hm, looks like I was wrong above (can serve reads -> can coordinate), so it
would be nice to have this documented somewhere.
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -107,11 +118,11 @@ public static AcceptOutcome recover(SafeCommandStore
safeStore, SafeCommand safe
private static AcceptOutcome preacceptOrRecover(SafeCommandStore
safeStore, SafeCommand safeCommand, StoreParticipants participants, TxnId
txnId, long acceptEpoch, PartialTxn partialTxn, FullRoute<?> route, Ballot
ballot)
{
final Command command = safeCommand.current();
-
if (command.hasBeen(Truncated))
{
logger.trace("{}: skipping preaccept - command is truncated",
txnId);
- return command.is(Invalidated) ? AcceptOutcome.RejectedBallot :
AcceptOutcome.Truncated;
+ return command.is(Invalidated) ? AcceptOutcome.RejectedBallot :
participants.owns().isEmpty()
Review Comment:
nit: would you mind to expand double ternary?
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -598,9 +582,11 @@ public static boolean maybeExecute(SafeCommandStore
safeStore, SafeCommand safeC
{
case Stable:
// TODO (desirable, efficiency): maintain distinct ReadyToRead
and ReadyToWrite states
- // TODO (required): we can have dangling transactions in some
cases when proposing in a future epoch but
- // later deciding on an earlier epoch. We should probably
turn this into an erased vestigial command,
- // but we should tighten up our semantics there in general.
+ // TODO (required): we can have dangling transactions if we
don't execute them,
+ // but we don't want to erase the transaction until
Review Comment:
Sentence seems to be finished abruptly?
##########
accord-core/src/main/java/accord/local/RedundantBefore.java:
##########
@@ -712,37 +857,95 @@ public Ranges removePreBootstrap(TxnId txnId, Ranges
ranges)
{
if (maxBootstrap.compareTo(txnId) <= 0)
return ranges;
- return foldl(ranges, Entry::removePreBootstrap, ranges, txnId, null, r
-> false);
+ return foldl(ranges, Entry::withoutPreBootstrap, ranges, txnId, null,
r -> false);
}
/**
- * Subtract any ranges we consider stale or pre-bootstrap
+ * Subtract anything we don't need to coordinate (because they are known
to be shard durable),
+ * and we don't execute locally, i.e. are pre-bootstrap or stale (or for
RX are on ranges that are already retired)
*/
- public Ranges expectToExecute(TxnId txnId, @Nonnull Timestamp executeAt,
Ranges ranges)
+ public Participants<?> expectToOwn(TxnId txnId, @Nullable EpochSupplier
executeAt, Participants<?> participants)
{
- Invariants.checkArgument(executeAt != null, "executeAt must not be
null");
- if (maxBootstrap.compareTo(txnId) <= 0 && (staleRanges == null ||
!staleRanges.intersects(ranges)))
- return ranges;
- return foldl(ranges, Entry::expectToExecute, ranges, txnId, executeAt,
r -> false);
+ if (txnId.is(ExclusiveSyncPoint))
+ {
+ if (!mayFilterStaleOrPreBootstrapOrRetired(txnId, participants))
+ return participants;
+
+ return foldl(participants,
Entry::withoutRedundantAnd_StaleOrPreBootstrapOrRetired, participants, txnId, i
-> false);
+ }
+ else
+ {
+ if (!mayFilterStaleOrPreBootstrap(txnId, participants))
+ return participants;
+
+ return foldl(participants,
Entry::withoutRedundantAnd_StaleOrPreBootstrap, participants, txnId, executeAt,
r -> false);
+ }
}
/**
- * Subtract any ranges we consider stale or pre-bootstrap at any point
+ * Subtract anything we won't execute locally, i.e. are pre-bootstrap or
stale (or for RX are on ranges that are already retired)
*/
- public Ranges everExpectToExecute(TxnId txnId, Ranges ranges)
+ public Participants<?> expectToExecute(TxnId txnId, @Nullable
EpochSupplier executeAt, Participants<?> participants)
{
- if (maxBootstrap.compareTo(txnId) <= 0 && (staleRanges == null ||
!staleRanges.intersects(ranges)))
- return ranges;
- return foldl(ranges, Entry::expectToExecute, ranges, txnId, null, r ->
false);
+ if (txnId.is(ExclusiveSyncPoint))
+ {
+ if (!mayFilterStaleOrPreBootstrapOrRetired(txnId, participants))
+ return participants;
+
+ return foldl(participants,
Entry::withoutStaleOrPreBootstrapOrLocallyRetired, participants, txnId, i ->
false);
+ }
+ else
+ {
+ if (!mayFilterStaleOrPreBootstrap(txnId, participants))
Review Comment:
nit: do I understand right that we can get here only for non-exclusive
syncpoints?
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -1162,41 +1246,27 @@ static Command
removeNoLongerOwnedDependency(SafeCommandStore safeStore, SafeCom
return safeCommand.updateWaitingOn(update);
}
- /**
- * A key nominated to represent the "home" shard - only members of the
home shard may be nominated to recover
- * a transaction, to reduce the cluster-wide overhead of ensuring
progress. A transaction that has only been
- * witnessed at PreAccept may however trigger a process of ensuring the
home shard is durably informed of
- * the transaction.
- *
- * Note that for ProgressLog purposes the "home shard" is the shard as of
txnId.epoch.
- * For recovery purposes the "home shard" is as of txnId.epoch until
Committed, and executeAt.epoch once Executed
- */
- @SuppressWarnings({"unchecked", "rawtypes"})
- public static CommonAttributes updateParticipants(Command command,
StoreParticipants participants)
- {
- participants = command.participants().supplement(participants);
+ public static CommonAttributes supplementParticipants(Command command,
StoreParticipants participants)
+ { // TODO (required): invoke participants.supplement before creating
mutable
if (command.participants() == participants)
return command;
- return command.mutable().updateParticipants(participants);
+ return command.mutable().supplementParticipants(participants);
}
public static CommonAttributes updateRoute(Command command, Route<?> route)
{
StoreParticipants participants =
command.participants().supplement(route);
- if (command.participants() == participants)
- return command;
-
- return command.mutable().updateParticipants(participants);
+ return supplementParticipants(command, participants);
}
- public static Command updateParticipants(SafeCommandStore safeStore,
SafeCommand safeCommand, StoreParticipants participants)
+ public static Command supplementParticipants(SafeCommandStore safeStore,
SafeCommand safeCommand, StoreParticipants participants)
{
Command current = safeCommand.current();
if (current.saveStatus().compareTo(Erased) >= 0)
return current;
-
- CommonAttributes updated = updateParticipants(current, participants);
+ // TODO (required): invoke updateParticipants directly
Review Comment:
nit: supplement?
##########
accord-core/src/main/java/accord/local/RedundantStatus.java:
##########
@@ -80,6 +92,26 @@ public enum RedundantStatus
*/
LOCALLY_REDUNDANT,
+ /**
+ * The relevant owned ranges are all shard-redundant AND (pre-bootstrap or
stale), meaning any intersecting transaction
Review Comment:
Are these new states required for correctness?
##########
accord-core/src/main/java/accord/local/cfk/CommandsForKey.java:
##########
@@ -740,7 +753,7 @@ public enum InternalStatus
STABLE(true, false),
APPLIED(true, false),
INVALIDATED(false, false),
- TRUNCATED_OR_PRUNED(false, false)
Review Comment:
could you elaborate a difference between truncated and pruned here?
##########
accord-core/src/main/java/accord/local/cfk/PostProcess.java:
##########
@@ -275,30 +275,54 @@ static NotifyUnmanagedResult notifyUnmanaged(Unmanaged[]
unmanageds,
}
{
- Timestamp applyTo = null;
if (newInfo != null && newInfo.is(APPLIED))
{
TxnInfo maxContiguousApplied =
CommandsForKey.maxContiguousManagedApplied(committedByExecuteAt,
maxAppliedWriteByExecuteAt, bootstrappedAt);
if (maxContiguousApplied != null &&
maxContiguousApplied.compareExecuteAt(newInfo) >= 0)
- applyTo = maxContiguousApplied.executeAt;
- }
- else if (newInfo == null)
- {
- TxnInfo maxContiguousApplied =
CommandsForKey.maxContiguousManagedApplied(committedByExecuteAt,
maxAppliedWriteByExecuteAt, bootstrappedAt);
- if (maxContiguousApplied != null)
- applyTo = maxContiguousApplied.executeAt;
-
- applyTo = Timestamp.nonNullOrMax(applyTo,
TxnId.nonNullOrMax(redundantBefore, bootstrappedAt));
+ {
+ Timestamp applyTo = maxContiguousApplied.executeAt;
+ int start = findFirstApply(unmanageds);
+ int end = findApply(unmanageds, start, applyTo);
+ if (start != end)
+ {
+ TxnId[] notifyNotWaiting = selectUnmanaged(unmanageds,
start, end);
+ unmanageds = removeUnmanaged(unmanageds, start, end);
+ notifier = new PostProcess.NotifyNotWaiting(notifier,
notifyNotWaiting);
+ }
+ }
}
-
- if (applyTo != null)
+ else if (newInfo == null && isNewBoundsInfo)
{
int start = findFirstApply(unmanageds);
- int end = findApply(unmanageds, start, applyTo);
- if (start != end)
+ int end = start;
+ int j = 1 +
maxContiguousManagedAppliedIndex(committedByExecuteAt,
maxAppliedWriteByExecuteAt, bootstrappedAt);
+ while (end < unmanageds.length && j <
committedByExecuteAt.length)
+ {
+ int c =
committedByExecuteAt[j].executeAt.compareTo(unmanageds[end].waitingUntil);
+ if (c == 0)
+ {
+ if (start != end)
+ {
+ TxnId[] notifyNotWaiting =
selectUnmanaged(unmanageds, start, end);
+ unmanageds = removeUnmanaged(unmanageds, start,
end);
+ end -= (end - start);
Review Comment:
Had to re-read this several times. Isn't this just `end = start` ?
##########
accord-core/src/main/java/accord/messages/AbstractRequest.java:
##########
@@ -81,7 +81,7 @@ public final void process(Node on, Node.Id replyTo,
ReplyContext replyContext)
{
long expiresAt = node.agent().expiresAt(replyContext,
MICROSECONDS);
if (expiresAt > 0)
- {
+ { // TODO (required): now!! this should be registerAt (not
changing to avoid messing up seed)
Review Comment:
Reminding of this one
##########
accord-core/src/main/java/accord/messages/ReadData.java:
##########
@@ -53,12 +53,15 @@
import static accord.api.ProgressLog.BlockedUntil.HasStableDeps;
import static accord.messages.MessageType.READ_RSP;
import static accord.messages.ReadData.CommitOrReadNack.Insufficient;
+import static accord.messages.ReadData.CommitOrReadNack.Invalid;
import static accord.messages.ReadData.CommitOrReadNack.Redundant;
import static accord.messages.TxnRequest.latestRelevantEpochIndex;
import static accord.primitives.Routables.Slice.Minimal;
import static accord.utils.Invariants.illegalState;
import static java.util.concurrent.TimeUnit.MICROSECONDS;
+// TODO (expected): if one shard timesout waiting to reply, but another shard
produces a reply, return a partial response (or response with suitably
populated unavailable)
Review Comment:
nit: times out
##########
accord-core/src/main/java/accord/messages/Commit.java:
##########
@@ -173,64 +174,65 @@ public static void stableAndRead(Node node, Topologies
all, Kind kind, TxnId txn
{
Invariants.checkState(all.oldestEpoch() == txnId.epoch());
Invariants.checkState(all.currentEpoch() == executeAt.epoch());
- Topology executes = all.forEpoch(executeAt.epoch());
- Topology coordinates = all.forEpoch(txnId.epoch());
+ Topology executes = all.getEpoch(executeAt.epoch());
+ Topology coordinates = all.getEpoch(txnId.epoch());
SortedArrayList<Id> contact = all.nodes().without(all::isFaulty);
- sendTo(contact, readSet, (set, id) -> set.contains(id.id), readScope,
node, coordinates, executes, all, kind, Ballot.ZERO,
+ sendTo(contact, readSet, (set, id) -> set.contains(id.id), (set, id)
-> false, readScope, node, coordinates, executes, all, kind, Ballot.ZERO,
Review Comment:
Did you intend to add this _after_ `(set, id) -> set.contains(id.id)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]