ifesdjeen commented on code in PR #198:
URL: https://github.com/apache/cassandra-accord/pull/198#discussion_r2132327387
##########
accord-core/src/main/java/accord/messages/ReadData.java:
##########
@@ -364,13 +365,25 @@ protected AsyncChain<Data> beginRead(SafeCommandStore
safeStore, Timestamp execu
static Ranges unavailable(SafeCommandStore safeStore, Command command)
{
- Timestamp executeAt = command.executesAtLeast();
- if (executeAt == null) executeAt = command.executeAtOrTxnId();
- // TODO (required): for awaitsOnlyDeps commands, if we cannot infer an
actual executeAtLeast we should confirm no situation where txnId is not an
adequately conservative value for unavailable/unsafeToRead
- Ranges ranges = safeStore.unsafeToReadAt(executeAt);
- if (ranges.size() > command.route().size())
- ranges = ranges.intersecting(command.route(), Minimal);
- return ranges;
+ // note: syncpoints and ephemeral reads simply consume the latest
information (whatever it is),
+ // which is represented by the latest possible safeToRead entry
(which is only updated on successful bootstrap)
+ // We DO NOT use executesAtLeast here, this is used only to compute
retryInFutureEpoch which reports ranges
+ // we aren't able to serve information newer than executesAtLeast for.
+ // executeAtLeast can yield false answers for safeToRead even for
single keys by picking a time
+ TxnId txnId = command.txnId();
+ Timestamp executeAt = command.executeAtIfKnown();
+ if (executeAt == null)
+ {
+ Invariants.require(txnId.awaitsOnlyDeps());
+ executeAt = txnId;
+ }
+ Timestamp readAt = txnId.awaitsOnlyDeps() ? Timestamp.MAX : executeAt;
+ Ranges safeToRead = safeStore.safeToReadAt(readAt);
+ Ranges reads = safeStore.ranges().allAt(executeAt);
+ Ranges unsafeToRead = reads.without(safeToRead);
+ if (unsafeToRead.size() > command.route().size())
+ unsafeToRead = unsafeToRead.intersecting(command.route(),
Minimal);
Review Comment:
nit: indentation
##########
accord-core/src/main/java/accord/messages/ReadData.java:
##########
@@ -364,13 +365,25 @@ protected AsyncChain<Data> beginRead(SafeCommandStore
safeStore, Timestamp execu
static Ranges unavailable(SafeCommandStore safeStore, Command command)
{
- Timestamp executeAt = command.executesAtLeast();
- if (executeAt == null) executeAt = command.executeAtOrTxnId();
- // TODO (required): for awaitsOnlyDeps commands, if we cannot infer an
actual executeAtLeast we should confirm no situation where txnId is not an
adequately conservative value for unavailable/unsafeToRead
- Ranges ranges = safeStore.unsafeToReadAt(executeAt);
- if (ranges.size() > command.route().size())
- ranges = ranges.intersecting(command.route(), Minimal);
- return ranges;
+ // note: syncpoints and ephemeral reads simply consume the latest
information (whatever it is),
+ // which is represented by the latest possible safeToRead entry
(which is only updated on successful bootstrap)
+ // We DO NOT use executesAtLeast here, this is used only to compute
retryInFutureEpoch which reports ranges
+ // we aren't able to serve information newer than executesAtLeast for.
+ // executeAtLeast can yield false answers for safeToRead even for
single keys by picking a time
Review Comment:
I am afraid I can not parse (or am missing context to do so) this comment,
particularly this last line.
##########
accord-core/src/test/java/accord/impl/basic/MonitoredPendingQueue.java:
##########
@@ -75,6 +75,30 @@ public void add(Pending item, long delay, TimeUnit units)
wrapped.add(item, delay, units);
}
+ @Override
+ public void preregister(Pending item)
Review Comment:
Could you add a small comment on distinction between reregister and
addPreregistered?
looks like this was done for cancellation, but thought it might be useful
for explicit. Maybe we could change name for addPreregistered to have larger
distinction? Maybe `addPreregistered` could be something like schedule?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
