This is an automated email from the ASF dual-hosted git repository.
benedict pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-accord.git
The following commit(s) were added to refs/heads/trunk by this push:
new 53be172 Triage TODOs (#24)
53be172 is described below
commit 53be1722ad48e9d3c85996787fcc0d69e3ada6fa
Author: Benedict Elliott Smith <[email protected]>
AuthorDate: Wed Jan 11 12:10:08 2023 +0000
Triage TODOs (#24)
---
accord-core/src/main/java/accord/api/Result.java | 2 +-
accord-core/src/main/java/accord/api/Write.java | 2 +-
.../src/main/java/accord/coordinate/CheckOn.java | 4 +-
.../main/java/accord/coordinate/CheckShards.java | 2 +-
.../main/java/accord/coordinate/Coordinate.java | 19 ++++----
.../src/main/java/accord/coordinate/FetchData.java | 2 +-
.../java/accord/coordinate/InformHomeOfTxn.java | 4 +-
.../main/java/accord/coordinate/Invalidate.java | 20 ++++-----
.../main/java/accord/coordinate/MaybeRecover.java | 4 +-
.../src/main/java/accord/coordinate/Persist.java | 6 +--
.../java/accord/coordinate/ReadCoordinator.java | 8 ++--
.../src/main/java/accord/coordinate/Recover.java | 17 ++++----
.../java/accord/coordinate/RecoverWithRoute.java | 2 +-
.../accord/coordinate/tracking/ReadTracker.java | 12 +++---
.../java/accord/impl/InMemoryCommandStore.java | 7 +--
.../main/java/accord/impl/SimpleProgressLog.java | 33 +++++++-------
.../src/main/java/accord/local/Command.java | 50 +++++++++-------------
.../src/main/java/accord/local/CommandsForKey.java | 6 +--
accord-core/src/main/java/accord/local/Node.java | 21 +++------
.../src/main/java/accord/local/PartialCommand.java | 27 ------------
.../src/main/java/accord/local/PreLoadContext.java | 2 +-
.../src/main/java/accord/local/SaveStatus.java | 6 +--
.../main/java/accord/local/ShardDistributor.java | 4 +-
accord-core/src/main/java/accord/local/Status.java | 5 ++-
.../main/java/accord/local/SyncCommandStores.java | 2 +-
.../src/main/java/accord/messages/Accept.java | 5 ++-
.../src/main/java/accord/messages/Apply.java | 1 -
.../main/java/accord/messages/BeginRecovery.java | 8 ++--
.../src/main/java/accord/messages/Callback.java | 3 +-
.../src/main/java/accord/messages/Commit.java | 13 +++---
.../src/main/java/accord/messages/Defer.java | 11 +++--
.../src/main/java/accord/messages/GetDeps.java | 1 -
.../main/java/accord/messages/InformDurable.java | 6 +--
.../java/accord/messages/InformHomeDurable.java | 2 +-
.../main/java/accord/messages/InformOfTxnId.java | 6 +--
.../src/main/java/accord/messages/PreAccept.java | 2 +-
.../src/main/java/accord/messages/ReadData.java | 14 +++---
.../src/main/java/accord/messages/TxnRequest.java | 20 ++++-----
.../main/java/accord/primitives/AbstractKeys.java | 7 ++-
.../java/accord/primitives/AbstractRanges.java | 9 ++--
.../src/main/java/accord/primitives/Deps.java | 23 +++++-----
.../main/java/accord/primitives/FullKeyRoute.java | 8 ----
.../src/main/java/accord/primitives/Keys.java | 3 +-
.../java/accord/primitives/PartialKeyRoute.java | 7 ---
.../java/accord/primitives/PartialRangeRoute.java | 2 -
.../main/java/accord/primitives/PartialTxn.java | 7 +--
.../main/java/accord/primitives/ProgressToken.java | 1 -
.../main/java/accord/primitives/RoutableKey.java | 2 +-
.../src/main/java/accord/topology/Shard.java | 4 +-
.../src/main/java/accord/topology/Topologies.java | 4 +-
.../src/main/java/accord/topology/Topology.java | 40 ++---------------
.../main/java/accord/topology/TopologyManager.java | 4 +-
.../src/main/java/accord/utils/ArrayBuffers.java | 2 +-
.../accord/utils/DeterministicIdentitySet.java | 2 +-
.../main/java/accord/utils/IndexedTriConsumer.java | 1 -
.../java/accord/utils/IntrusiveLinkedList.java | 2 +-
.../java/accord/utils/IntrusiveLinkedListNode.java | 2 +-
.../src/main/java/accord/utils/Invariants.java | 5 +++
.../src/main/java/accord/utils/MapReduce.java | 2 +-
.../src/main/java/accord/utils/SortedArrays.java | 10 ++---
accord-core/src/main/java/accord/utils/Utils.java | 2 +-
accord-core/src/test/java/accord/KeysTest.java | 2 -
.../accord/burn/BurnTestConfigurationService.java | 2 +-
.../src/test/java/accord/burn/TopologyUpdates.java | 2 +-
.../tracking/InvalidationTrackerReconciler.java | 3 +-
.../tracking/RecoveryTrackerReconciler.java | 2 +-
.../coordinate/tracking/TrackerReconciler.java | 4 +-
.../src/test/java/accord/impl/TopologyFactory.java | 1 -
.../src/test/java/accord/impl/basic/Cluster.java | 7 +--
.../src/test/java/accord/impl/list/ListAgent.java | 2 +-
.../test/java/accord/impl/list/ListRequest.java | 2 +-
.../test/java/accord/impl/mock/MockCluster.java | 2 +-
.../src/test/java/accord/impl/mock/Network.java | 4 +-
.../java/accord/topology/TopologyRandomizer.java | 4 +-
accord-core/src/test/java/accord/txn/DepsTest.java | 11 +----
.../accord/verify/LinearizabilityVerifier.java | 2 +-
.../accord/verify/SerializabilityVerifier.java | 7 +--
.../accord/verify/SerializabilityVerifierTest.java | 2 +-
.../verify/StrictSerializabilityVerifier.java | 19 ++++----
.../src/main/java/accord/maelstrom/Cluster.java | 2 +-
.../src/main/java/accord/maelstrom/Json.java | 1 -
.../java/accord/maelstrom/MaelstromRequest.java | 1 -
.../src/main/java/accord/maelstrom/Value.java | 1 -
.../src/test/java/accord/maelstrom/Runner.java | 2 +-
84 files changed, 229 insertions(+), 364 deletions(-)
diff --git a/accord-core/src/main/java/accord/api/Result.java
b/accord-core/src/main/java/accord/api/Result.java
index be7abff..a5ac298 100644
--- a/accord-core/src/main/java/accord/api/Result.java
+++ b/accord-core/src/main/java/accord/api/Result.java
@@ -24,7 +24,7 @@ import accord.primitives.ProgressToken;
/**
* A result to be returned to a client, or be stored in a node's command state.
*
- * TODO: support minimizing the result for storage in a node's command state
(e.g. to only retain success/failure)
+ * TODO (expected, efficiency): support minimizing the result for storage in a
node's command state (e.g. to only retain success/failure)
*/
public interface Result extends Outcome
{
diff --git a/accord-core/src/main/java/accord/api/Write.java
b/accord-core/src/main/java/accord/api/Write.java
index 15fa6c8..6e5f439 100644
--- a/accord-core/src/main/java/accord/api/Write.java
+++ b/accord-core/src/main/java/accord/api/Write.java
@@ -25,7 +25,7 @@ import org.apache.cassandra.utils.concurrent.Future;
/**
* A collection of data to write to one or more stores
*
- * TODO: support splitting so as to minimise duplication of data across shards
+ * TODO (desired, efficiency): support splitting so as to minimise duplication
of data across shards
*/
public interface Write
{
diff --git a/accord-core/src/main/java/accord/coordinate/CheckOn.java
b/accord-core/src/main/java/accord/coordinate/CheckOn.java
index 18ab6f9..d37a827 100644
--- a/accord-core/src/main/java/accord/coordinate/CheckOn.java
+++ b/accord-core/src/main/java/accord/coordinate/CheckOn.java
@@ -64,7 +64,7 @@ public class CheckOn extends CheckShards
CheckOn(Node node, Known sufficient, TxnId txnId, Route<?> route,
Unseekables<?, ?> routeWithHomeKey, long srcEpoch, long untilLocalEpoch,
BiConsumer<? super CheckStatusOkFull, Throwable> callback)
{
- // TODO (soon): restore behaviour of only collecting info if e.g.
Committed or Executed
+ // TODO (desired, efficiency): restore behaviour of only collecting
info if e.g. Committed or Executed
super(node, txnId, routeWithHomeKey, srcEpoch, IncludeInfo.All);
Preconditions.checkArgument(routeWithHomeKey.contains(route.homeKey()));
this.sufficient = sufficient;
@@ -73,7 +73,7 @@ public class CheckOn extends CheckShards
this.untilLocalEpoch = untilLocalEpoch;
}
- // TODO: many callers only need to consult precisely executeAt.epoch
remotely
+ // TODO (required, consider): many callers only need to consult precisely
executeAt.epoch remotely
public static CheckOn checkOn(Known sufficientStatus, Node node, TxnId
txnId, Route<?> route, long srcEpoch, long untilLocalEpoch, BiConsumer<? super
CheckStatusOkFull, Throwable> callback)
{
CheckOn checkOn = new CheckOn(node, sufficientStatus, txnId, route,
srcEpoch, untilLocalEpoch, callback);
diff --git a/accord-core/src/main/java/accord/coordinate/CheckShards.java
b/accord-core/src/main/java/accord/coordinate/CheckShards.java
index dd91993..9ad05fd 100644
--- a/accord-core/src/main/java/accord/coordinate/CheckShards.java
+++ b/accord-core/src/main/java/accord/coordinate/CheckShards.java
@@ -19,7 +19,7 @@ public abstract class CheckShards extends
ReadCoordinator<CheckStatusReply>
/**
* The epoch until which we want to fetch data from remotely
- * TODO: configure the epoch we want to start with
+ * TODO (required, consider): configure the epoch we want to start with
*/
final long untilRemoteEpoch;
final IncludeInfo includeInfo;
diff --git a/accord-core/src/main/java/accord/coordinate/Coordinate.java
b/accord-core/src/main/java/accord/coordinate/Coordinate.java
index 836bfdb..ae65014 100644
--- a/accord-core/src/main/java/accord/coordinate/Coordinate.java
+++ b/accord-core/src/main/java/accord/coordinate/Coordinate.java
@@ -44,7 +44,7 @@ import static
accord.messages.Commit.Invalidate.commitInvalidate;
* Perform initial rounds of PreAccept and Accept until we have reached
agreement about when we should execute.
* If we are preempted by a recovery coordinator, we abort and let them
complete (and notify us about the execution result)
*
- * TODO: dedicated burn test to validate outcomes
+ * TODO (desired, testing): dedicated burn test to validate outcomes
*/
public class Coordinate extends AsyncFuture<Result> implements
Callback<PreAcceptReply>, BiConsumer<Result, Throwable>
{
@@ -70,7 +70,7 @@ public class Coordinate extends AsyncFuture<Result>
implements Callback<PreAccep
private void start()
{
- // TODO: consider sending only to electorate of most recent topology
(as only these PreAccept votes matter)
+ // TODO (desired, efficiency): consider sending only to electorate of
most recent topology (as only these PreAccept votes matter)
// note that we must send to all replicas of old topology, as
electorate may not be reachable
node.send(tracker.nodes(), to -> new PreAccept(to,
tracker.topologies(), txnId, txn, route), this);
}
@@ -124,9 +124,9 @@ public class Coordinate extends AsyncFuture<Result>
implements Callback<PreAccep
successes.add(ok);
boolean fastPath = ok.witnessedAt.compareTo(txnId) == 0;
- // TODO: update formalisation (and proof), as we do not seek
additional pre-accepts from later epochs.
- // instead we rely on accept to do our work: a quorum of accept
in the later epoch
- // and its effect on preaccepted timestamps and the deps it
returns create our sync point.
+ // TODO (desired, safety): update formalisation (and proof), as we do
not seek additional pre-accepts from later epochs.
+ // instead we rely on accept to do our work: a
quorum of accept in the later epoch
+ // and its effect on preaccepted timestamps
and the deps it returns create our sync point.
if (tracker.recordSuccess(from, fastPath) == RequestStatus.Success)
onPreAccepted();
}
@@ -151,9 +151,10 @@ public class Coordinate extends AsyncFuture<Result>
implements Callback<PreAccep
executeAt = accumulate;
}
- // TODO: perhaps don't submit Accept immediately if we almost have
enough for fast-path,
- // but by sending accept we rule out hybrid fast-path
- // TODO: if we receive a MAX response, perhaps defer to permit at
least one other node to respond before invalidating
+ // TODO (low priority, efficiency): perhaps don't submit Accept
immediately if we almost have enough for fast-path,
+ // but by sending accept we rule
out hybrid fast-path
+ // TODO (low priority, efficiency): if we receive an expired
response, perhaps defer to permit at least one other
+ // node to respond before
invalidating
if (node.agent().isExpired(txnId, executeAt.real))
{
proposeInvalidate(node, Ballot.ZERO, txnId, route.homeKey(),
(success, fail) -> {
@@ -165,7 +166,7 @@ public class Coordinate extends AsyncFuture<Result>
implements Callback<PreAccep
{
node.withEpoch(executeAt.epoch, () -> {
commitInvalidate(node, txnId, route, executeAt);
- // TODO: this should be Invalidated rather than
Timeout?
+ // TODO (required, API consistency): this should
be Invalidated rather than Timeout?
accept(null, new Timeout(txnId, route.homeKey()));
});
}
diff --git a/accord-core/src/main/java/accord/coordinate/FetchData.java
b/accord-core/src/main/java/accord/coordinate/FetchData.java
index f1e876c..1bdba70 100644
--- a/accord-core/src/main/java/accord/coordinate/FetchData.java
+++ b/accord-core/src/main/java/accord/coordinate/FetchData.java
@@ -18,7 +18,7 @@ import static accord.local.Status.Outcome.OutcomeUnknown;
/**
* Find data and persist locally
*
- * TODO accept lower bound epoch to avoid fetching data we should already have
+ * TODO (desired, efficiency): accept lower bound epoch to avoid fetching data
we should already have
*/
public class FetchData
{
diff --git a/accord-core/src/main/java/accord/coordinate/InformHomeOfTxn.java
b/accord-core/src/main/java/accord/coordinate/InformHomeOfTxn.java
index 9ab76fc..79fac8b 100644
--- a/accord-core/src/main/java/accord/coordinate/InformHomeOfTxn.java
+++ b/accord-core/src/main/java/accord/coordinate/InformHomeOfTxn.java
@@ -69,7 +69,7 @@ public class InformHomeOfTxn extends AsyncFuture<Void>
implements Callback<Simpl
break;
case Nack:
- // TODO: stale topology should be impossible right now
+ // TODO (required, consider): stale topology should be
impossible right now
onFailure(from, new StaleTopology());
}
}
@@ -80,7 +80,7 @@ public class InformHomeOfTxn extends AsyncFuture<Void>
implements Callback<Simpl
if (this.failure == null) this.failure = failure;
else this.failure.addSuppressed(failure);
- // TODO: if we fail and have an incorrect topology, trigger refresh
+ // TODO (required, consider): if we fail and have an incorrect
topology, trigger refresh
if (tracker.onFailure(null) == Fail)
tryFailure(this.failure);
}
diff --git a/accord-core/src/main/java/accord/coordinate/Invalidate.java
b/accord-core/src/main/java/accord/coordinate/Invalidate.java
index f923fe2..1ed1b75 100644
--- a/accord-core/src/main/java/accord/coordinate/Invalidate.java
+++ b/accord-core/src/main/java/accord/coordinate/Invalidate.java
@@ -166,7 +166,7 @@ public class Invalidate implements Callback<InvalidateReply>
case ReadyToExecute:
case PreApplied:
case Applied:
- // TODO: if we see Committed or above, go straight to
Execute if we have assembled enough information
+ // TODO (desired, efficiency): if we see Committed or
above, go straight to Execute if we have assembled enough information
if (route != null)
{
// The data we see might have made it only to a
minority in the event of PreAccept ONLY.
@@ -218,7 +218,7 @@ public class Invalidate implements Callback<InvalidateReply>
return;
case Invalidated:
- // TODO: standardise semantics of async/sync local
application prior to callback
+ // TODO (desired, API consistency): standardise semantics
of whether local application of state prior is async or sync to callback
isDone = true;
commitInvalidate();
return;
@@ -227,13 +227,13 @@ public class Invalidate implements
Callback<InvalidateReply>
// if we have witnessed the transaction, but are able to invalidate,
do we want to proceed?
// Probably simplest to do so, but perhaps better for user if we don't.
- // TODO (RangeTxns): This should be a Routable, or we should guarantee
it is safe to operate on any key in the range
+ // TODO (now, rangetxns): This should be a Routable, or we should
guarantee it is safe to operate on any key in the range
RoutingKey invalidateWithKey =
invalidateWith.slice(Ranges.of(tracker.promisedShard().range)).get(0).someIntersectingRoutingKey();
proposeInvalidate(node, ballot, txnId, invalidateWithKey, (success,
fail) -> {
- /**
- * We're now inside our *exactly once* callback we registered with
proposeInvalidate, and we need to
- * make sure we honour our own exactly once semantics with {@code
callback}.
- * So we are responsible for all exception handling.
+ /*
+ We're now inside our *exactly once* callback we registered with
proposeInvalidate, and we need to
+ make sure we honour our own exactly once semantics with {@code
callback}.
+ So we are responsible for all exception handling.
*/
isDone = true;
if (fail != null)
@@ -257,15 +257,15 @@ public class Invalidate implements
Callback<InvalidateReply>
private void commitInvalidate()
{
@Nullable Route<?> route = InvalidateReply.mergeRoutes(replies);
- // TODO: commitInvalidate (and others) should skip the network for
local applications,
+ // TODO (desired, efficiency): commitInvalidate (and others) should
skip the network for local applications,
// so we do not need to explicitly do so here before notifying the
waiter
Commit.Invalidate.commitInvalidate(node, txnId, route != null ?
Unseekables.merge(route, (Unseekables)invalidateWith) : invalidateWith, txnId);
- // TODO: pick a reasonable upper bound, so we don't invalidate into an
epoch/commandStore that no longer cares about this command
+ // TODO (required, consider): pick a reasonable upper bound, so we
don't invalidate into an epoch/commandStore that no longer cares about this
command
node.forEachLocalSince(contextFor(txnId), invalidateWith, txnId,
safeStore -> {
safeStore.command(txnId).commitInvalidate(safeStore);
}).addCallback((s, f) -> {
callback.accept(INVALIDATED, null);
- if (f != null) // TODO: consider exception handling more
carefully: should we catch these prior to passing to callbacks?
+ if (f != null) // TODO (required): consider exception handling
more carefully: should we catch these prior to passing to callbacks?
node.agent().onUncaughtException(f);
});
}
diff --git a/accord-core/src/main/java/accord/coordinate/MaybeRecover.java
b/accord-core/src/main/java/accord/coordinate/MaybeRecover.java
index 7840666..1522ec4 100644
--- a/accord-core/src/main/java/accord/coordinate/MaybeRecover.java
+++ b/accord-core/src/main/java/accord/coordinate/MaybeRecover.java
@@ -69,7 +69,7 @@ public class MaybeRecover extends CheckShards
public boolean hasMadeProgress(CheckStatusOk ok)
{
- return ok != null && (ok.isCoordinating // TODO (now): make this
coordinatingSince so can be pre-empted by others if stuck
+ return ok != null && (ok.isCoordinating // TODO (required, liveness):
make this coordinatingSince so can be pre-empted by others if stuck
|| ok.toProgressToken().compareTo(prevProgress)
> 0);
}
@@ -106,7 +106,7 @@ public class MaybeRecover extends CheckShards
break;
case InvalidationApplied:
- // TODO: we should simply invoke commitInvalidate
+ // TODO (easy, efficiency): we should simply invoke
commitInvalidate
Unseekables<?, ?> someKeys =
reduceNonNull(Unseekables::merge, (Unseekables)contact, merged.route, route);
Invalidate.invalidate(node, txnId, someKeys.with(homeKey),
callback);
}
diff --git a/accord-core/src/main/java/accord/coordinate/Persist.java
b/accord-core/src/main/java/accord/coordinate/Persist.java
index 7d9c997..a7daec9 100644
--- a/accord-core/src/main/java/accord/coordinate/Persist.java
+++ b/accord-core/src/main/java/accord/coordinate/Persist.java
@@ -92,7 +92,7 @@ public class Persist implements Callback<ApplyReply>
{
if (!isDone)
{
- // TODO: send to non-home replicas also, so they may
clear their log more easily?
+ // TODO (low priority, consider, efficiency): send to
non-home replicas also, so they may clear their log more easily?
Shard homeShard =
node.topology().forEpochIfKnown(route.homeKey(), txnId.epoch);
node.send(homeShard, new InformHomeDurable(txnId,
route.homeKey(), executeAt, Durable, persistedOn));
isDone = true;
@@ -106,7 +106,7 @@ public class Persist implements Callback<ApplyReply>
break;
case Insufficient:
Topologies topologies = node.topology().preciseEpochs(route,
txnId.epoch, executeAt.epoch);
- // TODO (review with Ariel): use static method in Commit
+ // TODO (easy, cleanup): use static method in Commit
node.send(from, new Commit(Kind.Maximal, from,
topologies.forEpoch(txnId.epoch), topologies, txnId, txn, route, null,
executeAt, deps, false));
}
}
@@ -114,7 +114,7 @@ public class Persist implements Callback<ApplyReply>
@Override
public void onFailure(Id from, Throwable failure)
{
- // TODO: send knowledge of partial persistence?
+ // TODO (desired, consider): send knowledge of partial persistence?
}
@Override
diff --git a/accord-core/src/main/java/accord/coordinate/ReadCoordinator.java
b/accord-core/src/main/java/accord/coordinate/ReadCoordinator.java
index 3c9ccbf..837b2cc 100644
--- a/accord-core/src/main/java/accord/coordinate/ReadCoordinator.java
+++ b/accord-core/src/main/java/accord/coordinate/ReadCoordinator.java
@@ -179,10 +179,10 @@ abstract class ReadCoordinator<Reply extends
accord.messages.Reply> extends Read
@Override
protected RequestStatus trySendMore()
{
- // TODO: due to potential re-entrancy into this method, if the node we
are contacting is unavailable
- // so onFailure is invoked immediately, for the moment we copy nodes
to an intermediate list.
- // would be better to prevent reentrancy either by detecting this
inside trySendMore or else queueing
- // callbacks externally, so two may not be in-flight at once
+ // TODO (low priority): due to potential re-entrancy into this method,
if the node we are contacting is unavailable
+ // so onFailure is invoked immediately, for the
moment we copy nodes to an intermediate list.
+ // would be better to prevent reentrancy either
by detecting this inside trySendMore or else
+ // queueing callbacks externally, so two may not
be in-flight at once
List<Id> contact = new ArrayList<>(1);
RequestStatus status = trySendMore(List::add, contact);
contact.forEach(this::contact);
diff --git a/accord-core/src/main/java/accord/coordinate/Recover.java
b/accord-core/src/main/java/accord/coordinate/Recover.java
index 12eee58..9f8e80c 100644
--- a/accord-core/src/main/java/accord/coordinate/Recover.java
+++ b/accord-core/src/main/java/accord/coordinate/Recover.java
@@ -51,14 +51,14 @@ import static
accord.coordinate.tracking.RequestStatus.Failed;
import static accord.coordinate.tracking.RequestStatus.Success;
import static accord.messages.BeginRecovery.RecoverOk.maxAcceptedOrLater;
-// TODO: rename to Recover (verb); rename Recover message to not clash
+// TODO (low priority, cleanup): rename to Recover (verb); rename Recover
message to not clash
public class Recover implements Callback<RecoverReply>, BiConsumer<Result,
Throwable>
{
class AwaitCommit extends AsyncFuture<Timestamp> implements
Callback<WaitOnCommitOk>
{
- // TODO: this should collect the executeAt of any commit, and
terminate as soon as one is found
- // that is earlier than TxnId for the Txn we are recovering; if
all commits we wait for
- // are given earlier timestamps we can retry without restarting.
+ // TODO (desired, efficiency): this should collect the executeAt of
any commit, and terminate as soon as one is found
+ // that is earlier than TxnId for the Txn
we are recovering; if all commits we wait for
+ // are given earlier timestamps we can
retry without restarting.
final QuorumTracker tracker;
AwaitCommit(Node node, TxnId txnId, Unseekables<?, ?> unseekables)
@@ -100,7 +100,6 @@ public class Recover implements Callback<RecoverReply>,
BiConsumer<Result, Throw
for (int i = 0 ; i < waitOn.txnIdCount() ; ++i)
{
TxnId txnId = waitOn.txnId(i);
- // TODO (now): this should perhaps use RouteFragment as we might
need to handle txns that are range-only
new AwaitCommit(node, txnId,
waitOn.someRoutables(txnId)).addCallback((success, failure) -> {
if (future.isDone())
return;
@@ -212,7 +211,7 @@ public class Recover implements Callback<RecoverReply>,
BiConsumer<Result, Throw
case Applied:
case PreApplied:
- // TODO: in some cases we can use the deps we already have
(e.g. if we have a quorum of Committed responses)
+ // TODO (desired, efficiency): in some cases we can use
the deps we already have (e.g. if we have a quorum of Committed responses)
node.withEpoch(executeAt.epoch, () -> {
CollectDeps.withDeps(node, txnId, route, txn,
acceptOrCommit.executeAt, (deps, fail) -> {
if (fail != null)
@@ -221,7 +220,7 @@ public class Recover implements Callback<RecoverReply>,
BiConsumer<Result, Throw
}
else
{
- // TODO: when writes/result are partially
replicated, need to confirm we have quorum of these
+ // TODO (required, consider): when
writes/result are partially replicated, need to confirm we have quorum of these
Persist.persistAndCommit(node, txnId, route,
txn, executeAt, deps, acceptOrCommit.writes, acceptOrCommit.result);
accept(acceptOrCommit.result, null);
}
@@ -232,7 +231,7 @@ public class Recover implements Callback<RecoverReply>,
BiConsumer<Result, Throw
case ReadyToExecute:
case PreCommitted:
case Committed:
- // TODO: in some cases we can use the deps we already have
(e.g. if we have a quorum of Committed responses)
+ // TODO (desired, efficiency): in some cases we can use
the deps we already have (e.g. if we have a quorum of Committed responses)
node.withEpoch(executeAt.epoch, () -> {
CollectDeps.withDeps(node, txnId, route, txn,
executeAt, (deps, fail) -> {
if (fail != null) accept(null, fail);
@@ -273,7 +272,7 @@ public class Recover implements Callback<RecoverReply>,
BiConsumer<Result, Throw
// we have to be certain these commands have not successfully
committed without witnessing us (thereby
// ruling out a fast path decision for us and changing our
recovery decision).
// So, we wait for these commands to finish committing before
retrying recovery.
- // TODO: check paper: do we assume that witnessing in PreAccept
implies witnessing in Accept? Not guaranteed.
+ // TODO (required): check paper: do we assume that witnessing in
PreAccept implies witnessing in Accept? Not guaranteed.
// See whitepaper for more details
awaitCommits(node, earlierAcceptedNoWitness).addCallback((success,
failure) -> {
if (failure != null) accept(null, failure);
diff --git a/accord-core/src/main/java/accord/coordinate/RecoverWithRoute.java
b/accord-core/src/main/java/accord/coordinate/RecoverWithRoute.java
index 59c95a1..775f1d7 100644
--- a/accord-core/src/main/java/accord/coordinate/RecoverWithRoute.java
+++ b/accord-core/src/main/java/accord/coordinate/RecoverWithRoute.java
@@ -136,7 +136,7 @@ public class RecoverWithRoute extends CheckShards
case OutcomeKnown:
Invariants.checkState(known.definition.isKnown());
Invariants.checkState(known.executeAt.isDecisionKnown());
- // TODO: we might not be able to reconstitute Txn if we have
GC'd on some shards
+ // TODO (required): we might not be able to reconstitute Txn
if we have GC'd on some shards
Txn txn = merged.partialTxn.reconstitute(route);
if (known.deps.isDecisionKnown())
{
diff --git
a/accord-core/src/main/java/accord/coordinate/tracking/ReadTracker.java
b/accord-core/src/main/java/accord/coordinate/tracking/ReadTracker.java
index e86f2d4..73d70f2 100644
--- a/accord-core/src/main/java/accord/coordinate/tracking/ReadTracker.java
+++ b/accord-core/src/main/java/accord/coordinate/tracking/ReadTracker.java
@@ -157,16 +157,16 @@ public class ReadTracker extends
AbstractTracker<ReadTracker.ReadShardTracker, B
}
}
- // TODO: abstract the candidate selection process so the implementation
may prioritise based on distance/health etc
- final Set<Id> inflight; // TODO: use Agrona's IntHashSet as soon as
Node.Id switches from long to int
- final List<Id> candidates; // TODO: use Agrona's IntArrayList as soon as
Node.Id switches from long to int
- private Set<Id> slow; // TODO: use Agrona's IntHashSet as soon as
Node.Id switches from long to int
+ // TODO (required): abstract the candidate selection process so the
implementation may prioritise based on distance/health etc
+ final Set<Id> inflight; // TODO (easy, efficiency): use Agrona's
IntHashSet as soon as Node.Id switches from long to int
+ final List<Id> candidates; // TODO (easy, efficiency): use Agrona's
IntArrayList as soon as Node.Id switches from long to int
+ private Set<Id> slow; // TODO (easy, efficiency): use Agrona's
IntHashSet as soon as Node.Id switches from long to int
protected int waitingOnData;
public ReadTracker(Topologies topologies)
{
super(topologies, ReadShardTracker[]::new, ReadShardTracker::new);
- this.candidates = new ArrayList<>(topologies.nodes()); // TODO:
copyOfNodesAsList to avoid unnecessary copies
+ this.candidates = new ArrayList<>(topologies.nodes()); // TODO (low
priority, efficiency): copyOfNodesAsList to avoid unnecessary copies
this.inflight = newHashSetWithExpectedSize(maxShardsPerEpoch());
this.waitingOnData = waitingOnShards;
}
@@ -256,7 +256,7 @@ public class ReadTracker extends
AbstractTracker<ReadTracker.ReadShardTracker, B
Invariants.checkState(toRead != null, "We were asked to read more, but
found no shards in need of reading more");
- // TODO: maybe for each additional candidate do one linear compare run
to find better secondary match
+ // TODO (desired, consider): maybe for each additional candidate do
one linear compare run to find better secondary match
// OR at least discount candidates that do not contribute
additional knowledge beyond those additional
// candidates already contacted, since implementations are
likely to sort primarily by health
candidates.sort((a, b) -> topologies().compare(a, b, toRead));
diff --git a/accord-core/src/main/java/accord/impl/InMemoryCommandStore.java
b/accord-core/src/main/java/accord/impl/InMemoryCommandStore.java
index f6b2d57..f112d0c 100644
--- a/accord-core/src/main/java/accord/impl/InMemoryCommandStore.java
+++ b/accord-core/src/main/java/accord/impl/InMemoryCommandStore.java
@@ -54,6 +54,7 @@ import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
+// TODO (low priority): efficiency
public class InMemoryCommandStore
{
public static abstract class State implements SafeCommandStore
@@ -84,7 +85,7 @@ public class InMemoryCommandStore
return commands.get(txnId);
}
- // TODO (soon): mimic caching to test C* behaviour
+ // TODO (required): mimic caching to test C* behaviour
public Command ifLoaded(TxnId txnId)
{
return commands.get(txnId);
@@ -227,7 +228,6 @@ public class InMemoryCommandStore
default:
throw new AssertionError();
case Key:
- // TODO: efficiency
AbstractKeys<Key, ?> keys = (AbstractKeys<Key, ?>)
keysOrRanges;
return keys.stream()
.filter(slice::contains)
@@ -235,7 +235,6 @@ public class InMemoryCommandStore
.map(map)
.reduce(initialValue, reduce);
case Range:
- // TODO: efficiency
Ranges ranges = (Ranges) keysOrRanges;
return ranges.slice(slice).stream().flatMap(range ->
commandsForKey.subMap(range.start(),
range.startInclusive(), range.end(), range.endInclusive()).values().stream()
@@ -254,7 +253,6 @@ public class InMemoryCommandStore
break;
case Range:
Ranges ranges = (Ranges) keysOrRanges;
- // TODO: zero allocation
ranges.slice(slice).forEach(range -> {
commandsForKey.subMap(range.start(),
range.startInclusive(), range.end(), range.endInclusive())
.values().forEach(forEach);
@@ -274,7 +272,6 @@ public class InMemoryCommandStore
break;
case Range:
Range range = (Range) keyOrRange;
- // TODO: zero allocation
Ranges.of(range).slice(slice).forEach(r -> {
commandsForKey.subMap(r.start(), r.startInclusive(),
r.end(), r.endInclusive())
.values().forEach(forEach);
diff --git a/accord-core/src/main/java/accord/impl/SimpleProgressLog.java
b/accord-core/src/main/java/accord/impl/SimpleProgressLog.java
index c6069dc..060af15 100644
--- a/accord-core/src/main/java/accord/impl/SimpleProgressLog.java
+++ b/accord-core/src/main/java/accord/impl/SimpleProgressLog.java
@@ -63,7 +63,7 @@ import static accord.local.Status.PreApplied;
import static accord.local.Status.PreCommitted;
import static accord.primitives.Route.isFullRoute;
-// TODO: consider propagating invalidations in the same way as we do applied
+// TODO (desired, consider): consider propagating invalidations in the same
way as we do applied
public class SimpleProgressLog implements ProgressLog.Factory
{
enum Progress { NoneExpected, Expected, NoProgress, Investigating, Done }
@@ -184,7 +184,7 @@ public class SimpleProgressLog implements
ProgressLog.Factory
void updateMax(ProgressToken ok)
{
- // TODO: perhaps set localProgress back to Waiting if
Investigating and we update anything?
+ // TODO (low priority): perhaps set localProgress back to
Waiting if Investigating and we update anything?
token = token.merge(ok);
}
@@ -247,7 +247,6 @@ public class SimpleProgressLog implements
ProgressLog.Factory
return;
ProgressToken token =
success.asProgressToken();
- // TODO: avoid returning null
(need to change semantics here in this case, though, as Recover doesn't return
CheckStatusOk)
if
(token.durability.isDurable())
{
commandStore.execute(contextFor(txnId), safeStore -> {
@@ -284,7 +283,7 @@ public class SimpleProgressLog implements
ProgressLog.Factory
@Override
public void onSuccess(Id from, SimpleReply reply)
{
- // TODO: callbacks should be associated with a
commandStore for processing to avoid this
+ // TODO (required, efficiency): callbacks should be
associated with a commandStore for processing to avoid this
commandStore.execute(PreLoadContext.empty(), ignore ->
{
if (progress() == Done)
return;
@@ -306,8 +305,8 @@ public class SimpleProgressLog implements
ProgressLog.Factory
}
DisseminateStatus status = NotExecuted;
- Set<Id> notAwareOfDurability; // TODO: use Agrona's IntHashSet
as soon as Node.Id switches from long to int
- Set<Id> notPersisted; // TODO: use Agrona's IntHashSet
as soon as Node.Id switches from long to int
+ Set<Id> notAwareOfDurability; // TODO (easy, efficiency): use
Agrona's IntHashSet as soon as Node.Id switches from long to int
+ Set<Id> notPersisted; // TODO (easy, efficiency): use
Agrona's IntHashSet as soon as Node.Id switches from long to int
List<Runnable> whenReady;
@@ -424,7 +423,7 @@ public class SimpleProgressLog implements
ProgressLog.Factory
setProgress(Investigating);
if (notAwareOfDurability.isEmpty())
{
- // TODO: also track actual durability
+ // TODO (required, consider): also track actual
durability
status = DisseminateStatus.Done;
setProgress(Done);
return;
@@ -483,7 +482,7 @@ public class SimpleProgressLog implements
ProgressLog.Factory
// first make sure we have enough information to obtain
the command locally
Timestamp executeAt = command.hasBeen(PreCommitted) ?
command.executeAt() : null;
long srcEpoch = (executeAt != null ? executeAt :
txnId).epoch;
- // TODO: compute fromEpoch, the epoch we already have this
txn replicated until
+ // TODO (desired, consider): compute fromEpoch, the epoch
we already have this txn replicated until
long toEpoch = Math.max(srcEpoch, node.topology().epoch());
Unseekables<?, ?> someKeys = unseekables(command);
@@ -514,7 +513,7 @@ public class SimpleProgressLog implements
ProgressLog.Factory
private void invalidate(Node node, TxnId txnId, Unseekables<?,
?> someKeys)
{
setProgress(Investigating);
- // TODO (RangeTxns): This should be a Routable, or we
should guarantee it is safe to operate on any key in the range
+ // TODO (now, rangetxns): This should be a Routable, or we
should guarantee it is safe to operate on any key in the range
RoutingKey someKey = Route.isRoute(someKeys) ?
(Route.castToRoute(someKeys)).homeKey() :
someKeys.get(0).someIntersectingRoutingKey();
someKeys = someKeys.with(someKey);
debugInvestigating = Invalidate.invalidate(node, txnId,
someKeys, (success, fail) -> {
@@ -775,20 +774,20 @@ public class SimpleProgressLog implements
ProgressLog.Factory
public void durable(TxnId txnId, Unseekables<?, ?> unseekables,
ProgressShard shard)
{
State state = ensure(txnId);
- // TODO (progress consider-prerelease): we can probably simplify
things by requiring (empty) Apply messages to be sent also to the coordinating
topology
+ // TODO (desirable, efficiency): we can probably simplify things
by requiring (empty) Apply messages to be sent also to the coordinating topology
state.recordBlocking(txnId, PreApplied.minKnown, unseekables);
}
@Override
public void waiting(TxnId blockedBy, Known blockedUntil,
Unseekables<?, ?> blockedOn)
{
- // TODO (perf+ consider-prerelease): consider triggering a
preemption of existing coordinator (if any) in some circumstances;
- // today, an LWT can pre-empt more efficiently (i.e.
instantly) a failed operation whereas Accord will
- // wait for some progress interval before taking over; there
is probably some middle ground where we trigger
- // faster preemption once we're blocked on a transaction,
while still offering some amount of time to complete.
- // TODO (soon): forward to local progress shard for processing (if
known)
- // TODO (soon): if we are co-located with the home shard, don't
need to do anything unless we're in a
- // later topology that wasn't covered by its
coordination
+ // TODO (consider): consider triggering a preemption of existing
coordinator (if any) in some circumstances;
+ // today, an LWT can pre-empt more efficiently
(i.e. instantly) a failed operation whereas Accord will
+ // wait for some progress interval before taking
over; there is probably some middle ground where we trigger
+ // faster preemption once we're blocked on a
transaction, while still offering some amount of time to complete.
+ // TODO (desirable, efficiency): forward to local progress shard
for processing (if known)
+ // TODO (desirable, efficiency): if we are co-located with the
home shard, don't need to do anything unless we're in a
+ // later topology that wasn't
covered by its coordination
ensure(blockedBy).recordBlocking(blockedBy, blockedUntil,
blockedOn);
}
diff --git a/accord-core/src/main/java/accord/local/Command.java
b/accord-core/src/main/java/accord/local/Command.java
index 592c90f..3915411 100644
--- a/accord-core/src/main/java/accord/local/Command.java
+++ b/accord-core/src/main/java/accord/local/Command.java
@@ -72,11 +72,10 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
public abstract TxnId txnId();
- // TODO (now): pack this into TxnId
public abstract Kind kind();
public abstract void setKind(Kind kind);
- // TODO (now): should any of these calls be replaced by corresponding
known() registers?
+ // TODO (desirable, API consistency): should any of these calls be
replaced by corresponding known() registers?
public boolean hasBeen(Status status)
{
return status().hasBeen(status);
@@ -120,9 +119,6 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
* so that there is only one copy per node that can be consulted to
construct the full set of involved keys.
*
* If hasBeen(Committed) this must contain the keys for both txnId.epoch
and executeAt.epoch
- *
- * TODO: maybe set this for all local shards, but slice to only those
participating keys
- * (would probably need to remove hashIntersects)
*/
public abstract @Nullable Route<?> route();
protected abstract void setRoute(Route<?> route);
@@ -193,7 +189,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
@Override
public Seekables<?, ?> keys()
{
- // TODO (now): when do we need this, and will it always be sufficient?
+ // TODO (expected, consider): when do we need this, and will it always
be sufficient?
return partialTxn().keys();
}
@@ -265,7 +261,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
}
else
{
- // TODO: in the case that we are pre-committed but had not been
preaccepted/accepted, should we inform progressLog?
+ // TODO (expected, ?): in the case that we are pre-committed but
had not been preaccepted/accepted, should we inform progressLog?
setSaveStatus(SaveStatus.enrich(saveStatus(), DefinitionOnly));
}
set(safeStore, Ranges.EMPTY, coordinateRanges, shard, route,
partialTxn, Set, null, Ignore);
@@ -373,7 +369,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
}
Ranges coordinateRanges = coordinateRanges(safeStore);
- // TODO (now): consider ranges between coordinateRanges and
executeRanges? Perhaps don't need them
+ // TODO (expected, consider): consider ranges between coordinateRanges
and executeRanges? Perhaps don't need them
Ranges executeRanges = executeRanges(safeStore, executeAt);
ProgressShard shard = progressShard(safeStore, route, progressKey,
coordinateRanges);
@@ -389,7 +385,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
safeStore.progressLog().committed(this, shard);
- // TODO (now): introduce intermediate status to avoid reentry when
notifying listeners (which might notify us)
+ // TODO (expected, safety): introduce intermediate status to avoid
reentry when notifying listeners (which might notify us)
maybeExecute(safeStore, shard, true, true);
return CommitOutcome.Success;
}
@@ -438,9 +434,10 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
break;
case PreCommitted:
case Committed:
- // TODO: split into ReadyToRead and ReadyToWrite;
- // the distributed read can be performed as
soon as those keys are ready, and in parallel with any other reads
- // the client can even ACK immediately
after; only the write needs to be postponed until other in-progress reads
complete
+ // TODO (desired, efficiency): split into
ReadyToRead and ReadyToWrite;
+ // the distributed
read can be performed as soon as those keys are ready,
+ // and in parallel
with any other reads. the client can even ACK immediately after;
+ // only the write
needs to be postponed until other in-progress reads complete
case ReadyToExecute:
case PreApplied:
case Applied:
@@ -454,7 +451,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
}
}
- // TODO (now): commitInvalidate may need to update cfks _if_ possible
+ // TODO (expected, ?): commitInvalidate may need to update cfks _if_
possible
public void commitInvalidate(SafeCommandStore safeStore)
{
if (hasBeen(PreCommitted))
@@ -501,7 +498,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
ProgressShard shard = progressShard(safeStore, route,
coordinateRanges);
if (!validate(coordinateRanges, executeRanges, shard, route, Check,
null, Check, partialDeps, hasBeen(Committed) ? Add : TrySet))
- return ApplyOutcome.Insufficient; // TODO: this should probably be
an assertion failure if !TrySet
+ return ApplyOutcome.Insufficient; // TODO (expected, consider):
this should probably be an assertion failure if !TrySet
setWrites(writes);
setResult(result);
@@ -582,9 +579,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
return partialTxn().read(safeStore, this);
}
- // TODO: maybe split into maybeExecute and maybeApply?
- // TODO (performance): If we are a no-op on this shard, just immediately
apply.
- // NOTE: if we ever do transitive dependency elision this could be
dangerous
+ // TODO (expected, API consistency): maybe split into maybeExecute and
maybeApply?
private boolean maybeExecute(SafeCommandStore safeStore, ProgressShard
shard, boolean alwaysNotifyListeners, boolean notifyWaitingOn)
{
if (logger.isTraceEnabled())
@@ -610,7 +605,6 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
switch (status())
{
case Committed:
- // TODO: maintain distinct ReadyToRead and ReadyToWrite states
setStatus(ReadyToExecute);
logger.trace("{}: set to ReadyToExecute", txnId());
safeStore.progressLog().readyToExecute(this, shard);
@@ -625,6 +619,8 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
}
else
{
+ // TODO (desirable, performance): This could be performed
immediately upon Committed
+ // but: if we later support transitive dependency
elision this could be dangerous
logger.trace("{}: applying no-op", txnId());
setStatus(Applied);
notifyListeners(safeStore);
@@ -644,7 +640,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
{
logger.trace("{}: {} is invalidated. Stop listening and removing
from waiting on commit set.", txnId(), dependency.txnId());
dependency.removeListener(this);
- removeWaitingOnCommit(dependency.txnId()); // TODO (now): this was
missing in partial-replication; might be redundant?
+ removeWaitingOnCommit(dependency.txnId());
return true;
}
else if (dependency.executeAt().compareTo(executeAt()) > 0)
@@ -819,10 +815,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
*
* 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
- *
- * TODO: Markdown documentation explaining the home shard and local shard
concepts
*/
-
public final void homeKey(RoutingKey homeKey)
{
RoutingKey current = homeKey();
@@ -1007,7 +1000,6 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
if (shard.isProgress()) setRoute(Route.merge(route(), (Route)route));
else setRoute(Route.merge(route(), (Route)route.slice(allRanges)));
- // TODO (soon): stop round-robin hashing; partition only on ranges
switch (ensurePartialTxn)
{
case Add:
@@ -1018,7 +1010,6 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
{
partialTxn = partialTxn.slice(allRanges, shard.isHome());
Routables.foldlMissing((Seekables)partialTxn.keys(),
partialTxn().keys(), (keyOrRange, p, v, i) -> {
- // TODO: duplicate application of ranges
safeStore.forEach(keyOrRange, allRanges, forKey ->
forKey.register(this));
return v;
}, 0, 0, 1);
@@ -1030,9 +1021,9 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
case TrySet:
setKind(partialTxn.kind());
setPartialTxn(partialTxn = partialTxn.slice(allRanges,
shard.isHome()));
- // TODO: duplicate application of ranges
+ // TODO (expected, efficiency): we may register the same
ranges more than once
safeStore.forEach(partialTxn.keys(), allRanges, forKey -> {
- // TODO: no need to register on PreAccept if already
Accepted
+ // TODO (desirable, efficiency): no need to register on
PreAccept if already Accepted
forKey.register(this);
});
break;
@@ -1134,7 +1125,7 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
return true;
}
- // TODO: callers should try to consult the local progress shard (if any)
to obtain the full set of keys owned locally
+ // TODO (low priority, progress): callers should try to consult the local
progress shard (if any) to obtain the full set of keys owned locally
public Route<?> someRoute()
{
if (route() != null)
@@ -1196,8 +1187,8 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
return txn != null && txn.query() != null;
}
- // TODO: this is an ugly hack, need to encode progress/homeKey/Route state
combinations much more clearly
- // (perhaps introduce encapsulating class representing each possible
arrangement)
+ // TODO (low priority, API): this is an ugly hack, need to encode
progress/homeKey/Route state combinations much more clearly
+ // (perhaps introduce encapsulating class
representing each possible arrangement)
static class NoProgressKey implements RoutingKey
{
@Override
@@ -1205,7 +1196,6 @@ public abstract class Command implements CommandListener,
BiConsumer<SafeCommand
{
throw new UnsupportedOperationException();
}
-
}
private static final NoProgressKey NO_PROGRESS_KEY = new NoProgressKey();
diff --git a/accord-core/src/main/java/accord/local/CommandsForKey.java
b/accord-core/src/main/java/accord/local/CommandsForKey.java
index eb03f28..142440f 100644
--- a/accord-core/src/main/java/accord/local/CommandsForKey.java
+++ b/accord-core/src/main/java/accord/local/CommandsForKey.java
@@ -62,11 +62,7 @@ public abstract class CommandsForKey implements
CommandListener
* Note that {@code testDep} applies only to commands that know at
least proposed deps; if specified any
* commands that do not know any deps will be ignored.
*
- * TODO (soon): TestDep should be asynchronous; data should not be
kept memory-resident as only used for recovery
- *
- * TODO: we don't really need TestStatus anymore, but for clarity it
might be nice to retain it to declare intent.
- * This is because we only use it in places where TestDep is
specified, and the statuses we want to rule-out
- * do not have any deps.
+ * TODO (expected, efficiency): TestDep should be asynchronous; data
should not be kept memory-resident as only used for recovery
*/
Stream<T> before(Timestamp timestamp, TestKind testKind, TestDep
testDep, @Nullable TxnId depId, TestStatus testStatus, @Nullable Status status);
diff --git a/accord-core/src/main/java/accord/local/Node.java
b/accord-core/src/main/java/accord/local/Node.java
index e348a76..6b1ed4e 100644
--- a/accord-core/src/main/java/accord/local/Node.java
+++ b/accord-core/src/main/java/accord/local/Node.java
@@ -104,11 +104,6 @@ public class Node implements
ConfigurationService.Listener, NodeTimeService
return promised.node.equals(id) && coordinating.containsKey(txnId);
}
- public static int numCommandShards()
- {
- return 8; // TODO: make configurable
- }
-
private final Id id;
private final MessageSink messageSink;
private final ConfigurationService configService;
@@ -120,10 +115,10 @@ public class Node implements
ConfigurationService.Listener, NodeTimeService
private final Agent agent;
private final Random random;
- // TODO: this really needs to be thought through some more, as it needs to
be per-instance in some cases, and per-node in others
+ // TODO (expected, consider): this really needs to be thought through some
more, as it needs to be per-instance in some cases, and per-node in others
private final Scheduler scheduler;
- // TODO (soon): monitor the contents of this collection for stalled
coordination, and excise them
+ // TODO (expected, liveness): monitor the contents of this collection for
stalled coordination, and excise them
private final Map<TxnId, Future<? extends Outcome>> coordinating = new
ConcurrentHashMap<>();
public Node(Id id, MessageSink messageSink, ConfigurationService
configService, LongSupplier nowSupplier,
@@ -228,7 +223,7 @@ public class Node implements ConfigurationService.Listener,
NodeTimeService
public Timestamp uniqueNow()
{
return now.updateAndGet(cur -> {
- // TODO: this diverges from proof; either show isomorphism or make
consistent
+ // TODO (low priority, proof): this diverges from proof; either
show isomorphism or make consistent
long now = nowSupplier.getAsLong();
long epoch = Math.max(cur.epoch, topology.epoch());
return (now > cur.real)
@@ -370,12 +365,11 @@ public class Node implements
ConfigurationService.Listener, NodeTimeService
public Future<Result> coordinate(TxnId txnId, Txn txn)
{
- // TODO: The combination of updating the epoch of the next timestamp
with epochs we don't have topologies for,
+ // TODO (desirable, consider): The combination of updating the epoch
of the next timestamp with epochs we don't have topologies for,
// and requiring preaccept to talk to its topology epoch means that
learning of a new epoch via timestamp
// (ie not via config service) will halt any new txns from a node
until it receives this topology
Future<Result> result = withEpoch(txnId.epoch, () ->
initiateCoordination(txnId, txn));
coordinating.putIfAbsent(txnId, result);
- // TODO: if we fail, nominate another node to try instead
result.addCallback((success, fail) -> coordinating.remove(txnId,
result));
return result;
}
@@ -470,14 +464,11 @@ public class Node implements
ConfigurationService.Listener, NodeTimeService
return future;
});
coordinating.putIfAbsent(txnId, result);
- result.addCallback((success, fail) -> {
- coordinating.remove(txnId, result);
- // TODO: if we fail, nominate another node to try instead
- });
+ result.addCallback((success, fail) -> coordinating.remove(txnId,
result));
return result;
}
- // TODO: coalesce other maybeRecover calls also? perhaps have mutable
knownStatuses so we can inject newer ones?
+ // TODO (low priority, API/efficiency): coalesce maybeRecover calls?
perhaps have mutable knownStatuses so we can inject newer ones?
public Future<? extends Outcome> maybeRecover(TxnId txnId, RoutingKey
homeKey, @Nullable Route<?> route, ProgressToken prevProgress)
{
Future<? extends Outcome> result = coordinating.get(txnId);
diff --git a/accord-core/src/main/java/accord/local/PartialCommand.java
b/accord-core/src/main/java/accord/local/PartialCommand.java
deleted file mode 100644
index 93756d8..0000000
--- a/accord-core/src/main/java/accord/local/PartialCommand.java
+++ /dev/null
@@ -1,27 +0,0 @@
-package accord.local;
-
-import accord.primitives.*;
-
-/**
- * A minimal, read only view of a full command. Provided so C* can denormalize
command data onto
- * CommandsForKey and Command implementations. Implementer must guarantee that
PartialCommand
- * implementations behave as an immutable view and are always in sync with the
'real' command;
- */
-public interface PartialCommand
-{
- TxnId txnId();
- Timestamp executeAt();
-
- // TODO: pack this into TxnId
- Txn.Kind kind();
- Status status();
-
- default boolean hasBeen(Status status)
- {
- return status().hasBeen(status);
- }
- default boolean is(Status status)
- {
- return status() == status;
- }
-}
diff --git a/accord-core/src/main/java/accord/local/PreLoadContext.java
b/accord-core/src/main/java/accord/local/PreLoadContext.java
index 0c5b5ab..d00e1c4 100644
--- a/accord-core/src/main/java/accord/local/PreLoadContext.java
+++ b/accord-core/src/main/java/accord/local/PreLoadContext.java
@@ -58,7 +58,7 @@ public interface PreLoadContext
switch (keysOrRanges.kindOfContents())
{
default: throw new AssertionError();
- case Range: return contextFor(txnId); // TODO (soon): this won't
work for actual range queries
+ case Range: return contextFor(txnId); // TODO (required,
correctness): this won't work for actual range queries
case Key: return contextFor(Collections.singleton(txnId),
keysOrRanges);
}
}
diff --git a/accord-core/src/main/java/accord/local/SaveStatus.java
b/accord-core/src/main/java/accord/local/SaveStatus.java
index caae672..3215d1a 100644
--- a/accord-core/src/main/java/accord/local/SaveStatus.java
+++ b/accord-core/src/main/java/accord/local/SaveStatus.java
@@ -53,7 +53,7 @@ public enum SaveStatus
public final Status status;
public final Phase phase;
- public final Known known; // TODO: duplicate contents here to reduce
indirection for majority of cases
+ public final Known known; // TODO (easy, API/efficiency): duplicate
contents here to reduce indirection for majority of cases
SaveStatus(Status status)
{
@@ -74,7 +74,7 @@ public enum SaveStatus
return this.status.compareTo(status) >= 0;
}
- // TODO: exhaustive testing, particularly around PreCommitted
+ // TODO (expected, testing): exhaustive testing, particularly around
PreCommitted
public static SaveStatus get(Status status, Known known)
{
switch (status)
@@ -88,7 +88,7 @@ public enum SaveStatus
return known.isDefinitionKnown() ?
AcceptedInvalidateWithDefinition : AcceptedInvalidate;
// If we know the executeAt decision then we do not clear it,
and fall-through to PreCommitted
// however, we still clear the deps, as any deps we might have
previously seen proposed are now expired
- // TODO: consider clearing Command.partialDeps in this case
also
+ // TODO (expected, consider): consider clearing
Command.partialDeps in this case also
known = known.with(DepsUnknown);
case Accepted:
if (!known.executeAt.isDecisionKnown())
diff --git a/accord-core/src/main/java/accord/local/ShardDistributor.java
b/accord-core/src/main/java/accord/local/ShardDistributor.java
index 620ebaf..d0b2a2d 100644
--- a/accord-core/src/main/java/accord/local/ShardDistributor.java
+++ b/accord-core/src/main/java/accord/local/ShardDistributor.java
@@ -10,8 +10,8 @@ import java.util.List;
public interface ShardDistributor
{
- // TODO: this is overly simplistic: need to supply existing distribution,
- // and support gradual local redistribution to keep number of shards
eventually the same
+ // TODO (expected, topology): this is overly simplistic: need to supply
existing distribution, and support
+ // gradual local redistribution to keep number
of shards eventually the same
List<Ranges> split(Ranges ranges);
class EvenSplit<T> implements ShardDistributor
diff --git a/accord-core/src/main/java/accord/local/Status.java
b/accord-core/src/main/java/accord/local/Status.java
index ad60e00..494bcc8 100644
--- a/accord-core/src/main/java/accord/local/Status.java
+++ b/accord-core/src/main/java/accord/local/Status.java
@@ -370,8 +370,9 @@ public enum Status
this.minKnown = new Known(definition, executeAt, deps, outcome);
}
- // TODO: investigate all uses of hasBeen, and migrate as many as possible
to testing Phase, ReplicationPhase and ExecutionStatus
- // where these concepts are inadequate, see if additional concepts
can be introduced
+ // TODO (desired, clarity): investigate all uses of hasBeen, and migrate
as many as possible to testing
+ // Phase, ReplicationPhase and ExecutionStatus
where these concepts are inadequate,
+ // see if additional concepts can be introduced
public boolean hasBeen(Status equalOrGreaterThan)
{
return compareTo(equalOrGreaterThan) >= 0;
diff --git a/accord-core/src/main/java/accord/local/SyncCommandStores.java
b/accord-core/src/main/java/accord/local/SyncCommandStores.java
index fde0fe4..93476f7 100644
--- a/accord-core/src/main/java/accord/local/SyncCommandStores.java
+++ b/accord-core/src/main/java/accord/local/SyncCommandStores.java
@@ -10,7 +10,7 @@ import accord.utils.MapReduceConsume;
import java.util.function.Function;
import java.util.stream.IntStream;
-// TODO (soon): introduce new CommandStores that mimics asynchrony by
integrating with Cluster scheduling for List workload
+// TODO (desired, testing): introduce new CommandStores that mimics asynchrony
by integrating with Cluster scheduling for List workload
public class SyncCommandStores extends
CommandStores<SyncCommandStores.SyncCommandStore>
{
public interface SafeSyncCommandStore extends SafeCommandStore
diff --git a/accord-core/src/main/java/accord/messages/Accept.java
b/accord-core/src/main/java/accord/messages/Accept.java
index 59dcc10..22d2a87 100644
--- a/accord-core/src/main/java/accord/messages/Accept.java
+++ b/accord-core/src/main/java/accord/messages/Accept.java
@@ -41,7 +41,8 @@ import javax.annotation.Nullable;
import static accord.local.Command.AcceptOutcome.*;
import static accord.messages.PreAccept.calculatePartialDeps;
-// TODO: use different objects for send and receive, so can be more efficient
(e.g. serialize without slicing, and without unnecessary fields)
+// TODO (low priority, efficiency): use different objects for send and
receive, so can be more efficient
+// (e.g. serialize without slicing, and
without unnecessary fields)
public class Accept extends TxnRequest.WithUnsynced<Accept.AcceptReply>
{
public static class SerializerSupport
@@ -90,7 +91,7 @@ public class Accept extends
TxnRequest.WithUnsynced<Accept.AcceptReply>
case RejectedBallot:
return new AcceptReply(command.promised());
case Success:
- // TODO: we don't need to calculate deps if executeAt == txnId
+ // TODO (desirable, efficiency): we don't need to calculate
deps if executeAt == txnId
return new AcceptReply(calculatePartialDeps(safeStore, txnId,
keys, kind, executeAt, safeStore.ranges().between(minEpoch, executeAt.epoch)));
}
}
diff --git a/accord-core/src/main/java/accord/messages/Apply.java
b/accord-core/src/main/java/accord/messages/Apply.java
index c1709ba..8554af2 100644
--- a/accord-core/src/main/java/accord/messages/Apply.java
+++ b/accord-core/src/main/java/accord/messages/Apply.java
@@ -53,7 +53,6 @@ public class Apply extends TxnRequest<ApplyReply>
{
super(to, sendTo, route, txnId);
this.untilEpoch = untilEpoch;
- // TODO: we shouldn't send deps unless we need to (but need to
implement fetching them if they're not present)
Ranges slice = applyTo == sendTo ? scope.covering() :
applyTo.computeRangesForNode(to);
this.deps = deps.slice(slice);
this.executeAt = executeAt;
diff --git a/accord-core/src/main/java/accord/messages/BeginRecovery.java
b/accord-core/src/main/java/accord/messages/BeginRecovery.java
index 8421c63..db8958c 100644
--- a/accord-core/src/main/java/accord/messages/BeginRecovery.java
+++ b/accord-core/src/main/java/accord/messages/BeginRecovery.java
@@ -123,7 +123,7 @@ public class BeginRecovery extends
TxnRequest<BeginRecovery.RecoverReply>
if (!rejectsFastPath)
rejectsFastPath =
committedExecutesAfterWithoutWitnessing(safeStore, txnId, ranges,
partialTxn.keys()).anyMatch(ignore -> true);
- // TODO: introduce some good unit tests for verifying these two
functions in a real repair scenario
+ // TODO (expected, testing): introduce some good unit tests for
verifying these two functions in a real repair scenario
// committed txns with an earlier txnid and have our txnid as a
dependency
earlierCommittedWitness =
committedStartedBeforeAndWitnessed(safeStore, txnId, ranges, partialTxn.keys());
@@ -136,8 +136,8 @@ public class BeginRecovery extends
TxnRequest<BeginRecovery.RecoverReply>
@Override
public RecoverReply reduce(RecoverReply r1, RecoverReply r2)
{
- // TODO: should not operate on dependencies directly here, as we only
merge them;
- // should have a cheaply mergeable variant (or should collect
them before merging)
+ // TODO (low priority, efficiency): should not operate on dependencies
directly here, as we only merge them;
+ // want a cheaply mergeable variant
(or should collect them before merging)
if (!r1.isOk()) return r1;
if (!r2.isOk()) return r2;
@@ -213,7 +213,7 @@ public class BeginRecovery extends
TxnRequest<BeginRecovery.RecoverReply>
public static class RecoverOk extends RecoverReply
{
- public final TxnId txnId; // TODO for debugging?
+ public final TxnId txnId; // for debugging
public final Status status;
public final Ballot accepted;
public final Timestamp executeAt;
diff --git a/accord-core/src/main/java/accord/messages/Callback.java
b/accord-core/src/main/java/accord/messages/Callback.java
index 932ad44..69a663f 100644
--- a/accord-core/src/main/java/accord/messages/Callback.java
+++ b/accord-core/src/main/java/accord/messages/Callback.java
@@ -22,7 +22,8 @@ import accord.local.Node.Id;
/**
* Represents some execution for handling responses from messages a node has
sent.
- * TODO: associate a Callback with a CommandShard or other context for
execution (for coordination, usually its home shard)
+ * TODO (expected, efficiency): associate a Callback with a CommandShard or
other context for execution
+ * (for coordination, usually its home shard)
*/
public interface Callback<T>
{
diff --git a/accord-core/src/main/java/accord/messages/Commit.java
b/accord-core/src/main/java/accord/messages/Commit.java
index 74521d0..1d9f47a 100644
--- a/accord-core/src/main/java/accord/messages/Commit.java
+++ b/accord-core/src/main/java/accord/messages/Commit.java
@@ -37,7 +37,6 @@ import accord.topology.Topology;
import static accord.local.Status.Committed;
import static accord.local.Status.Known.DefinitionOnly;
-// TODO: CommitOk responses, so we can send again if no reply received? Or
leave to recovery?
public class Commit extends TxnRequest<ReadNack>
{
public static class SerializerSupport
@@ -59,8 +58,8 @@ public class Commit extends TxnRequest<ReadNack>
public enum Kind { Minimal, Maximal }
- // TODO: cleanup passing of topologies here - maybe fetch them afresh from
Node? Or perhaps introduce well-named
- // classes to represent different topology combinations
+ // TODO (low priority, clarity): cleanup passing of topologies here -
maybe fetch them afresh from Node?
+ // Or perhaps introduce well-named classes
to represent different topology combinations
public Commit(Kind kind, Id to, Topology coordinateTopology, Topologies
topologies, TxnId txnId, Txn txn, FullRoute<?> route, @Nullable Seekables<?, ?>
readScope, Timestamp executeAt, Deps deps, boolean read)
{
super(to, topologies, route, txnId);
@@ -100,8 +99,8 @@ public class Commit extends TxnRequest<ReadNack>
this.read = read;
}
- // TODO (soon): accept Topology not Topologies
- // TODO: do not commit if we're already ready to execute (requires extra
info in Accept responses)
+ // TODO (low priority, clarity): accept Topology not Topologies
+ // TODO (desired, efficiency): do not commit if we're already ready to
execute (requires extra info in Accept responses)
public static void commitMinimalAndRead(Node node, Topologies
executeTopologies, TxnId txnId, Txn txn, FullRoute<?> route, Seekables<?, ?>
readScope, Timestamp executeAt, Deps deps, Set<Id> readSet, Callback<ReadReply>
callback)
{
Topologies allTopologies = executeTopologies;
@@ -144,7 +143,7 @@ public class Commit extends TxnRequest<ReadNack>
node.mapReduceConsumeLocal(this, txnId.epoch, executeAt.epoch, this);
}
- // TODO (soon): do not guard with synchronized; let mapReduceLocal decide
how to enforce mutual exclusivity
+ // TODO (expected, efficiency, clarity): do not guard with synchronized;
let mapReduceLocal decide how to enforce mutual exclusivity
@Override
public synchronized ReadNack apply(SafeCommandStore safeStore)
{
@@ -213,7 +212,7 @@ public class Commit extends TxnRequest<ReadNack>
public static void commitInvalidate(Node node, TxnId txnId,
Unseekables<?, ?> inform, long untilEpoch)
{
- // TODO: this kind of check needs to be inserted in all equivalent
methods
+ // TODO (expected, safety): this kind of check needs to be
inserted in all equivalent methods
Invariants.checkState(untilEpoch >= txnId.epoch);
Invariants.checkState(node.topology().hasEpoch(untilEpoch));
Topologies commitTo = node.topology().preciseEpochs(inform,
txnId.epoch, untilEpoch);
diff --git a/accord-core/src/main/java/accord/messages/Defer.java
b/accord-core/src/main/java/accord/messages/Defer.java
index 07a592b..ede92ba 100644
--- a/accord-core/src/main/java/accord/messages/Defer.java
+++ b/accord-core/src/main/java/accord/messages/Defer.java
@@ -1,25 +1,24 @@
package accord.messages;
-import java.util.BitSet;
import java.util.function.Function;
import accord.local.*;
import accord.local.Status.Known;
import accord.primitives.TxnId;
import accord.utils.Invariants;
+import com.carrotsearch.hppc.IntHashSet;
import static accord.messages.Defer.Ready.Expired;
import static accord.messages.Defer.Ready.No;
import static accord.messages.Defer.Ready.Yes;
-// TODO: use something more efficient? could probably assign each CommandStore
a unique ascending integer and use an int[]
class Defer implements CommandListener
{
public enum Ready { No, Yes, Expired }
final Function<Command, Ready> waitUntil;
final TxnRequest<?> request;
- BitSet waitingOn = new BitSet(); // TODO: move to compressed integer hash
map to permit easier reclamation of ids
+ IntHashSet waitingOn = new IntHashSet(); // TODO (easy): use Agrona when
available
int waitingOnCount;
boolean isDone;
@@ -45,7 +44,7 @@ class Defer implements CommandListener
if (isDone)
throw new IllegalStateException("Recurrent retry of " + request);
- waitingOn.set(commandStore.id());
+ waitingOn.add(commandStore.id());
++waitingOnCount;
command.addListener(this);
}
@@ -59,9 +58,9 @@ class Defer implements CommandListener
if (ready == Expired) return;
int id = safeStore.commandStore().id();
- if (!waitingOn.get(id))
+ if (!waitingOn.contains(id))
throw new IllegalStateException();
- waitingOn.clear(id);
+ waitingOn.remove(id);
if (0 == --waitingOnCount)
{
diff --git a/accord-core/src/main/java/accord/messages/GetDeps.java
b/accord-core/src/main/java/accord/messages/GetDeps.java
index fd62a91..b2f296d 100644
--- a/accord-core/src/main/java/accord/messages/GetDeps.java
+++ b/accord-core/src/main/java/accord/messages/GetDeps.java
@@ -50,7 +50,6 @@ public class GetDeps extends
TxnRequest.WithUnsynced<PartialDeps>
@Override
public PartialDeps apply(SafeCommandStore instance)
{
- // TODO: shrink ranges to those that intersect key
Ranges ranges = instance.ranges().between(minEpoch, executeAt.epoch);
return calculatePartialDeps(instance, txnId, keys, kind, executeAt,
ranges);
}
diff --git a/accord-core/src/main/java/accord/messages/InformDurable.java
b/accord-core/src/main/java/accord/messages/InformDurable.java
index 3631003..da6123a 100644
--- a/accord-core/src/main/java/accord/messages/InformDurable.java
+++ b/accord-core/src/main/java/accord/messages/InformDurable.java
@@ -52,8 +52,8 @@ public class InformDurable extends TxnRequest<Reply>
implements PreLoadContext
{
// we need to pick a progress log, but this node might not have
participated in the coordination epoch
// in this rare circumstance we simply pick a key to select some
progress log to coordinate this
- // TODO (now): We might not replicate either txnId.epoch OR
executeAt.epoch, but some inbetween.
- // Do we need to receive this message in that case? If
so, we need to account for this when selecting a progress key
+ // TODO (required, consider): We might not replicate either
txnId.epoch OR executeAt.epoch, but some inbetween.
+ // Do we need to receive this message
in that case? If so, we need to account for this when selecting a progress key
at = executeAt;
progressKey = node.selectProgressKey(executeAt.epoch, scope,
scope.homeKey());
shard = Adhoc;
@@ -63,7 +63,7 @@ public class InformDurable extends TxnRequest<Reply>
implements PreLoadContext
shard = scope.homeKey().equals(progressKey) ? Home : Local;
}
- // TODO (soon): do not load from disk to perform this update
+ // TODO (expected, efficiency): do not load from disk to perform this
update
node.mapReduceConsumeLocal(contextFor(txnId), progressKey, at.epoch,
this);
}
diff --git a/accord-core/src/main/java/accord/messages/InformHomeDurable.java
b/accord-core/src/main/java/accord/messages/InformHomeDurable.java
index cf7f611..a263cc7 100644
--- a/accord-core/src/main/java/accord/messages/InformHomeDurable.java
+++ b/accord-core/src/main/java/accord/messages/InformHomeDurable.java
@@ -31,7 +31,7 @@ public class InformHomeDurable implements Request
public void process(Node node, Id replyToNode, ReplyContext replyContext)
{
- // TODO (soon): do not load txnId first
+ // TODO (expected, efficiency): do not load txnId first
node.ifLocal(contextFor(txnId), homeKey, txnId.epoch, safeStore -> {
Command command = safeStore.command(txnId);
command.setDurability(safeStore, durability, homeKey, executeAt);
diff --git a/accord-core/src/main/java/accord/messages/InformOfTxnId.java
b/accord-core/src/main/java/accord/messages/InformOfTxnId.java
index bfaea3b..a11b0db 100644
--- a/accord-core/src/main/java/accord/messages/InformOfTxnId.java
+++ b/accord-core/src/main/java/accord/messages/InformOfTxnId.java
@@ -24,7 +24,7 @@ public class InformOfTxnId extends
AbstractEpochRequest<Reply> implements Reques
public void process()
{
- // TODO (soon): do not first load txnId
+ // TODO (expected, efficiency): do not first load txnId
node.mapReduceConsumeLocal(this, homeKey, txnId.epoch, this);
}
@@ -70,8 +70,8 @@ public class InformOfTxnId extends
AbstractEpochRequest<Reply> implements Reques
@Override
public Iterable<TxnId> txnIds()
{
- // TODO (soon): should be empty list, as can be written without
existing state
- // (though perhaps might check existing in-memory state
in case already present)
+ // TODO (expected, efficiency): should be empty list, as can be
written without existing state
+ // (though perhaps might check existing
in-memory state in case already present)
return Collections.singleton(txnId);
}
}
diff --git a/accord-core/src/main/java/accord/messages/PreAccept.java
b/accord-core/src/main/java/accord/messages/PreAccept.java
index 0a694b8..576797e 100644
--- a/accord-core/src/main/java/accord/messages/PreAccept.java
+++ b/accord-core/src/main/java/accord/messages/PreAccept.java
@@ -123,7 +123,7 @@ public class PreAccept extends
WithUnsynced<PreAccept.PreAcceptReply>
@Override
public void accept(PreAcceptReply reply, Throwable failure)
{
- // TODO: communicate back the failure
+ // TODO (required, error handling): communicate back the failure
node.reply(replyTo, replyContext, reply);
}
diff --git a/accord-core/src/main/java/accord/messages/ReadData.java
b/accord-core/src/main/java/accord/messages/ReadData.java
index e003bca..9471c8d 100644
--- a/accord-core/src/main/java/accord/messages/ReadData.java
+++ b/accord-core/src/main/java/accord/messages/ReadData.java
@@ -29,7 +29,6 @@ import accord.local.*;
import accord.api.Data;
import accord.topology.Topologies;
import com.google.common.base.Preconditions;
-import com.google.common.collect.Iterables;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -42,7 +41,7 @@ import static accord.messages.ReadData.ReadNack.Redundant;
import static accord.messages.TxnRequest.*;
import static accord.utils.MapReduceConsume.forEach;
-// TODO (soon): dedup - can currently have infinite pending reads that will be
executed independently
+// TODO (required, efficiency): dedup - can currently have infinite pending
reads that will be executed independently
public class ReadData extends AbstractEpochRequest<ReadData.ReadNack>
implements CommandListener
{
private static final Logger logger =
LoggerFactory.getLogger(ReadData.class);
@@ -56,10 +55,10 @@ public class ReadData extends
AbstractEpochRequest<ReadData.ReadNack> implements
}
public final long executeAtEpoch;
- public final Seekables<?, ?> readScope; // TODO: this should be
RoutingKeys as we have the Keys locally - but for simplicity for now we use
Keys to implement keys()
+ public final Seekables<?, ?> readScope; // TODO (low priority,
efficiency): this should be RoutingKeys, as we have the Keys locally, but for
simplicity we use this to implement keys()
private final long waitForEpoch;
private Data data;
- private transient boolean isObsolete; // TODO: respond with the Executed
result we have stored?
+ private transient boolean isObsolete; // TODO (low priority, semantics):
respond with the Executed result we have stored?
private transient BitSet waitingOn;
private transient int waitingOnCount;
@@ -193,10 +192,11 @@ public class ReadData extends
AbstractEpochRequest<ReadData.ReadNack> implements
}
else if (failure != null)
{
- // TODO (soon): test
+ // TODO (expected, testing): test
node.reply(replyTo, replyContext, ReadNack.Error);
data = null;
- node.agent().onUncaughtException(failure); // TODO: probably a
better way to handle this, as might not be uncaught
+ // TODO (expected, exceptions): probably a better way to handle
this, as might not be uncaught
+ node.agent().onUncaughtException(failure);
node.commandStores().mapReduceConsume(this, waitingOn.stream(),
forEach(in -> in.command(txnId).removeListener(this), node.agent()));
}
else
@@ -229,7 +229,7 @@ public class ReadData extends
AbstractEpochRequest<ReadData.ReadNack> implements
command.read(safeStore).addCallback((next, throwable) -> {
if (throwable != null)
{
- // TODO (now): exception integration (non-trivial, as might be
handled)
+ // TODO (expected, exceptions): should send exception to
client, and consistency handle/propagate locally
logger.trace("{}: read failed for {}: {}", txnId, unsafeStore,
throwable);
node.reply(replyTo, replyContext, ReadNack.Error);
}
diff --git a/accord-core/src/main/java/accord/messages/TxnRequest.java
b/accord-core/src/main/java/accord/messages/TxnRequest.java
index c34fa05..b80968d 100644
--- a/accord-core/src/main/java/accord/messages/TxnRequest.java
+++ b/accord-core/src/main/java/accord/messages/TxnRequest.java
@@ -42,7 +42,7 @@ public abstract class TxnRequest<R> implements Request,
PreLoadContext, MapReduc
{
public static abstract class WithUnsynced<R> extends TxnRequest<R>
{
- public final long minEpoch; // TODO: can this just always be
TxnId.epoch?
+ public final long minEpoch; // TODO (low priority, clarity): can this
just always be TxnId.epoch?
public final boolean doNotComputeProgressKey;
public WithUnsynced(Id to, Topologies topologies, TxnId txnId,
FullRoute<?> route)
@@ -56,20 +56,20 @@ public abstract class TxnRequest<R> implements Request,
PreLoadContext, MapReduc
this.minEpoch = topologies.oldestEpoch();
this.doNotComputeProgressKey = doNotComputeProgressKey(topologies,
startIndex, txnId, waitForEpoch());
- // TODO (now): alongside Invariants class, introduce PARANOID mode
for checking extra invariants
Ranges ranges = topologies.forEpoch(txnId.epoch).rangesForNode(to);
if (doNotComputeProgressKey)
{
Invariants.checkState(!route.intersects(ranges)); // confirm
dest is not a replica on txnId.epoch
}
- else
+ else if (Invariants.isParanoid())
{
- boolean intersects = route.intersects(ranges);
+ // check that the destination's newer topology does not yield
different ranges
long progressEpoch = Math.min(waitForEpoch(), txnId.epoch);
Ranges computesRangesOn =
topologies.forEpoch(progressEpoch).rangesForNode(to);
- boolean check = computesRangesOn != null &&
route.intersects(computesRangesOn);
- if (check != intersects)
- throw new IllegalStateException();
+ if (computesRangesOn == null)
+ Invariants.checkState(!route.intersects(ranges));
+ else
+
Invariants.checkState(route.slice(computesRangesOn).equals(route.slice(ranges)));
}
}
@@ -140,7 +140,7 @@ public abstract class TxnRequest<R> implements Request,
PreLoadContext, MapReduc
this.node = on;
this.replyTo = replyTo;
this.replyContext = replyContext;
- this.progressKey = progressKey(node); // TODO: not every class that
extends TxnRequest needs this set
+ this.progressKey = progressKey(node); // TODO (low priority, clarity):
not every class that extends TxnRequest needs this set
process();
}
@@ -231,7 +231,7 @@ public abstract class TxnRequest<R> implements Request,
PreLoadContext, MapReduc
return computeScope(node, topologies, route, startIndex, Route::slice,
PartialRoute::union);
}
- // TODO: move to Topologies
+ // TODO (low priority, clarity): move to Topologies
public static <I, O> O computeScope(Node.Id node, Topologies topologies, I
keys, int startIndex, BiFunction<I, Ranges, O> slice, BiFunction<O, O, O> merge)
{
O scope = computeScopeInternal(node, topologies, keys, startIndex,
slice, merge);
@@ -275,7 +275,7 @@ public abstract class TxnRequest<R> implements Request,
PreLoadContext, MapReduc
// So in these cases we send a special flag indicating that the
progress key should not be computed
// (as it might be done so with stale ring information)
- // TODO (soon): this would be better defined as "hasProgressKey"
+ // TODO (low priority, clarity): this would be better defined as
"hasProgressKey"
return waitForEpoch < txnId.epoch && startIndex > 0
&& topologies.get(startIndex).epoch() < txnId.epoch;
}
diff --git a/accord-core/src/main/java/accord/primitives/AbstractKeys.java
b/accord-core/src/main/java/accord/primitives/AbstractKeys.java
index 6886848..b777696 100644
--- a/accord-core/src/main/java/accord/primitives/AbstractKeys.java
+++ b/accord-core/src/main/java/accord/primitives/AbstractKeys.java
@@ -16,7 +16,7 @@ import net.nicoulaj.compilecommand.annotations.Inline;
import static accord.primitives.Routable.Kind.Key;
@SuppressWarnings("rawtypes")
-// TODO: check that foldl call-sites are inlined and optimised by HotSpot
+// TODO (desired, efficiency): check that foldl call-sites are inlined and
optimised by HotSpot
public abstract class AbstractKeys<K extends RoutableKey, KS extends
Routables<K, ?>> implements Iterable<K>, Routables<K, KS>
{
final K[] keys;
@@ -72,9 +72,8 @@ public abstract class AbstractKeys<K extends RoutableKey, KS
extends Routables<K
}
@Override
- public boolean intersects(AbstractRanges<?> ranges)
+ public final boolean intersects(AbstractRanges<?> ranges)
{
- // TODO (now): make this final
return ranges.intersects(this);
}
@@ -144,7 +143,7 @@ public abstract class AbstractKeys<K extends RoutableKey,
KS extends Routables<K
return stream().map(Object::toString).collect(Collectors.joining(",",
"[", "]"));
}
- // TODO (now): accept cached buffers
+ // TODO (expected, efficiency): accept cached buffers
protected K[] slice(Ranges ranges, IntFunction<K[]> factory)
{
return SortedArrays.sliceWithMultipleMatches(keys, ranges.ranges,
factory, (k, r) -> -r.compareTo(k), Range::compareTo);
diff --git a/accord-core/src/main/java/accord/primitives/AbstractRanges.java
b/accord-core/src/main/java/accord/primitives/AbstractRanges.java
index 44e1220..a87adce 100644
--- a/accord-core/src/main/java/accord/primitives/AbstractRanges.java
+++ b/accord-core/src/main/java/accord/primitives/AbstractRanges.java
@@ -230,7 +230,7 @@ public abstract class AbstractRanges<RS extends
Routables<Range, ?>> implements
* terminating at the first indexes where this ceases to be true
* @return index of {@code as} in upper 32bits, {@code bs} in lower 32bits
*
- * TODO: better support for merging runs of overlapping or adjacent ranges
+ * TODO (low priority, efficiency): better support for merging runs of
overlapping or adjacent ranges
*/
static long supersetLinearMerge(Range[] as, Range[] bs)
{
@@ -262,7 +262,7 @@ public abstract class AbstractRanges<RS extends
Routables<Range, ?>> implements
{
// use a temporary counter, so that if we don't find a run of
ranges that enforce the superset
// condition we exit at the start of the mismatch run (and
permit it to be merged)
- // TODO: use exponentialSearch
+ // TODO (easy, efficiency): use exponentialSearch
int tmpai = ai;
do
{
@@ -313,7 +313,7 @@ public abstract class AbstractRanges<RS extends
Routables<Range, ?>> implements
if (bi == bs.length)
return constructor.construct(param1, param2, (as == left.ranges ?
left : right).ranges);
- // TODO (now): caching
+ // TODO (expected, efficiency): ArrayBuffers caching
Range[] result = new Range[as.length + (bs.length - bi)];
int resultCount;
switch (mode)
@@ -345,7 +345,7 @@ public abstract class AbstractRanges<RS extends
Routables<Range, ?>> implements
}
else
{
- // TODO: we don't seem to currently merge adjacent (but
non-overlapping)
+ // TODO (desired, efficiency/semantics): we don't seem to
currently merge adjacent (but non-overlapping)
RoutingKey start = a.start().compareTo(b.start()) <= 0 ?
a.start() : b.start();
RoutingKey end = a.end().compareTo(b.end()) >= 0 ? a.end() :
b.end();
ai++;
@@ -411,7 +411,6 @@ public abstract class AbstractRanges<RS extends
Routables<Range, ?>> implements
if (ranges.length == 0)
return input;
- // TODO: use cache
ObjectBuffers<Range> cachedKeyRanges = cachedRanges();
Range[] buffer = cachedKeyRanges.get(ranges.length);
try
diff --git a/accord-core/src/main/java/accord/primitives/Deps.java
b/accord-core/src/main/java/accord/primitives/Deps.java
index ff0f7a0..f72b33b 100644
--- a/accord-core/src/main/java/accord/primitives/Deps.java
+++ b/accord-core/src/main/java/accord/primitives/Deps.java
@@ -40,7 +40,7 @@ import static accord.utils.SortedArrays.Search.FAST;
* A collection of dependencies for a transaction, organised by the key the
dependency is adopted via.
* An inverse map from TxnId to Key may also be constructed and stored in this
collection.
*/
-// TODO: switch to RoutingKey? Would mean adopting execution dependencies less
precisely
+// TODO (desired, consider): switch to RoutingKey? Would mean adopting
execution dependencies less precisely, but saving ser/deser of large keys
public class Deps implements Iterable<Map.Entry<Key, TxnId>>
{
private static final boolean DEBUG_CHECKS = true;
@@ -84,7 +84,7 @@ public class Deps implements Iterable<Map.Entry<Key, TxnId>>
return new OrderedBuilder(hasOrderedTxnId);
}
- // TODO: cache this object to reduce setup/teardown and allocation
+ // TODO (expected, efficiency): cache this object per thread
public static abstract class AbstractOrderedBuilder<T extends Deps>
implements AutoCloseable
{
final ObjectBuffers<TxnId> cachedTxnIds = cachedTxnIds();
@@ -152,7 +152,7 @@ public class Deps implements Iterable<Map.Entry<Key, TxnId>>
if (totalCount != keyOffset && !hasOrderedTxnId)
{
- // TODO: this allocates a significant amount of memory: would
be preferable to be able to sort using a pre-defined scratch buffer
+ // TODO (low priority, efficiency): this allocates a
significant amount of memory: would be preferable to be able to sort using a
pre-defined scratch buffer
Arrays.sort(keyToTxnId, keyOffset, totalCount);
for (int i = keyOffset + 1 ; i < totalCount ; ++i)
{
@@ -411,7 +411,7 @@ public class Deps implements Iterable<Map.Entry<Key, TxnId>>
}
final Keys keys; // unique Keys
- final TxnId[] txnIds; // unique TxnId TODO: this should perhaps be a BTree?
+ final TxnId[] txnIds; // unique TxnId TODO (low priority, efficiency):
this could be a BTree?
/**
* This represents a map of {@code Key -> [TxnId] } where each TxnId is
actually a pointer into the txnIds array.
@@ -445,7 +445,6 @@ public class Deps implements Iterable<Map.Entry<Key, TxnId>>
checkValid();
}
- // TODO: offer option of computing the maximal KeyRanges that covers the
same set of keys as covered by the parameter
public PartialDeps slice(Ranges ranges)
{
if (isEmpty())
@@ -594,9 +593,9 @@ public class Deps implements Iterable<Map.Entry<Key, TxnId>>
return true;
}
- // TODO: this method supports merging keyToTxnId OR txnIdToKey; we can
perhaps save time and effort when constructing
- // Deps on remote hosts by only producing txnIdToKey with
OrderedCollector and serializing only this,
- // and merging on the recipient before inverting, so that we only
have to invert the final assembled deps
+ // TODO (low priority, efficiency): this method supports merging
keyToTxnId OR txnIdToKey; we can perhaps save time
+ // and effort when constructing Deps on remote hosts by only producing
txnIdToKey with OrderedCollector and serializing
+ // only this, and merging on the recipient before inverting, so that we
only have to invert the final assembled deps
private static <K extends Comparable<? super K>, V extends Comparable<?
super V>, T>
T linearUnion(K[] leftKeys, int leftKeysLength, V[] leftValues, int
leftValuesLength, int[] left, int leftLength,
K[] rightKeys, int rightKeysLength, V[] rightValues, int
rightValuesLength, int[] right, int rightLength,
@@ -609,7 +608,6 @@ public class Deps implements Iterable<Map.Entry<Key, TxnId>>
try
{
- // TODO: this is a little clunky for getting back the buffer and
its length
outKeys = SortedArrays.linearUnion(leftKeys, leftKeysLength,
rightKeys, rightKeysLength, keyBuffers);
outKeysLength = keyBuffers.lengthOfLast(outKeys);
outValues = SortedArrays.linearUnion(leftValues, leftValuesLength,
rightValues, rightValuesLength, valueBuffers);
@@ -878,7 +876,6 @@ public class Deps implements Iterable<Map.Entry<Key, TxnId>>
return result;
}
- // TODO: optimise for case where none removed
public Deps without(Predicate<TxnId> remove)
{
if (isEmpty())
@@ -1077,8 +1074,8 @@ public class Deps implements Iterable<Map.Entry<Key,
TxnId>>
// Find all keys within the ranges, but record existence within an
int64 bitset. Since the bitset is limited
// to 64, this search must be called multiple times searching for
different TxnIds in txnIds; this also has
// the property that forEach is called in TxnId order.
- //TODO Should TxnId order be part of the public docs or just a hidden
implementation detail? The only caller
- // does not rely on this ordering.
+ //TODO (expected, efficiency): reconsider this, probably not worth
trying to save allocations at cost of multiple loop
+ // use BitSet, or perhaps extend so we can
have no nested allocations when few bits
for (int offset = 0 ; offset < txnIds.length ; offset += 64)
{
long bitset = Routables.foldl(keys, ranges, (key, off, value,
keyIndex) -> {
@@ -1086,7 +1083,7 @@ public class Deps implements Iterable<Map.Entry<Key,
TxnId>>
int end = endOffset(keyIndex);
if (off > 0)
{
- // TODO: interpolation search probably great here
+ // TODO (low priority, efficiency): interpolation search
probably great here
index = Arrays.binarySearch(keyToTxnId, index, end,
(int)off);
if (index < 0)
index = -1 - index;
diff --git a/accord-core/src/main/java/accord/primitives/FullKeyRoute.java
b/accord-core/src/main/java/accord/primitives/FullKeyRoute.java
index cd9f3a9..469ed59 100644
--- a/accord-core/src/main/java/accord/primitives/FullKeyRoute.java
+++ b/accord-core/src/main/java/accord/primitives/FullKeyRoute.java
@@ -30,18 +30,10 @@ public class FullKeyRoute extends KeyRoute implements
FullRoute<RoutingKey>
return true;
}
- @Override
- public boolean intersects(AbstractRanges<?> ranges)
- {
- // TODO (now): remove this in favour of parent implementation -
ambiguous at present
- return true;
- }
-
@Override
public FullKeyRoute with(RoutingKey withKey)
{
Invariants.checkArgument(contains(withKey));
- // TODO (now): remove this in favour of parent implementation -
ambiguous at present
return this;
}
diff --git a/accord-core/src/main/java/accord/primitives/Keys.java
b/accord-core/src/main/java/accord/primitives/Keys.java
index 2d9c7b8..a4f9cee 100644
--- a/accord-core/src/main/java/accord/primitives/Keys.java
+++ b/accord-core/src/main/java/accord/primitives/Keys.java
@@ -27,8 +27,7 @@ import accord.utils.ArrayBuffers.ObjectBuffers;
import static accord.utils.ArrayBuffers.cachedKeys;
-// TODO: this should probably be a BTree
-// TODO: check that foldl call-sites are inlined and optimised by HotSpot
+// TODO (low priority, efficiency): this should probably be a BTree
public class Keys extends AbstractKeys<Key, Keys> implements Seekables<Key,
Keys>
{
public static class SerializationSupport
diff --git a/accord-core/src/main/java/accord/primitives/PartialKeyRoute.java
b/accord-core/src/main/java/accord/primitives/PartialKeyRoute.java
index c964aef..da9143a 100644
--- a/accord-core/src/main/java/accord/primitives/PartialKeyRoute.java
+++ b/accord-core/src/main/java/accord/primitives/PartialKeyRoute.java
@@ -54,13 +54,6 @@ public class PartialKeyRoute extends KeyRoute implements
PartialRoute<RoutingKey
return covering.containsAll(ranges);
}
- @Override
- public boolean intersects(AbstractRanges<?> ranges)
- {
- // TODO (now): remove this in favour of parent implementation -
ambiguous at present
- return ranges.intersects(covering);
- }
-
@Override
public AbstractRoutableKeys<?> with(RoutingKey withKey)
{
diff --git a/accord-core/src/main/java/accord/primitives/PartialRangeRoute.java
b/accord-core/src/main/java/accord/primitives/PartialRangeRoute.java
index 1cad856..42a9aac 100644
--- a/accord-core/src/main/java/accord/primitives/PartialRangeRoute.java
+++ b/accord-core/src/main/java/accord/primitives/PartialRangeRoute.java
@@ -61,7 +61,6 @@ public class PartialRangeRoute extends RangeRoute implements
PartialRoute<Range>
@Override
public Unseekables<Range, ?> toMaximalUnseekables()
{
- // TODO (now)
throw new UnsupportedOperationException();
}
@@ -75,7 +74,6 @@ public class PartialRangeRoute extends RangeRoute implements
PartialRoute<Range>
public Unseekables<Range, ?> with(RoutingKey withKey)
{
- // TODO: this is left unimplemented until we actually have a range
transaction to decide how best to address it
throw new UnsupportedOperationException();
}
diff --git a/accord-core/src/main/java/accord/primitives/PartialTxn.java
b/accord-core/src/main/java/accord/primitives/PartialTxn.java
index 3da4715..f9588b1 100644
--- a/accord-core/src/main/java/accord/primitives/PartialTxn.java
+++ b/accord-core/src/main/java/accord/primitives/PartialTxn.java
@@ -9,7 +9,7 @@ import accord.api.Update;
public interface PartialTxn extends Txn
{
Ranges covering();
- // TODO: merge efficient merge when more than one input
+ // TODO (low priority, efficiency): efficient merge when more than one
input
PartialTxn with(PartialTxn add);
Txn reconstitute(FullRoute<?> route);
PartialTxn reconstitutePartial(PartialRoute<?> route);
@@ -28,12 +28,12 @@ public interface PartialTxn extends Txn
return covering().containsAll(unseekables);
}
- // TODO: override toString
static PartialTxn merge(@Nullable PartialTxn a, @Nullable PartialTxn b)
{
return a == null ? b : b == null ? a : a.with(b);
}
+ // TODO (low priority, clarity): override toString
class InMemory extends Txn.InMemory implements PartialTxn
{
public final Ranges covering;
@@ -50,7 +50,6 @@ public interface PartialTxn extends Txn
return covering;
}
- // TODO: merge efficient merge when more than one input
public PartialTxn with(PartialTxn add)
{
if (!add.kind().equals(kind()))
@@ -74,8 +73,6 @@ public interface PartialTxn extends Txn
return new PartialTxn.InMemory(covering, kind(), keys, read,
query, update);
}
- // TODO: override toString
-
public boolean covers(Ranges ranges)
{
return covering.containsAll(ranges);
diff --git a/accord-core/src/main/java/accord/primitives/ProgressToken.java
b/accord-core/src/main/java/accord/primitives/ProgressToken.java
index 02a4646..3f3b05a 100644
--- a/accord-core/src/main/java/accord/primitives/ProgressToken.java
+++ b/accord-core/src/main/java/accord/primitives/ProgressToken.java
@@ -50,7 +50,6 @@ public class ProgressToken implements
Comparable<ProgressToken>, Outcome
{
Durability durability = this.durability.compareTo(that.durability) >=
0 ? this.durability : that.durability;
Status status = this.status.compareTo(that.status) >= 0 ? this.status
: that.status;
- // TODO: slightly inefficient
Ballot promised = this.promised.compareTo(that.promised) >= 0 ?
this.promised : that.promised;
boolean isAccepted = (this.isAccepted &&
this.promised.equals(promised)) || (that.isAccepted &&
that.promised.equals(promised));
if (isSame(durability, status, promised, isAccepted))
diff --git a/accord-core/src/main/java/accord/primitives/RoutableKey.java
b/accord-core/src/main/java/accord/primitives/RoutableKey.java
index b6ba11b..4e90fe0 100644
--- a/accord-core/src/main/java/accord/primitives/RoutableKey.java
+++ b/accord-core/src/main/java/accord/primitives/RoutableKey.java
@@ -12,7 +12,7 @@ public interface RoutableKey extends Routable,
Comparable<RoutableKey>
*
* All RoutingKey implementations must sort correctly with this type.
*
- * TODO: need to partition range from/to -/+ infinity as otherwise we
exclude at least one key
+ * TODO (expected, testing): need to partition range from/to -/+ infinity
as otherwise we exclude at least one key
*/
class InfiniteRoutableKey implements RoutableKey
{
diff --git a/accord-core/src/main/java/accord/topology/Shard.java
b/accord-core/src/main/java/accord/topology/Shard.java
index c749d40..bb22017 100644
--- a/accord-core/src/main/java/accord/topology/Shard.java
+++ b/accord-core/src/main/java/accord/topology/Shard.java
@@ -33,11 +33,11 @@ import com.google.common.collect.Iterables;
import static accord.utils.Invariants.checkArgument;
-// TODO: concept of region/locality
+// TODO (expected, efficiency): concept of region/locality
public class Shard
{
public final Range range;
- // TODO: use BTreeSet to combine these two (or introduce version that
operates over long values)
+ // TODO (desired, clarity): use BTreeSet to combine these two (or
introduce version that operates over long values)
public final List<Id> nodes;
public final Set<Id> nodeSet;
public final Set<Id> fastPathElectorate;
diff --git a/accord-core/src/main/java/accord/topology/Topologies.java
b/accord-core/src/main/java/accord/topology/Topologies.java
index f799193..13a00ad 100644
--- a/accord-core/src/main/java/accord/topology/Topologies.java
+++ b/accord-core/src/main/java/accord/topology/Topologies.java
@@ -28,8 +28,8 @@ import accord.utils.Invariants;
import java.util.*;
-// TODO: we can probably most efficiently create a new synthetic Topology that
applies for a range of epochs
-// and permit Topology to implement it, so that
+// TODO (desired, efficiency/clarity): since Topologies are rarely needed,
should optimise API for single topology case
+// (e.g. at least implementing Topologies by Topology)
public interface Topologies extends TopologySorter
{
Topology current();
diff --git a/accord-core/src/main/java/accord/topology/Topology.java
b/accord-core/src/main/java/accord/topology/Topology.java
index 277446f..a502bd4 100644
--- a/accord-core/src/main/java/accord/topology/Topology.java
+++ b/accord-core/src/main/java/accord/topology/Topology.java
@@ -38,12 +38,14 @@ public class Topology
final long epoch;
final Shard[] shards;
final Ranges ranges;
+ /**
+ * TODO (desired, efficiency): do not recompute nodeLookup for
sub-topologies
+ */
final Map<Id, NodeInfo> nodeLookup;
/**
* This array is used to permit cheaper sharing of Topology objects
between requests, as we must only specify
* the indexes within the parent Topology that we contain. This also
permits us to perform efficient merges with
* {@code NodeInfo.supersetIndexes} to find the shards that intersect a
given node without recomputing the NodeInfo.
- * TODO: do not recompute nodeLookup
*/
final Ranges subsetOfRanges;
final int[] supersetIndexes;
@@ -215,7 +217,6 @@ public class Topology
{
Ranges rangeSubset = ranges.select(newSubset);
- // TODO: more efficient sharing of nodeLookup state
Map<Id, NodeInfo> nodeLookup = new HashMap<>();
for (int shardIndex : newSubset)
{
@@ -229,8 +230,6 @@ public class Topology
private Topology forSubset(int[] newSubset, Collection<Id> nodes)
{
Ranges rangeSubset = ranges.select(newSubset);
-
- // TODO: more efficient sharing of nodeLookup state
Map<Id, NodeInfo> nodeLookup = new HashMap<>();
for (Id id : nodes)
nodeLookup.put(id, this.nodeLookup.get(id));
@@ -395,40 +394,9 @@ public class Topology
return initialValue;
}
- public int matchesOn(Id on, IndexedPredicate<Shard> consumer)
- {
- // TODO: this can be done by divide-and-conquer splitting of the lists
and recursion, which should be more efficient
- int count = 0;
- NodeInfo info = nodeLookup.get(on);
- if (info == null)
- return 0;
- int[] a = supersetIndexes, b = info.supersetIndexes;
- int ai = 0, bi = 0;
- while (ai < a.length && bi < b.length)
- {
- if (a[ai] == b[bi])
- {
- if (consumer.test(shards[a[ai]], ai))
- ++count;
- ++ai; ++bi;
- }
- else if (a[ai] < b[bi])
- {
- ai = exponentialSearch(a, ai + 1, a.length, b[bi]);
- if (ai < 0) ai = -1 -ai;
- }
- else
- {
- bi = exponentialSearch(b, bi + 1, b.length, a[ai]);
- if (bi < 0) bi = -1 -bi;
- }
- }
- return count;
- }
-
public <P> int foldlIntOn(Id on, IndexedIntFunction<P> consumer, P param,
int offset, int initialValue, int terminalValue)
{
- // TODO: this can be done by divide-and-conquer splitting of the lists
and recursion, which should be more efficient
+ // TODO (low priority, efficiency/clarity): use findNextIntersection?
NodeInfo info = nodeLookup.get(on);
if (info == null)
return initialValue;
diff --git a/accord-core/src/main/java/accord/topology/TopologyManager.java
b/accord-core/src/main/java/accord/topology/TopologyManager.java
index ec1061f..38d6589 100644
--- a/accord-core/src/main/java/accord/topology/TopologyManager.java
+++ b/accord-core/src/main/java/accord/topology/TopologyManager.java
@@ -48,8 +48,8 @@ import static accord.utils.Invariants.checkArgument;
*
* Assumes a topology service that won't report epoch n without having n-1 etc
also available
*
- * TODO: make TopologyManager a Topologies and copy-on-write update to it, so
we can always just take a reference for
- * transactions instead of copying every time (and index into it by the
txnId.epoch)
+ * TODO (desired, efficiency/clarity): make TopologyManager a Topologies and
copy-on-write update to it,
+ * so we can always just take a reference for transactions instead of copying
every time (and index into it by the txnId.epoch)
*/
public class TopologyManager implements ConfigurationService.Listener
{
diff --git a/accord-core/src/main/java/accord/utils/ArrayBuffers.java
b/accord-core/src/main/java/accord/utils/ArrayBuffers.java
index 2dce966..d1ad72a 100644
--- a/accord-core/src/main/java/accord/utils/ArrayBuffers.java
+++ b/accord-core/src/main/java/accord/utils/ArrayBuffers.java
@@ -51,7 +51,7 @@ public class ArrayBuffers
{
private static final boolean FULLY_UNCACHED = true;
- // TODO: we should periodically clear the thread locals to ensure we
aren't slowly accumulating unnecessarily large objects on every thread
+ // TODO (low priority, efficiency): we should periodically clear the
thread locals to ensure we aren't slowly accumulating unnecessarily large
objects on every thread
private static final ThreadLocal<IntBufferCache> INTS =
ThreadLocal.withInitial(() -> new IntBufferCache(4, 1 << 14));
private static final ThreadLocal<ObjectBufferCache<Key>> KEYS =
ThreadLocal.withInitial(() -> new ObjectBufferCache<>(3, 1 << 9, Key[]::new));
private static final ThreadLocal<ObjectBufferCache<RoutingKey>>
ROUTINGKEYS = ThreadLocal.withInitial(() -> new ObjectBufferCache<>(3, 1 << 9,
RoutingKey[]::new));
diff --git
a/accord-core/src/main/java/accord/utils/DeterministicIdentitySet.java
b/accord-core/src/main/java/accord/utils/DeterministicIdentitySet.java
index a8208a1..e380e39 100644
--- a/accord-core/src/main/java/accord/utils/DeterministicIdentitySet.java
+++ b/accord-core/src/main/java/accord/utils/DeterministicIdentitySet.java
@@ -38,7 +38,7 @@ public class DeterministicIdentitySet<T> extends
AbstractSet<T>
}
}
- // TODO: an identity hash map that doesn't mind concurrent modification /
iteration
+ // TODO (low priority): an identity hash map that doesn't mind concurrent
modification / iteration
final IdentityHashMap<T, Entry<T>> lookup;
final Entry<T> head = new Entry<T>(null);
diff --git a/accord-core/src/main/java/accord/utils/IndexedTriConsumer.java
b/accord-core/src/main/java/accord/utils/IndexedTriConsumer.java
index 418c69c..4400ff8 100644
--- a/accord-core/src/main/java/accord/utils/IndexedTriConsumer.java
+++ b/accord-core/src/main/java/accord/utils/IndexedTriConsumer.java
@@ -1,6 +1,5 @@
package accord.utils;
-// TODO (now): migrate to utils, but must standardise on parameter order with
index last
public interface IndexedTriConsumer<P1, P2, P3>
{
void accept(P1 p1, P2 p2, P3 p3, int index);
diff --git a/accord-core/src/main/java/accord/utils/IntrusiveLinkedList.java
b/accord-core/src/main/java/accord/utils/IntrusiveLinkedList.java
index eadcb92..8b822e9 100644
--- a/accord-core/src/main/java/accord/utils/IntrusiveLinkedList.java
+++ b/accord-core/src/main/java/accord/utils/IntrusiveLinkedList.java
@@ -29,7 +29,7 @@ import static java.util.Spliterators.spliteratorUnknownSize;
* A simple intrusive double-linked list for maintaining a list of tasks,
* useful for invalidating queued ordered tasks
*
- * TODO COPIED FROM CASSANDRA
+ * TODO (low priority): COPIED FROM CASSANDRA
*/
@SuppressWarnings("unchecked")
diff --git
a/accord-core/src/main/java/accord/utils/IntrusiveLinkedListNode.java
b/accord-core/src/main/java/accord/utils/IntrusiveLinkedListNode.java
index 767d262..c180cf1 100644
--- a/accord-core/src/main/java/accord/utils/IntrusiveLinkedListNode.java
+++ b/accord-core/src/main/java/accord/utils/IntrusiveLinkedListNode.java
@@ -19,7 +19,7 @@
package accord.utils;
/**
- * TODO COPIED FROM CASSANDRA
+ * TODO (low priority): COPIED FROM CASSANDRA
*/
public abstract class IntrusiveLinkedListNode
{
diff --git a/accord-core/src/main/java/accord/utils/Invariants.java
b/accord-core/src/main/java/accord/utils/Invariants.java
index 4242c5b..de7f2b0 100644
--- a/accord-core/src/main/java/accord/utils/Invariants.java
+++ b/accord-core/src/main/java/accord/utils/Invariants.java
@@ -8,6 +8,11 @@ public class Invariants
{
private static final boolean PARANOID = true;
+ public static boolean isParanoid()
+ {
+ return PARANOID;
+ }
+
public static <T1, T2 extends T1> T2 checkType(T1 cast)
{
return (T2)cast;
diff --git a/accord-core/src/main/java/accord/utils/MapReduce.java
b/accord-core/src/main/java/accord/utils/MapReduce.java
index 04b0f04..5cc4adc 100644
--- a/accord-core/src/main/java/accord/utils/MapReduce.java
+++ b/accord-core/src/main/java/accord/utils/MapReduce.java
@@ -4,7 +4,7 @@ import java.util.function.Function;
public interface MapReduce<I, O> extends Function<I, O>
{
- // TODO (soon): ensure mutual exclusivity when calling each of these
methods
+ // TODO (desired, safety): ensure mutual exclusivity when calling each of
these methods
O apply(I in);
O reduce(O o1, O o2);
}
diff --git a/accord-core/src/main/java/accord/utils/SortedArrays.java
b/accord-core/src/main/java/accord/utils/SortedArrays.java
index a0aa473..542f8e2 100644
--- a/accord-core/src/main/java/accord/utils/SortedArrays.java
+++ b/accord-core/src/main/java/accord/utils/SortedArrays.java
@@ -56,9 +56,9 @@ public class SortedArrays
*
* Otherwise, depending on {@code buffers}, a result buffer may itself be
returned or a new array.
*
- * TODO: introduce exponential search optimised version
- * TODO: also compare with Hwang and Lin algorithm
- * TODO: could also compare with a recursive partitioning scheme like
quicksort
+ * TODO (low priority, efficiency): introduce exponential search optimised
version
+ * also compare with Hwang and Lin
algorithm
+ * could also compare with a recursive
partitioning scheme like quicksort
* (note that dual exponential search is also an optimal algorithm, just
seemingly ignored by the literature,
* and may be in practice faster for lists that are more often similar in
size, and only occasionally very different.
* Without performing extensive analysis, exponential search likely has
higher constant factors in terms of the
@@ -218,7 +218,7 @@ public class SortedArrays
*
* Otherwise, depending on {@code buffers}, a result buffer may itself be
returned or a new array.
*
- * TODO: introduce exponential search optimised version
+ * TODO (low priority, efficiency): introduce exponential search optimised
version
*/
public static <T> T[] linearIntersection(T[] left, int leftLength, T[]
right, int rightLength, Comparator<T> comparator, ObjectBuffers<T> buffers)
{
@@ -321,7 +321,7 @@ public class SortedArrays
*
* Otherwise, depending on {@code buffers}, a result buffer may itself be
returned or a new array.
*
- * TODO: introduce exponential search optimised version
+ * TODO (low priority, efficiency): introduce exponential search optimised
version
*/
public static <T2, T1 extends Comparable<? super T2>> T1[]
linearIntersection(T1[] left, int leftLength, T2[] right, int rightLength,
ObjectBuffers<T1> buffers)
{
diff --git a/accord-core/src/main/java/accord/utils/Utils.java
b/accord-core/src/main/java/accord/utils/Utils.java
index 6995c7d..0c5ff5a 100644
--- a/accord-core/src/main/java/accord/utils/Utils.java
+++ b/accord-core/src/main/java/accord/utils/Utils.java
@@ -21,7 +21,7 @@ package accord.utils;
import java.util.*;
import java.util.function.IntFunction;
-// TODO: remove when jdk8 support is dropped
+// TODO (low priority): remove when jdk8 support is dropped
public class Utils
{
// reimplements Collection#toArray
diff --git a/accord-core/src/test/java/accord/KeysTest.java
b/accord-core/src/test/java/accord/KeysTest.java
index d7867f2..c8f7650 100644
--- a/accord-core/src/test/java/accord/KeysTest.java
+++ b/accord-core/src/test/java/accord/KeysTest.java
@@ -250,8 +250,6 @@ public class KeysTest
});
}
- //TODO test foldlIntersect
-
private static Gen<List<Raw>> keysGen() {
return Gens.lists(Gens.ints().between(-1000, 1000).map(IntKey::key))
.unique()
diff --git
a/accord-core/src/test/java/accord/burn/BurnTestConfigurationService.java
b/accord-core/src/test/java/accord/burn/BurnTestConfigurationService.java
index 6cbb4d1..c50e4dd 100644
--- a/accord-core/src/test/java/accord/burn/BurnTestConfigurationService.java
+++ b/accord-core/src/test/java/accord/burn/BurnTestConfigurationService.java
@@ -65,7 +65,7 @@ public class BurnTestConfigurationService implements
TestableConfigurationServic
private static class EpochHistory
{
- // TODO: move pendingEpochs / FetchTopology into here?
+ // TODO (low priority): move pendingEpochs / FetchTopology into here?
private final List<EpochState> epochs = new ArrayList<>();
private long lastReceived = 0;
diff --git a/accord-core/src/test/java/accord/burn/TopologyUpdates.java
b/accord-core/src/test/java/accord/burn/TopologyUpdates.java
index d3136bd..61aa6b9 100644
--- a/accord-core/src/test/java/accord/burn/TopologyUpdates.java
+++ b/accord-core/src/test/java/accord/burn/TopologyUpdates.java
@@ -89,7 +89,7 @@ public class TopologyUpdates
if (minStatus == null || minStatus.phase.compareTo(status.phase)
>= 0)
{
- // TODO: minStatus == null means we're sending redundant
messages
+ // TODO (low priority): minStatus == null means we're sending
redundant messages
onDone.accept(true);
return;
}
diff --git
a/accord-core/src/test/java/accord/coordinate/tracking/InvalidationTrackerReconciler.java
b/accord-core/src/test/java/accord/coordinate/tracking/InvalidationTrackerReconciler.java
index e4bcd94..51cf8cb 100644
---
a/accord-core/src/test/java/accord/coordinate/tracking/InvalidationTrackerReconciler.java
+++
b/accord-core/src/test/java/accord/coordinate/tracking/InvalidationTrackerReconciler.java
@@ -8,7 +8,6 @@ import org.junit.jupiter.api.Assertions;
import java.util.ArrayList;
import java.util.Random;
-// TODO: check fast path accounting
public class InvalidationTrackerReconciler extends
TrackerReconciler<InvalidationShardTracker, InvalidationTracker,
InvalidationTrackerReconciler.Rsp>
{
enum Rsp { PROMISED_FAST, NOT_PROMISED_FAST, PROMISED_SLOW,
NOT_PROMISED_SLOW, FAIL }
@@ -59,7 +58,7 @@ public class InvalidationTrackerReconciler extends
TrackerReconciler<Invalidatio
case NoChange:
Assertions.assertFalse(tracker.any(InvalidationShardTracker::isPromised)
&&
tracker.any(InvalidationShardTracker::isFastPathRejected));
- // TODO: it would be nice for InvalidationShardTracker to
respond as soon as no shards are able to promise, but would require significant
refactoring
+ // TODO (low priority): it would be nice for
InvalidationShardTracker to respond as soon as no shards are able to promise,
but would require significant refactoring
//
Assertions.assertTrue(tracker.any(InvalidationShardTracker::canPromise));
Assertions.assertFalse(tracker.all(InvalidationShardTracker::isDecided));
}
diff --git
a/accord-core/src/test/java/accord/coordinate/tracking/RecoveryTrackerReconciler.java
b/accord-core/src/test/java/accord/coordinate/tracking/RecoveryTrackerReconciler.java
index 5cb0fab..30bcdfc 100644
---
a/accord-core/src/test/java/accord/coordinate/tracking/RecoveryTrackerReconciler.java
+++
b/accord-core/src/test/java/accord/coordinate/tracking/RecoveryTrackerReconciler.java
@@ -8,7 +8,7 @@ import org.junit.jupiter.api.Assertions;
import java.util.ArrayList;
import java.util.Random;
-// TODO: check fast path accounting
+// TODO (required, testing): check fast path accounting
public class RecoveryTrackerReconciler extends
TrackerReconciler<RecoveryShardTracker, RecoveryTracker,
RecoveryTrackerReconciler.Rsp>
{
enum Rsp { FAST, SLOW, FAIL }
diff --git
a/accord-core/src/test/java/accord/coordinate/tracking/TrackerReconciler.java
b/accord-core/src/test/java/accord/coordinate/tracking/TrackerReconciler.java
index fc569cf..015d798 100644
---
a/accord-core/src/test/java/accord/coordinate/tracking/TrackerReconciler.java
+++
b/accord-core/src/test/java/accord/coordinate/tracking/TrackerReconciler.java
@@ -76,8 +76,8 @@ public abstract class TrackerReconciler<ST extends
ShardTracker, T extends Abstr
.collect(Collectors.toList());
}
- // TODO: generalise and parameterise topology generation a bit more
- // TODO: select a subset of the generated topologies to correctly simulate
topology consumption logic
+ // TODO (required, testing): generalise and parameterise topology
generation a bit more
+ // also, select a subset of the generated
topologies to correctly simulate topology consumption logic
private static Stream<Topologies> topologies(Random random)
{
TopologyFactory factory = new TopologyFactory(2 + random.nextInt(3),
IntHashKey.ranges(4 + random.nextInt(12)));
diff --git a/accord-core/src/test/java/accord/impl/TopologyFactory.java
b/accord-core/src/test/java/accord/impl/TopologyFactory.java
index baaef2f..10ac252 100644
--- a/accord-core/src/test/java/accord/impl/TopologyFactory.java
+++ b/accord-core/src/test/java/accord/impl/TopologyFactory.java
@@ -31,7 +31,6 @@ import static accord.utils.Utils.toArray;
public class TopologyFactory
{
public final int rf;
- // TODO: convert to KeyRanges
public final Range[] shardRanges;
public TopologyFactory(int rf, Range... shardRanges)
diff --git a/accord-core/src/test/java/accord/impl/basic/Cluster.java
b/accord-core/src/test/java/accord/impl/basic/Cluster.java
index 98ff331..5680c9c 100644
--- a/accord-core/src/test/java/accord/impl/basic/Cluster.java
+++ b/accord-core/src/test/java/accord/impl/basic/Cluster.java
@@ -132,7 +132,7 @@ public class Cluster implements Scheduler
Packet deliver = (Packet) next;
Node on = lookup.apply(deliver.dst);
- // TODO (soon): random drop chance independent of partition; also
port flaky connections etc. from simulator
+ // TODO (required, testing): random drop chance independent of
partition; also port flaky connections etc. from simulator
// Drop the message if it goes across the partition
boolean drop = ((Packet) next).src.id >= 0 &&
!(partitionSet.contains(deliver.src) &&
partitionSet.contains(deliver.dst)
@@ -206,9 +206,6 @@ public class Cluster implements Scheduler
run.run();
}
- // TODO: there may remain some inconsistency of execution, at least
causing different partitions if prior runs have happened;
- // unclear what source is, but less frequence now we split cluster
partitioning recurring task to its own random
- // might be deterministic based on prior runs (some evidence of
this), or non-deterministic
public static void run(Id[] nodes, Supplier<PendingQueue> queueSupplier,
Consumer<Packet> responseSink, Consumer<Throwable> onFailure, Supplier<Random>
randomSupplier, Supplier<LongSupplier> nowSupplier, TopologyFactory
topologyFactory, Supplier<Packet> in)
{
TopologyUpdates topologyUpdates = new TopologyUpdates();
@@ -249,7 +246,7 @@ public class Cluster implements Scheduler
sinks.partitionSet = Collections.emptySet();
// give progress log et al a chance to finish
- // TODO: would be nice to make this more certain than an arbitrary
number of additional rounds
+ // TODO (desired, testing): would be nice to make this more
certain than an arbitrary number of additional rounds
for (int i = 0 ; i < 10 ; ++i)
{
sinks.processAll();
diff --git a/accord-core/src/test/java/accord/impl/list/ListAgent.java
b/accord-core/src/test/java/accord/impl/list/ListAgent.java
index 16fcc2f..158c42b 100644
--- a/accord-core/src/test/java/accord/impl/list/ListAgent.java
+++ b/accord-core/src/test/java/accord/impl/list/ListAgent.java
@@ -58,7 +58,7 @@ public class ListAgent implements Agent
@Override
public void onUncaughtException(Throwable t)
{
- // TODO: ensure reported to runner
+ // TODO (required, testing): ensure reported to runner
onFailure.accept(t);
}
diff --git a/accord-core/src/test/java/accord/impl/list/ListRequest.java
b/accord-core/src/test/java/accord/impl/list/ListRequest.java
index 05d9703..fa8a29e 100644
--- a/accord-core/src/test/java/accord/impl/list/ListRequest.java
+++ b/accord-core/src/test/java/accord/impl/list/ListRequest.java
@@ -102,7 +102,7 @@ public class ListRequest implements Request
@Override
public void accept(Result success, Throwable fail)
{
- // TODO: error handling
+ // TODO (desired, testing): error handling
if (success != null)
{
node.reply(client, replyContext, (ListResult) success);
diff --git a/accord-core/src/test/java/accord/impl/mock/MockCluster.java
b/accord-core/src/test/java/accord/impl/mock/MockCluster.java
index e1c8173..7fe7aed 100644
--- a/accord-core/src/test/java/accord/impl/mock/MockCluster.java
+++ b/accord-core/src/test/java/accord/impl/mock/MockCluster.java
@@ -147,7 +147,7 @@ public class MockCluster implements Network, AutoCloseable,
Iterable<Node>
if (networkFilter.shouldDiscard(from, to, request))
{
- // TODO: more flexible timeouts
+ // TODO (desired, testing): more flexible timeouts
if (callback != null)
callback.onFailure(to, new Timeout(null, null));
logger.info("discarding filtered message from {} to {}: {}", from,
to, request);
diff --git a/accord-core/src/test/java/accord/impl/mock/Network.java
b/accord-core/src/test/java/accord/impl/mock/Network.java
index bed910c..4f64acc 100644
--- a/accord-core/src/test/java/accord/impl/mock/Network.java
+++ b/accord-core/src/test/java/accord/impl/mock/Network.java
@@ -54,13 +54,13 @@ public interface Network
@Override
public void send(Id from, Id to, Request request, Callback callback)
{
- // TODO: log
+ // TODO (easy, testing): log
}
@Override
public void reply(Id from, Id replyingToNode, long replyingToMessage,
Reply reply)
{
- // TODO: log
+ // TODO (easy, testing): log
}
};
}
diff --git a/accord-core/src/test/java/accord/topology/TopologyRandomizer.java
b/accord-core/src/test/java/accord/topology/TopologyRandomizer.java
index 5d29f91..7942979 100644
--- a/accord-core/src/test/java/accord/topology/TopologyRandomizer.java
+++ b/accord-core/src/test/java/accord/topology/TopologyRandomizer.java
@@ -30,7 +30,7 @@ import org.slf4j.LoggerFactory;
import java.util.*;
import java.util.function.*;
-// TODO: add change replication factor
+// TODO (required, testing): add change replication factor
public class TopologyRandomizer
{
private static final Logger logger =
LoggerFactory.getLogger(TopologyRandomizer.class);
@@ -235,7 +235,7 @@ public class TopologyRandomizer
Topology nextTopology = new Topology(current.epoch + 1, shards);
- // FIXME: remove this (and the corresponding check in CommandStores)
once lower bounds are implemented.
+ // TODO (expected, testing): remove this (and the corresponding check
in CommandStores) once lower bounds are implemented.
// In the meantime, the logic needed to support acquiring ranges that
we previously replicated is pretty
// convoluted without the ability to jettison epochs.
if (reassignsRanges(current, shards, previouslyReplicated))
diff --git a/accord-core/src/test/java/accord/txn/DepsTest.java
b/accord-core/src/test/java/accord/txn/DepsTest.java
index 00fbb64..9bd8d52 100644
--- a/accord-core/src/test/java/accord/txn/DepsTest.java
+++ b/accord-core/src/test/java/accord/txn/DepsTest.java
@@ -56,8 +56,8 @@ import static accord.utils.Gens.lists;
import static accord.utils.Property.qt;
import static accord.utils.Utils.toArray;
-// TODO: test Keys with no contents
-// TODO: test #without
+// TODO (expected, testing): test Keys with no contents, "without", "with"
where TxnId and Keys are the same, but Key -> [TxnId] does not match;
+// ensure high code coverage
public class DepsTest
{
private static final Logger logger =
LoggerFactory.getLogger(DepsTest.class);
@@ -270,13 +270,6 @@ public class DepsTest
executor.shutdown();
}
- //TODO test "with" where TxnId and Keys are the same, but Key -> [TxnId]
does not match
-
- private static Keys keys(List<Deps> list)
- {
- return list.stream().map(d -> (Keys)d.test.keys()).reduce(Keys.EMPTY,
Keys::union);
- }
-
private static void testMergedProperty(List<Deps> list)
{
Deps expected = Deps.merge(list);
diff --git
a/accord-core/src/test/java/accord/verify/LinearizabilityVerifier.java
b/accord-core/src/test/java/accord/verify/LinearizabilityVerifier.java
index 1c00ed7..125d416 100644
--- a/accord-core/src/test/java/accord/verify/LinearizabilityVerifier.java
+++ b/accord-core/src/test/java/accord/verify/LinearizabilityVerifier.java
@@ -34,7 +34,7 @@ import static
accord.verify.LinearizabilityVerifier.Witness.Type.UPDATE_UNKNOWN;
* We simply verify that there is no viewing of histories backwards or
forwards in time (i.e. that the time periods each
* unique list is witnessable for is disjoint) and that each list is a prefix
of any lists witnessed later
*
- * TODO: merge with SerializabilityVerifier.
+ * TODO (low priority): merge with SerializabilityVerifier.
*/
public class LinearizabilityVerifier
{
diff --git
a/accord-core/src/test/java/accord/verify/SerializabilityVerifier.java
b/accord-core/src/test/java/accord/verify/SerializabilityVerifier.java
index 00c7dc7..109a974 100644
--- a/accord-core/src/test/java/accord/verify/SerializabilityVerifier.java
+++ b/accord-core/src/test/java/accord/verify/SerializabilityVerifier.java
@@ -43,8 +43,7 @@ import java.util.stream.IntStream;
* That is, we maintain links to the maximum predecessor step for each key, at
each step for each key.
* In combination with a linearizability verifier for each register/partition,
we verify strict-serializability.
*
- * TODO: session serializability, i.e. use each client's sequence of events as
additional happens-before relations
- * TODO: find and report a path when we encounter a violation
+ * TODO (low priority): session serializability, i.e. use each client's
sequence of events as additional happens-before relations
*/
public class SerializabilityVerifier
{
@@ -77,7 +76,6 @@ public class SerializabilityVerifier
final int ofKey;
final int ofStepIndex;
- // TODO: we probably don't need this field, as it's implied by the
node we point to, that we have when we enqueue refresh
final int predecessorKey;
int predecessorStepIndex;
@@ -187,8 +185,6 @@ public class SerializabilityVerifier
/**
* The history of observations for a given key, or the set of nodes in
the graph of observations for this key.
- *
- * TODO: extend LinearizabilityVerifier
*/
class Register
{
@@ -409,7 +405,6 @@ public class SerializabilityVerifier
final int keyCount;
final Register[] registers;
- // TODO: use another intrusive list or intrusive tree
final TreeSet<MaxPredecessor> refresh = new TreeSet<>();
// [key]->the sequence returned by any read performed in this transaction
diff --git
a/accord-core/src/test/java/accord/verify/SerializabilityVerifierTest.java
b/accord-core/src/test/java/accord/verify/SerializabilityVerifierTest.java
index f5310ea..1f85130 100644
--- a/accord-core/src/test/java/accord/verify/SerializabilityVerifierTest.java
+++ b/accord-core/src/test/java/accord/verify/SerializabilityVerifierTest.java
@@ -151,7 +151,7 @@ public class SerializabilityVerifierTest
Observation[][] permuted = new Observation[permute.length][];
for (int offset = 0 ; offset < permute.length ; ++offset)
{
- // TODO: more permutations
+ // TODO (low priority): more permutations
for (int i = 0 ; i < permute.length ; ++i)
permuted[i] = permute[(offset + i) % permute.length];
diff --git
a/accord-core/src/test/java/accord/verify/StrictSerializabilityVerifier.java
b/accord-core/src/test/java/accord/verify/StrictSerializabilityVerifier.java
index 32d26a5..44374c1 100644
--- a/accord-core/src/test/java/accord/verify/StrictSerializabilityVerifier.java
+++ b/accord-core/src/test/java/accord/verify/StrictSerializabilityVerifier.java
@@ -48,7 +48,7 @@ import accord.utils.Invariants;
* That is, we maintain links to the maximum predecessor step for each key, at
each step for each key, and see if we can
* find a path of predecessors that would witness us.
*
- * TODO: find and report a path when we encounter a violation
+ * TODO (low priority): find and report a path when we encounter a violation
*/
public class StrictSerializabilityVerifier
{
@@ -83,7 +83,7 @@ public class StrictSerializabilityVerifier
// the step index we are tracking predecessors for
Step ofStep;
- // TODO: we probably don't need this field, as it's implied by the
node we point to, that we have when we enqueue refresh
+ // TODO (low priority): we probably don't need this field, as it's
implied by the node we point to, that we have when we enqueue refresh
// the key we are tracking the maximum predecessor for
final int predecessorKey;
@@ -223,7 +223,7 @@ public class StrictSerializabilityVerifier
*/
final MaxPredecessor[] maxPredecessors;
- // TODO: cleanup
+ // TODO (low priority): cleanup
List<UnknownStepHolder> unknownStepPeers;
Map<Step, UnknownStepPredecessor> unknownStepPredecessors;
Runnable onChange;
@@ -288,7 +288,7 @@ public class StrictSerializabilityVerifier
{
updated = true;
writtenAfter = start;
- // TODO: double check this will trigger an update of
maxPredecessorX properties on each node with us as a maxPredecessor
+ // TODO (low priority): double check this will trigger an
update of maxPredecessorX properties on each node with us as a maxPredecessor
}
if (writtenAfter > writtenBefore)
throw new HistoryViolation(ofKey, "Write operation time
conflicts with earlier read");
@@ -459,10 +459,7 @@ public class StrictSerializabilityVerifier
* Any write value we don't know the step index for because we did not
perform a coincident read;
* we wait until we witness a read containing the
*
- * TODO: report a violation if we have witnessed a sequence missing
any of these deferred operations
- * that started after they finished
- *
- * TODO: handle writes with unknown outcome
+ * TODO (desired, testing): handle writes with unknown outcome
*/
final Map<Integer, UnknownStepHolder> byWriteValue = new HashMap<>();
@@ -530,7 +527,7 @@ public class StrictSerializabilityVerifier
throw new AssertionError();
if (null != byTimestamp.putIfAbsent(end, unknownStep))
throw new AssertionError();
- // TODO: verify this propagation by unit test
+ // TODO (desired, testing): verify this propagation by unit test
unknownSteps[ofKey].step.receiveKnowledgePhasedPredecessors(this,
StrictSerializabilityVerifier.this);
}
@@ -701,7 +698,7 @@ public class StrictSerializabilityVerifier
final int keyCount;
final Register[] registers;
- // TODO: use another intrusive list or intrusive tree
+ // TODO (low priority): use another intrusive list or intrusive tree
final TreeSet<MaxPredecessor> refresh = new TreeSet<>();
// [key]->the sequence returned by any read performed in this transaction
@@ -712,7 +709,7 @@ public class StrictSerializabilityVerifier
final int[] bufNewPeerSteps;
final UnknownStepHolder[] bufUnknownSteps;
- // TODO (soon): verify operations with unknown outcomes are finalised by
the first operation that starts after the coordinator abandons the txn
+ // TODO (desired, testing): verify operations with unknown outcomes are
finalised by the first operation that starts after the coordinator abandons the
txn
public StrictSerializabilityVerifier(int keyCount)
{
this.keyCount = keyCount;
diff --git a/accord-maelstrom/src/main/java/accord/maelstrom/Cluster.java
b/accord-maelstrom/src/main/java/accord/maelstrom/Cluster.java
index 3f90348..d958047 100644
--- a/accord-maelstrom/src/main/java/accord/maelstrom/Cluster.java
+++ b/accord-maelstrom/src/main/java/accord/maelstrom/Cluster.java
@@ -47,7 +47,7 @@ import accord.messages.Request;
import accord.api.Scheduler;
import accord.topology.Topology;
-// TODO: merge with accord.impl.basic.Cluster
+// TODO (low priority, testing): merge with accord.impl.basic.Cluster
public class Cluster implements Scheduler
{
public interface Queue<T>
diff --git a/accord-maelstrom/src/main/java/accord/maelstrom/Json.java
b/accord-maelstrom/src/main/java/accord/maelstrom/Json.java
index f311261..e374f01 100644
--- a/accord-maelstrom/src/main/java/accord/maelstrom/Json.java
+++ b/accord-maelstrom/src/main/java/accord/maelstrom/Json.java
@@ -489,7 +489,6 @@ public class Json
static
{
- // TODO: Maelstrom hooks should be registered at run-time to permit
separate tree
GSON = new GsonBuilder().registerTypeAdapter(Packet.class,
Packet.GSON_ADAPTER)
.registerTypeAdapter(Id.class, ID_ADAPTER)
.registerTypeAdapter(Txn.class, TXN_ADAPTER)
diff --git
a/accord-maelstrom/src/main/java/accord/maelstrom/MaelstromRequest.java
b/accord-maelstrom/src/main/java/accord/maelstrom/MaelstromRequest.java
index b88891f..95c7709 100644
--- a/accord-maelstrom/src/main/java/accord/maelstrom/MaelstromRequest.java
+++ b/accord-maelstrom/src/main/java/accord/maelstrom/MaelstromRequest.java
@@ -47,7 +47,6 @@ public class MaelstromRequest extends Body implements Request
public void process(Node node, Id client, ReplyContext replyContext)
{
- // TODO: error handling
node.coordinate(txn).addCallback((success, fail) -> {
if (success != null) node.reply(client, replyContext, new
MaelstromReply(MaelstromReplyContext.messageIdFor(replyContext),
(MaelstromResult) success));
// else node.reply(client, messageId, new Error(messageId, 13,
fail.getMessage()));
diff --git a/accord-maelstrom/src/main/java/accord/maelstrom/Value.java
b/accord-maelstrom/src/main/java/accord/maelstrom/Value.java
index 55537b1..47e36a7 100644
--- a/accord-maelstrom/src/main/java/accord/maelstrom/Value.java
+++ b/accord-maelstrom/src/main/java/accord/maelstrom/Value.java
@@ -31,7 +31,6 @@ import com.google.gson.stream.JsonWriter;
import static accord.utils.Utils.toArray;
-// TODO: abstract
public class Value
{
public static final Value EMPTY = new Value();
diff --git a/accord-maelstrom/src/test/java/accord/maelstrom/Runner.java
b/accord-maelstrom/src/test/java/accord/maelstrom/Runner.java
index 9a4e926..f67b5bf 100644
--- a/accord-maelstrom/src/test/java/accord/maelstrom/Runner.java
+++ b/accord-maelstrom/src/test/java/accord/maelstrom/Runner.java
@@ -249,7 +249,7 @@ public class Runner
Main.listen(factory, parseOutput(delay, output, Function.identity()),
System.out, System.err);
}
- // TODO: we need to align response ids with the input; for now replies are
broken
+ // TODO (low priority, maelstrom): we need to align response ids with the
input; for now replies are broken
static void replay(int nodeCount, TopologyFactory factory, boolean delay,
Supplier<Packet> input) throws IOException
{
run(nodeCount, new QueueSupplier()
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
