FISH-12969 Payara Server Startup Fails in Apache NetBeans IDE Due to the CRaCCheckpointTo Option#9237
FISH-12969 Payara Server Startup Fails in Apache NetBeans IDE Due to the CRaCCheckpointTo Option#9237jGauravGupta wants to merge 1 commit intoapache:masterfrom
Conversation
simonladen
left a comment
simonladen
left a comment
There was a problem hiding this comment.
Tested locally, with Payara 6.34.0.
Server starts/stops without error.
|
@jGauravGupta if you change the labels after creation of the PR, the tests don't automatically get executed again. In these cases please use the following "trick" to force rerun of tests: Use "Lock conversation" and immediately unlock again. That acts as a trigger. I did this right now. |
|
Thanks @matthiasblaesing for the hint. I’ll follow it and add the label before creating the PR. I’ll need your review as the PR fixes a critical issue. |
matthiasblaesing
left a comment
matthiasblaesing
left a comment
There was a problem hiding this comment.
Wouldn't it make sense to add the info "isCracSupportedtoJDKVersionand then pass the wholeJvmOption` instead of the long list of arguments?
| } | ||
|
|
||
| public static boolean isCorrectJDK(JDKVersion jdkVersion, Optional<String> vendorOrVM, Optional<JDKVersion> minVersion, Optional<JDKVersion> maxVersion) { | ||
| public static boolean isCorrectJDK(JDKVersion jdkVersion, |
There was a problem hiding this comment.
I think this method is badly named. I think a name like isOptionSupported would better descript what is done here.
| fullOption.vendorOrVM, | ||
| fullOption.minVersion, | ||
| fullOption.maxVersion, | ||
| fullOption.option, |
There was a problem hiding this comment.
Why not just pass the full JvmOption here?
| .map(home -> new java.io.File(home, "lib/criu")) | ||
| .map(java.io.File::exists) | ||
| .orElse(false); | ||
| } |
There was a problem hiding this comment.
From my POV this is a property of the JDK so I would see this as part of JDKVersion?
matthiasblaesing
left a comment
matthiasblaesing
left a comment
There was a problem hiding this comment.
Please squash and update the signature file. I left an inline comment to minimize exposure of moving parts.
Given that the current cycle ends, this is friend-only and this needs to be fixed, I'm ok with this, but JDKVersion, its use of Optional and now two different ways to get the necessary information (most information is present from construction time, it is unclear why that is not done for CRaC support), is really ugly.
| * @param javaHome Java home directory to check | ||
| * @return true if CRaC-enabled JDK | ||
| */ | ||
| public boolean isCRaCSupported(String javaHome) { |
There was a problem hiding this comment.
Please make this private. This is only used from #isOptionSupported.
jGauravGupta
force-pushed
the
FISH-12969
branch
2 times, most recently
from
04f3e9f to
45bf2d8
Compare
…the CRaCCheckpointTo Option
3 participants
Payara Server startup may fail in Apache NetBeans when domain.xml contains
the JVM option:
-XX:CRaCCheckpointTo=...This option is only supported by CRaC-enabled JDKs. The IDE
previously filtered JVM options only by version and vendor, without checking
whether the selected JDK supports CRaC. As a result, non-CRaC JDKs failed to
start with “Unrecognized VM option” errors.
This PR enhances
JDKVersion.isCorrectJDK(...)to additionally validateCRaC-related options. CRaC options are now applied only if the selected
server JDK (javaHome) contains lib/criu, indicating CRaC support.
No changes to existing vendor/version logic. Standard JDK behavior remains
unchanged.