aweisberg commented on code in PR #56:
URL: https://github.com/apache/cassandra-accord/pull/56#discussion_r1297773964
##########
accord-core/src/main/java/accord/messages/ReadData.java:
##########
@@ -149,14 +149,16 @@ void read(SafeCommandStore safeStore, Timestamp
executeAt, PartialTxn txn)
Ranges unavailable = safeStore.ranges().unsafeToReadAt(executeAt);
txn.read(safeStore, executeAt).begin((next, throwable) -> {
- if (throwable != null)
+ // TODO (expected, exceptions): should send exception to client,
and consistency handle/propagate locally
+ logger.trace("{}: read failed for {}: {}", txnId, unsafeStore,
throwable);
+ synchronized (ReadData.this)
{
- // 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);
+ if (fail == null)
+ fail = throwable;
+ else
+ fail.addSuppressed(throwable);
}
- else
- readComplete(unsafeStore, next, unavailable);
+ readComplete(unsafeStore, next, unavailable);
Review Comment:
The derived classes aren't the ones that send back the errors. `ack` does
and that is `private`.
The derived classes can (and do) drop errors as part of their execution such
as `FetchRequest.readComplete` which starts a new `AsyncChain` and adds a
callback which passes the failure to `uncaughtExceptionHandler`.
I think the interface for listeners, task submission, and callbacks errors
is possibly what is wrong. Every execution should occur with a context that
specifies what the uncaught error handling should be, but you don't have to
provide that context along with the thing you are executing. Or you do, but
it's bespoke to that one thing you want to run (parameter to callback) rather
than common across a variety of things that might be done to execute a request.
That would help with making it so can't get an error and then drop it or
consume it in the wrong place because you would never handle it in the first
place unless you specifically provided your own error handler rather than the
one in the context. And when you do specify it should still hand off errors you
don't want to deal with to the parent error handling context.
--
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]
