Skip to content

Commit f8931b7

Browse files
committed
FetchRequest should report as unavailable any slice that executes in a later epoch that is not owned by the replicas
Improve: - Remove GetMaxConflict; use local MaxConflict collection Fix: - Invalidated should retain StoreParticipants so we can update CFK on journal replay - loading pruned uninitialised commands via CFK: make sure hasTouched contains key so that if invalidated we are notified - updateExecuteAtLeast should always be higher than TxnId - Async CFK callbacks treated pruned transactions incorrectly - node.withEpoch when ExecuteEphemeralRead in futureEpoch - Deps.without should be key/range aware - Durably mark bootstrapBeganAt and safeToReadAt in MaxConflicts - Don't attempt to calculate local deps when DepsErased in GetLatestDeps (command has durably applied to this shard) - filter StoreParticipants before invoking shouldCleanup - CFK should treat !stillTouches as outOfRange (rather than !touches)
1 parent c076383 commit f8931b7

39 files changed

+946
-929
lines changed

accord-core/src/main/java/accord/coordinate/ExecuteEphemeralRead.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import accord.primitives.TxnId;
4040
import accord.topology.Topologies;
4141
import accord.utils.Invariants;
42+
import accord.utils.WrappableException;
4243

4344
import static accord.coordinate.ReadCoordinator.Action.Aborted;
4445
import static accord.coordinate.ReadCoordinator.Action.Approve;
@@ -98,7 +99,9 @@ protected Action process(Id from, ReadReply reply)
9899
if (ok.futureEpoch > allTopologies.currentEpoch())
99100
{
100101
// TODO (expected): only submit new requests for the keys that execute in a later epoch
101-
new ExecuteEphemeralRead(node, node.topology().preciseEpochs(route, ok.futureEpoch, ok.futureEpoch), route, txnId.withEpoch(ok.futureEpoch), txn, ok.futureEpoch, deps, callback).start();
102+
node.withEpoch(ok.futureEpoch, callback, t -> WrappableException.wrap(t), () -> {
103+
new ExecuteEphemeralRead(node, node.topology().preciseEpochs(route, ok.futureEpoch, ok.futureEpoch), route, txnId.withEpoch(ok.futureEpoch), txn, ok.futureEpoch, deps, callback).start();
104+
});
102105
return Aborted;
103106
}
104107

accord-core/src/main/java/accord/coordinate/FetchMaxConflict.java

Lines changed: 0 additions & 123 deletions
This file was deleted.

accord-core/src/main/java/accord/coordinate/Recover.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -342,17 +342,17 @@ private void recover()
342342
Invariants.checkState(committedExecuteAt == null || committedExecuteAt.equals(txnId));
343343
// should all be PreAccept
344344
Deps proposeDeps = LatestDeps.mergeProposal(recoverOkList, ok -> ok == null ? null : ok.deps);
345-
Deps earlierAcceptedNoWitness = Deps.merge(recoverOkList, recoverOkList.size(), List::get, ok -> ok.earlierAcceptedNoWitness);
346-
Deps earlierCommittedWitness = Deps.merge(recoverOkList, recoverOkList.size(), List::get, ok -> ok.earlierCommittedWitness);
347-
earlierAcceptedNoWitness = earlierAcceptedNoWitness.without(earlierCommittedWitness::contains);
348-
if (!earlierAcceptedNoWitness.isEmpty())
345+
Deps earlierWait = Deps.merge(recoverOkList, recoverOkList.size(), List::get, ok -> ok.earlierWait);
346+
Deps earlierNoWait = Deps.merge(recoverOkList, recoverOkList.size(), List::get, ok -> ok.earlierNoWait);
347+
earlierWait = earlierWait.without(earlierNoWait);
348+
if (!earlierWait.isEmpty())
349349
{
350350
// If there exist commands that were proposed an earlier execution time than us that have not witnessed us,
351351
// we have to be certain these commands have not successfully committed without witnessing us (thereby
352352
// ruling out a fast path decision for us and changing our recovery decision).
353353
// So, we wait for these commands to finish committing before retrying recovery.
354354
// See whitepaper for more details
355-
awaitCommits(node, earlierAcceptedNoWitness).addCallback((success, failure) -> {
355+
awaitCommits(node, earlierWait).addCallback((success, failure) -> {
356356
if (failure != null) accept(null, failure);
357357
else retry();
358358
});

accord-core/src/main/java/accord/coordinate/tracking/SimpleTracker.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ public SimpleTracker(Topologies topologies, IntFunction<ST[]> arrayFactory, Func
3434
super(topologies, arrayFactory, trackerFactory);
3535
}
3636

37-
public SimpleTracker(Topologies topologies, IntFunction<ST[]> arrayFactory, ShardFactory<ST> trackerFactory)
38-
{
39-
super(topologies, arrayFactory, trackerFactory);
40-
}
41-
4237
public RequestStatus recordSuccess(Id from, boolean withFastPathTimestamp) { return recordSuccess(from); }
4338
public RequestStatus recordDelayed(Id from) { return NoChange; }
4439
public boolean hasFastPathAccepted() { return false; }

accord-core/src/main/java/accord/impl/AbstractFetchCoordinator.java

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.List;
2424
import java.util.Map;
2525

26+
import accord.local.Command;
2627
import accord.local.SafeCommandStore;
2728
import accord.messages.ReadData;
2829
import accord.utils.async.AsyncChain;
@@ -50,8 +51,11 @@
5051
import accord.utils.async.AsyncChains;
5152
import accord.utils.async.AsyncResult;
5253
import accord.utils.async.AsyncResults;
54+
55+
import javax.annotation.Nonnull;
5356
import javax.annotation.Nullable;
5457

58+
import static accord.messages.ReadEphemeralTxnData.retryInLaterEpoch;
5559
import static accord.primitives.SaveStatus.Applied;
5660
import static accord.primitives.SaveStatus.TruncatedApply;
5761
import static accord.messages.ReadData.CommitOrReadNack.Insufficient;
@@ -123,13 +127,8 @@ public CommandStore commandStore()
123127
}
124128

125129
protected abstract PartialTxn rangeReadTxn(Ranges ranges);
126-
127130
protected abstract void onReadOk(Node.Id from, CommandStore commandStore, Data data, Ranges ranges);
128-
129-
protected FetchRequest newFetchRequest(long sourceEpoch, TxnId syncId, Ranges ranges, PartialDeps partialDeps, PartialTxn partialTxn)
130-
{
131-
return new FetchRequest(sourceEpoch, syncId, ranges, partialDeps, rangeReadTxn(ranges));
132-
}
131+
protected abstract FetchRequest newFetchRequest(long sourceEpoch, TxnId syncId, Ranges ranges, PartialDeps partialDeps, PartialTxn partialTxn);
133132

134133
@Override
135134
public void contact(Node.Id to, Ranges ranges)
@@ -187,7 +186,7 @@ public void onSuccess(Node.Id from, ReadReply reply)
187186
}
188187

189188
// TODO (now): make sure it works if invoked in either order
190-
inflight.remove(key).started(ok.maxApplied);
189+
inflight.remove(key).started(ok.safeToReadAfter);
191190
onReadOk(to, commandStore, ok.data, received);
192191
// received must be invoked after submitting the persistence future, as it triggers onDone
193192
// which creates a ReducingFuture over {@code persisting}
@@ -237,11 +236,16 @@ void abort(Ranges abort)
237236
// TODO (expected): implement abort
238237
}
239238

240-
public static class FetchRequest extends ReadData
239+
public static abstract class FetchRequest extends ReadData
241240
{
241+
// Note for future: we cannot safely execute on an Erased sync point without more work.
242+
// Specifically, if the range has partially lost ownership on the recipient, the SyncPoint
243+
// will not represent a safe point to snapshot from, and we won't have enough information to
244+
// report the range as unavailable.
242245
private static final ExecuteOn EXECUTE_ON = new ExecuteOn(Applied, TruncatedApply);
243246
public final PartialTxn read;
244247
public final PartialDeps partialDeps;
248+
private transient Timestamp safeToReadAfter;
245249

246250
public FetchRequest(long sourceEpoch, TxnId syncId, Ranges ranges, PartialDeps partialDeps, PartialTxn partialTxn)
247251
{
@@ -268,22 +272,45 @@ protected AsyncChain<Data> beginRead(SafeCommandStore safeStore, Timestamp execu
268272
return read.read(safeStore, executeAt, unavailable);
269273
}
270274

275+
// must be invoked by implementations some time after the read has started OR must override safeToReadAt()
276+
protected void readStarted(SafeCommandStore safeStore, Ranges unavailable)
277+
{
278+
safeToReadAfter = Timestamp.nonNullOrMax(Timestamp.NONE, Timestamp.nonNullOrMax(safeToReadAfter, safeStore.commandStore().unsafeGetMaxConflicts().foldl(Timestamp::nonNullOrMax)));
279+
}
280+
281+
protected Timestamp safeToReadAfter()
282+
{
283+
return safeToReadAfter;
284+
}
285+
271286
@Override
272287
protected void readComplete(CommandStore commandStore, Data result, Ranges unavailable)
273288
{
274-
Ranges reportUnavailable = unavailable.slice((Ranges)this.readScope, Minimal);
289+
Ranges reportUnavailable = unavailable == null ? null : unavailable.slice((Ranges)this.readScope, Minimal);
275290
super.readComplete(commandStore, result, reportUnavailable);
276291
}
277292

278293
@Override
279294
protected ReadOk constructReadOk(Ranges unavailable, Data data)
280295
{
281-
return new FetchResponse(unavailable, data, maxApplied());
296+
Timestamp safeToReadAfter = safeToReadAfter();
297+
Invariants.checkState(data == null || safeToReadAfter != null);
298+
return new FetchResponse(unavailable, data, safeToReadAfter);
282299
}
283300

284-
protected Timestamp maxApplied()
301+
@Override
302+
protected void read(SafeCommandStore safeStore, Command command)
285303
{
286-
return null;
304+
long retryInLaterEpoch = retryInLaterEpoch(executeAtEpoch, safeStore, command);
305+
if (retryInLaterEpoch > 0)
306+
{
307+
Ranges unavailable = ((Ranges)readScope).slice(safeStore.ranges().allAt(executeAtEpoch), Minimal);
308+
readComplete(safeStore.commandStore(), null, unavailable);
309+
}
310+
else
311+
{
312+
super.read(safeStore, command);
313+
}
287314
}
288315

289316
@Override
@@ -295,11 +322,13 @@ public MessageType type()
295322

296323
public static class FetchResponse extends ReadOk
297324
{
298-
public final @Nullable Timestamp maxApplied;
299-
public FetchResponse(@Nullable Ranges unavailable, @Nullable Data data, @Nullable Timestamp maxApplied)
325+
// only null if retryInFutureEpoch is set
326+
public final @Nullable Timestamp safeToReadAfter;
327+
328+
public FetchResponse(@Nullable Ranges unavailable, @Nullable Data data, @Nonnull Timestamp safeToReadAfter)
300329
{
301330
super(unavailable, data);
302-
this.maxApplied = maxApplied;
331+
this.safeToReadAfter = safeToReadAfter;
303332
}
304333

305334
@Override

0 commit comments

Comments
 (0)