[crac] RFR: Fix ordering of invocation on Resources [v3]
Radim Vansa
duke at openjdk.org
Thu May 4 21:04:54 UTC 2023
On Thu, 4 May 2023 14:35:46 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:
> You're trading additional code for a complicated interface. What are the directions what to use: Core.recordException() vs throw new Exception() ? This two very similar interfaces are the sign we are trying to do something strange.
That's because of the non-standard requirement to continue after encountering an exception, and multiple rewraps in a hierarchy of contexts.
Resources should normally throw exceptions; Contexts are here to call into the Resource and aggregate errors. There's no point of propagating error higher up the hierarchy.
> Could you elaborate?.. I'm lost what "remove child context" means. I was reffering to the parent context being able to handle a CheckpointException from the child context.
And why would the child context throw the CheckpointException? The most common reason would be that one of X resources in child context has thrown. It's fine if a (child) Context removes the inner Resource after throwing. But that error should not propagate any higher, because if parent context were to remove it's (throwing) child context it would remove it along with the other X - 1 resources that are working correctly.
>> No; ACI was totally-ordered in the previous version of the PR, but now it doesn't care about ordering at all; it's the abstract `runBeforeCheckpoint` that decides on the order.
>
> I see, thanks. So ACI task is to track beforeCheckpoint order and provide afterRestore in the opposite order, this seems the most useful part of it. But its interface does not seem to be very consistent.
>
> Does it really needs an abstract base class that also maintains calling, logging and exception propagation? It will be nice to separate all this concerns for greater flexibility e.g. with composition of some classes in a Context implementation, rather than that to extend the base class with all of them combined.
We can further fine-tune the separation when the need arises and we have a concrete example.
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/60#discussion_r1185508424
PR Review Comment: https://git.openjdk.org/crac/pull/60#discussion_r1185510445
More information about the crac-dev
mailing list