aweisberg commented on code in PR #101:
URL: https://github.com/apache/cassandra-accord/pull/101#discussion_r1663209352
##########
accord-core/src/main/java/accord/local/Node.java:
##########
@@ -285,31 +286,24 @@ public void onEpochRedundant(Ranges ranges, long epoch)
topology.onEpochRedundant(ranges, epoch);
}
- public AsyncChain<?> awaitEpoch(long epoch)
- {
- if (topology.hasEpoch(epoch))
- return AsyncResults.SUCCESS_VOID;
- configService.fetchTopologyForEpoch(epoch);
- return topology.awaitEpoch(epoch);
- }
-
- public AsyncChain<?> awaitEpoch(@Nullable EpochSupplier epochSupplier)
+ public void withEpoch(EpochSupplier epochSupplier, BiConsumer<Void,
Throwable> callback)
{
if (epochSupplier == null)
- return AsyncResults.SUCCESS_VOID;
- return awaitEpoch(epochSupplier.epoch());
+ callback.accept(null, null);
+ else
+ withEpoch(epochSupplier.epoch(), callback);
}
- public void withEpoch(long epoch, Runnable runnable)
+ public void withEpoch(long epoch, BiConsumer<Void, Throwable> callback)
{
if (topology.hasEpoch(epoch))
{
- runnable.run();
+ callback.accept(null, null);
}
else
{
configService.fetchTopologyForEpoch(epoch);
- topology.awaitEpoch(epoch).addCallback(runnable).begin(agent);
+ topology.awaitEpoch(epoch).addCallback(callback).begin((ignored1,
ignored2) -> {});
Review Comment:
`begin` is documented as not being safe to call multiple times and the
result of `awaitEpoch` might not be a new `AsyncChain`, it might be one that is
already started. It happens to be that right now `AsyncResult` doesn't actually
error out when `begin` is called.
So by contract this is all pretty problematic and in fact `begin` should not
even need to be called. I think the problem is partially that `awaitEpoch`
should probably return `AsyncResult` not `AsyncChain` so you don't need to call
begin you can just add the callback.
The reason it returns `AsyncChain` is because the `withExecutor` in
`TopologyManager.awaitEpoch` converts it to an `AsyncChain`.
It might be better to add an `AsyncResults.withExecutor` and use that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
