[crac] RFR: Minor code cleanup and improvements

Anton Kozlov akozlov at openjdk.org
Thu May 18 16:16:23 UTC 2023


On Thu, 11 May 2023 06:13:12 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> src/java.base/share/classes/javax/crac/CheckpointException.java line 50:
>> 
>>> 48:      * @param message the detail message.
>>> 49:      */
>>> 50:     public CheckpointException(String message) {
>> 
>> What if we remove this constructor and hide the other one? https://github.com/openjdk/crac/pull/60/files#r1190070472
>
> 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.

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

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


More information about the crac-dev mailing list