[crac] RFR: Fix ordering of invocation on Resources [v3]

[Radim Vansa]

On Thu, 4 May 2023 20:59:42 GMT, Radim Vansa <duke 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.
>> 
>>> Your example is invalid: If a throwing resource is to be removed, it's the task of the parent context - and that one will see the exception. The parent context should not remove its child context since one of the N resources in the child context is failing.
>> 
>> 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.
>
>> 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.

I realized I haven't stressed enough one motivation for `Core.registerException`: when the context has finished its `beforeCheckpoint` (it won't be called again) and someone calls into this Context's `register` we cannot propagate the failure through throwing. So we won't avoid API change - and in my view it's natural to use the same method for reporting the failure during `beforeCheckpoint` execution, too.

-------------

PR Review Comment: https://git.openjdk.org/crac/pull/60#discussion_r1185718419