8367026: Reorder the timeout failure handler commands to have jstack run before the rest

[jaikiran]

Can I please get a review of this change to a jtreg failure handler configured in the JDK?

The change proposes to generate a thread dump much sooner than previously whenever a test times out. This should thus capture a much more accurate state of the test process when the test is considered timed out.

Due to the recent changes in the default timeout factor, we have noticed some tests which timeout and the jtreg failure handler actions start execution. While those are being executed the test sometimes completes. So by the time the "jstack" failure handler action is executed (can be several seconds later), the test's state will no longer be accurate.

The change here generates a thread dump using jstack as the first action in the set of failure handler actions. It does it only once and then moves to the rest of the actions, one of which subsequent "jstack" which generates thread dumps more than once.

I have verified that this change works as expected when a test times out. The action is named "thread_dump" instead of just reusing the "jstack" name because the current HTML rendering of the processes.html runs into trouble if there are more than one action with the same name.

I wanted to reorder some of the other commands in that set, but it causes some trouble in the rendering of the HTML and would require some changes to that part. So I decided to keep this simple and have it done sooner to help investigating timeout failures in our CI.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8367026: Reorder the timeout failure handler commands to have jstack run before the rest (Enhancement - P4)

Reviewers

• Erik Joelsson (@erikj79 - Reviewer)
• Leonid Mesnik (@lmesnik - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27574/head:pull/27574
$ git checkout pull/27574

Update a local copy of the PR:
$ git checkout pull/27574
$ git pull https://git.openjdk.org/jdk.git pull/27574/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27574

View PR using the GUI difftool:
$ git pr show -t 27574

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27574.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jpai! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@jaikiran 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:

8367026: Reorder the timeout failure handler commands to have jstack run before the rest

Reviewed-by: erikj, lmesnik

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 11 new commits pushed to the master branch:

• 6b4b102: 8368809: JFR: Remove events from testSettingConfiguration in TestActiveSettingEvent
• 8cc54ec: 8368563: JFR: Improve jfr query help text
• 07ea907: 8361606: ConsumeNextMnemonicKeyTypedTest.java fails on Windows: character typed with VK_A: a
• ... and 8 more: https://git.openjdk.org/jdk/compare/2746c1a555891564963299182b3b0293eaefc901...master

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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@jaikiran The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (503fe471)

[jaikiran]

/label remove core-libs
/label add build

[openjdk]

@jaikiran
The core-libs label was successfully removed.

[openjdk]

@jaikiran
The build label was successfully added.

[jaikiran]

Thank you Erik and Leonid for the reviews.

[jaikiran]

/integrate

[openjdk]

Going to push as commit 17d8fa8.
Since your change was applied there have been 15 commits pushed to the master branch:

• 0366d88: 8354894: java/lang/Thread/virtual/Starvation.java timeout on server with high CPUs
• 9b02896: 8360558: Use hex literals instead of decimal literals in math intrinsic constants
• fe9dbcc: 8368599: ShenandoahConcurrentMark could use ThreadsClaimTokenScope
• ... and 12 more: https://git.openjdk.org/jdk/compare/2746c1a555891564963299182b3b0293eaefc901...master

Your commit was automatically rebased without conflicts.

[openjdk]

@jaikiran Pushed as commit 17d8fa8.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.