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.

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

Issue Fix for build break Defect 303094 #30570

Open
wants to merge 1 commit into
base: integration
Choose a base branch
from

Conversation

pardhivkrishna
Copy link

Increasing the transaction timeout

@pardhivkrishna
Copy link
Author

pardhivkrishna commented Jan 16, 2025

#build (view Open Liberty Personal Build - ❌ completed with errors/failures.)

Note: Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

Code analysis and actions

DO NOT DELETE THIS COMMENT.
  • 1 FAT files were changed, added, or removed.

  • Check that the build did not break the affected FAT suite(s).

  • Please describe in a separate comment how you tested your changes.

@@ -136,7 +136,7 @@ public static void tearDown() throws Exception {
runInServlet("verifyTasksRunMultipleTimes");
} finally {
// wait for tasks to stop running
long waitForTaskCompletions = TimeUnit.SECONDS.toMillis(10);
long waitForTaskCompletions = TimeUnit.SECONDS.toMillis(60);
Copy link
Member

Choose a reason for hiding this comment

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

Adjusting this time will not have any impact on the failures reported by the BB defect, and will unnecessarily delay the completion of the test. I'd recommend not modifying this time.

The test is failing because the tasks are not completing within the specified missedTaskThreshold, which is configured on line 109. Instead of updating the sleep time here, I'd recommend updating the missedTaskThreshold on line 109, but perhaps not by such a large amount, as it may impact other timing in the test. Perhaps increase 109 from 10 to 25.

@@ -136,7 +136,7 @@ public static void tearDown() throws Exception {
runInServlet("verifyTasksRunMultipleTimes");
Copy link
Member

Choose a reason for hiding this comment

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

Copyright at top of file needs to increase the 2nd year to 2025

@tkburroughs
Copy link
Member

I'm not very familiar with this component or FAT, so I've added @njr-11 and @KyleAure to provide a better opinion as to whether my recommendation for adjusting the missedTaskThreshold will have any negative impact to this test.

Copy link
Contributor

@njr-11 njr-11 left a comment

Choose a reason for hiding this comment

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

Increasing the transaction timeout

The changes within this PR do not increase the transaction timeout, nor do they increase the missedTaskThreshold as Tracy points out.

The team owning this bucket needs to spend time to understand what the test is doing, perform investigation of the failure, and write up a description of the problem in the issue or PR as they understand and how they are expecting to solve it. It is not the place of the code reviewer to do that work instead. That would be backwards. The only work I see here documenting any investigation or analysis is all from Brian before transferring this defect over, and from Tracy in attempting to review it.

You can use the information they have kindly provided, but you need to perform your own analysis taking that into account and be able to explain and justify whatever change you are making.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants