[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