-
Notifications
You must be signed in to change notification settings - Fork 105
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.
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
8322142: JFR: Periodic tasks aren't orphaned between recordings #203
Conversation
Hi @antonha, welcome to this OpenJDK project and thanks for contributing! We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user antonha" as summary for the issue. If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing |
This backport pull request has now been updated with issue from the original commit. |
/approval JDK-8322142 request This issue affects production flight recording rather severely, it would be useful to have the fix in 21 as well. |
|
Webrevs
|
@antonha What testing have you done? Please enable GHA for your fork (minimum bar to get it integrated). Running JFR tests would be wise to do as well. Thanks! |
The fix request comment didn't mention it so I'm removing the approval label for now. Please re-apply once that's present/fixed. |
@jerboaa Thanks - I did run the tier1 suite locally before creating the PR. I also enabled GHA for the for the fork, but did that after creating the PR so I don't think that it automatically triggered - I now triggered a run which seems to have passed: https://github.com/antonha/jdk21u-dev/actions/runs/7646727789 I'm not that familiar with the test suite so I don't quite know which JFR tests there are, but I did now try running the ==============================
Test summary
==============================
TEST TOTAL PASS FAIL ERROR
jtreg:test/jdk/jdk/jfr 584 584 0 0
==============================
TEST SUCCESS Should I re-apply for approval now? |
@antonha Thanks!
Yes. Mention the testing you've done in the approval comment. |
/approval JDK-8322142 request GHA sanity checks passed (https://github.com/antonha/jdk21u-dev/actions/runs/7646727789). |
|
@antonha This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 21 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. As you do not have Committer status in this project an existing Committer must agree to sponsor your change. ➡️ To flag this PR as ready for integration with the above commit message, type |
Thanks for the update. Fix request comment was better now. Affects JFR usage only => low risk with rather high impact. Approved. |
/integrate |
/sponsor |
Going to push as commit a9498e5.
Your commit was automatically rebased without conflicts. |
Backport to fix https://bugs.openjdk.org/browse/JDK-8322142 for jdk21.
First time commiting to openjdk, so please pardon if this is the wrong approach to the PR. I followed instructions in https://wiki.openjdk.org/display/JDKUpdates/How+to+contribute+or+backport+a+fix - not sure if those are still valid for 21.
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/203/head:pull/203
$ git checkout pull/203
Update a local copy of the PR:
$ git checkout pull/203
$ git pull https://git.openjdk.org/jdk21u-dev.git pull/203/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 203
View PR using the GUI difftool:
$ git pr show -t 203
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/203.diff
Webrev
Link to Webrev Comment