Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make JITServer CRIU tests more robust #17313

Merged
merged 4 commits into from May 5, 2023
Merged

Make JITServer CRIU tests more robust #17313

merged 4 commits into from May 5, 2023

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented May 1, 2023 •
edited

In this PR

  • The jitserver tests are a bit more verbose to help with failures
  • A sleep to is added to jitOptionsTest to ensure that the JVM has time to produce the output needed for a success condition
  • The Test Metrics Disabled test is removed as it is not all that useful

Fixes #17311
Fixes #17324
Fixes #17333

@dsouzai
Make jitserver criu tests more verbose
849c989 
Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@dsouzai dsouzai added comp:test comp:jitserver Artifacts related to JIT-as-a-Service project criu Used to track CRIU snapshot related work labels May 1, 2023
@dsouzai
Copy link
Contributor Author

dsouzai commented May 1, 2023

jenkins test sanity zlinux JDK17

@dsouzai dsouzai changed the title Jitserver criu test verbose Make JITServer tests more robust May 1, 2023
@dsouzai dsouzai changed the title Make JITServer tests more robust Make JITServer CRIU tests more robust May 1, 2023
@dsouzai
Copy link
Contributor Author

dsouzai commented May 1, 2023 •
edited

Grinder to verify that this fixes #17311 is here https://openj9-jenkins.osuosl.org/job/Grinder/2304/

Grinder passed 50/50.

@dsouzai
Copy link
Contributor Author

dsouzai commented May 1, 2023

@tajila could you please review/merge?

@tajila tajila requested a review from JasonFengJ9 May 1, 2023 19:00
@dsouzai
Add sleep in jitOptionsTest to ensure JVM has time to produce necessa…
e749c6c 
…ry output

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@dsouzai
Copy link
Contributor Author

dsouzai commented May 2, 2023

@JasonFengJ9 good for review.

JasonFengJ9
JasonFengJ9 approved these changes May 2, 2023
View reviewed changes
Copy link
Member

@JasonFengJ9 JasonFengJ9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tajila
Copy link
Contributor

tajila commented May 2, 2023

jenkins test sanity xlinux jdk17

@tajila
Copy link
Contributor

tajila commented May 3, 2023

failure is infra related, ill respin

@tajila
Copy link
Contributor

tajila commented May 3, 2023

jenkins test sanity xlinux jdk17

1 similar comment
@dsouzai
Copy link
Contributor Author

dsouzai commented May 4, 2023

jenkins test sanity xlinux jdk17

@dsouzai
Copy link
Contributor Author

dsouzai commented May 4, 2023

Builds are still stuck on the infra issues; I aborted them for now.

@dsouzai
Remove Test Metrics Disabled test and print out jitserver PID
65c95e0 
Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@dsouzai
Copy link
Contributor Author

dsouzai commented May 4, 2023

@mpirvu could you review the latest changes?

mpirvu
mpirvu approved these changes May 4, 2023
View reviewed changes
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mpirvu
Copy link
Contributor

mpirvu commented May 4, 2023

jenkins test sanity plinuxjit,zlinuxjit jdk17

@mpirvu mpirvu self-assigned this May 4, 2023
@dsouzai
Copy link
Contributor Author

dsouzai commented May 4, 2023

The test is failing because of

[2023-05-04T16:47:49.187Z] >> Failure condition was found: [Output match: Connected to a server]

Because the tests run were jitserver tests (i.e. TEST_FLAG=JITAAS), it ended up putting -XX:+UseJITServer on the pre-checkpoint JVM option, which resulted in the test not being valid because it's supposed to be testing what happens when -XX:+UseJITServer is not specified either pre-checkpoint or post-restore.

Will add another commit to exclude the cmdLineTester_criu_jitserverAcrossCheckpoint tests from JITAAS runs.

@dsouzai
Exclude jitserverAcrossCheckpoint in JITAAS mode
2f9aa9b 
Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@dsouzai
Copy link
Contributor Author

dsouzai commented May 4, 2023

Hm, the plinux tests didn't run any of the CRIU tests; i guess because it didn't get scheduled on a machine that supported?

@dsouzai
Copy link
Contributor Author

dsouzai commented May 4, 2023

Added change to exclude cmdLineTester_criu_jitserverAcrossCheckpoint from JITAAS builds.

@tajila
Copy link
Contributor

tajila commented May 5, 2023

You need to run with Jenkins test sanity plinuxcriu jdk11 on P

@mpirvu
Copy link
Contributor

mpirvu commented May 5, 2023

Jenkins test sanity plinuxcriu jdk11

@mpirvu
Copy link
Contributor

mpirvu commented May 5, 2023

jenkins test sanity plinuxjit,zlinuxjit,plinux,zlinux jdk17

@dsouzai
Copy link
Contributor Author

dsouzai commented May 5, 2023

@mpirvu all tests passed, looks like this is good to be merged now.

@mpirvu mpirvu merged commit 746811d into eclipse-openj9:master May 5, 2023
13 checks passed
@jdmpapin jdmpapin mentioned this pull request May 5, 2023
Merged
@mpirvu mpirvu mentioned this pull request May 8, 2023
Open
@dsouzai dsouzai deleted the jitserverCriuTestVerbose branch April 3, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JasonFengJ9 JasonFengJ9 JasonFengJ9 approved these changes

@mpirvu mpirvu mpirvu approved these changes

Assignees

@mpirvu mpirvu

Labels
comp:jitserver Artifacts related to JIT-as-a-Service project comp:test criu Used to track CRIU snapshot related work
Projects
None yet
Milestone
No milestone
4 participants
@dsouzai @tajila @mpirvu @JasonFengJ9