[crac] RFR: Minor code cleanup and improvements

Radim Vansa duke at openjdk.org
Fri May 19 06:55:21 UTC 2023


On Thu, 18 May 2023 16:13:58 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

>> First of all, I think that `javax.crac` should mirror `jdk.crac` API- and docs-wise. It will be much easier when everyone will be able to just change the imports.
>> 
>> About the constructor with message: I find a bit confusing when an exception is thrown because of some problem with `criu` but there's no actionable message. I have added a simple 'Native checkpoint failed' but we should probably point user to the dump4.log file. (`criuengine` should also make some sanity checks on permissions but that's another thing). I wouldn't object to hiding it, though.
>> 
>> About the one without: `Context` narrows the `throws` to CE/RE and since we expect users to implement Context this would give them no chance to throw checked exceptions, not even the aggregating one. Hiding that won't work.
>
> Yes, that is a bug javax.crac does not follow jdk.crac.
> 
> Agree about the constructor without args.
> 
> And still want to delete the a constructor with message. We can introduce a jdk.crac.impl.CheckpointMessageException (along j.c.i.CheckpointOpenResourceException) which we use to communicate different reason(s) checkpoint is not successful.

What about turning the inheritance the other way: final CheckpointAggregateException extends CheckpointException? This would be easily distinguishable, no need to change anything on Context interface. The no-arg constructor in CheckpointException would be protected.
It's kind of natural that exceptions carry messages.

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

PR Review Comment: https://git.openjdk.org/crac/pull/64#discussion_r1198609760


More information about the crac-dev mailing list