Skip to content

Conversation

@abhisheknath2011
Copy link
Member

@abhisheknath2011 abhisheknath2011 commented Jun 9, 2025

Summary

Issue] Briefly discuss the summary of the changes made in this
pull request in 2-3 lines.

The submitted job has timeout of 3 hours for status check. Recently we have observed jobs are getting into long queued state and did not start. Eventually they are getting killed after 3 hours of status check. As a result overall runtime of jobs scheduler increased and going beyond 2 days. We had global timeout as part of jobs status check which was removed as part of the PR - https://github.com/linkedin/openhouse/pull/319/files#diff-8227a7ab323e96994351616f8ad5502bfb93ee277a579a0e7dccc82591803cabL176. This is the recent change in status check so my original thinking was the issue around removal of the explicit global timeout. But it seems like this is not the issue. That means the issue existed in the earlier code as well. Suspecting that status poller thread is not interruptible through future.cancel. So making the code interruptible so that the thread exits on interrupt.

Changes

  • Client-facing API Changes
  • Internal API Changes
  • Bug Fixes
  • New Features
  • Performance Improvements
  • Code Style
  • Refactoring
  • Documentation
  • Tests

For all the boxes checked, please include additional details of the changes made in this pull request.

Testing Done

Local builds passed and verified exiting tests. The issues are on Prod and staging cluster and based on that these fixes are added.

  • Manually Tested on local docker setup. Please include commands ran, and their output.
  • Added new tests for the changes made.
  • Updated existing tests to reflect the changes made.
  • No tests added or updated. Please explain why. If unsure, please feel free to ask for help.
  • Some other form of testing like staging or soak time in production. Please explain.

For all the boxes checked, include a detailed description of the testing done for the changes made in this pull request.

Additional Information

  • Breaking Changes
  • Deprecations
  • Large PR broken into smaller PRs, and PR plan linked in the description.

For all the boxes checked, include additional details of the changes made in this pull request.

@abhisheknath2011 abhisheknath2011 marked this pull request as ready for review June 9, 2025 22:47
Copy link
Collaborator

@jiang95-dev jiang95-dev left a comment

Choose a reason for hiding this comment

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

How do you reason this fixes the stuck problem?

If remainingTimeMillis <= 0, then taskFuture.get(remainingTimeMillis, TimeUnit.MILLISECONDS); will return immediately, and essentially doing the same thing as the removed line.

@abhisheknath2011
Copy link
Member Author

How do you reason this fixes the stuck problem?

If remainingTimeMillis <= 0, then taskFuture.get(remainingTimeMillis, TimeUnit.MILLISECONDS); will return immediately, and essentially doing the same thing as the removed line.

This is the recent change in status check so my original thinking was the issue around removal of the explicit global timeout. So if you are 100% confident that this is not the issue, that means the issue existed in the earlier code as well. I am suspecting that status poller thread is not interruptible through future.cancel. So I am making the code interruptible so that the thread exits on interrupt.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants