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

Revert event loop related changes #24548

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

shangm2
Copy link
Contributor

@shangm2 shangm2 commented Feb 13, 2025

Description

  1. we see some issues at certain clusters where workers are not receiving any splits. Reverting the following two prs back to unblock others so I can take time to root cause it.
    a. Use a wrapper for event loop to handle exceptions gracefully #24412
    b. Assign event loop to tasks and run a task's methods in the same loop #24288

Motivation and Context

  1. reverting event loop related changes to unblock others.

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* ... 
* ... 

Hive Connector Changes
* ... 
* ... 

If release note is NOT required, use:

== NO RELEASE NOTE ==

@shangm2 shangm2 requested a review from a team as a code owner February 13, 2025 01:13
@shangm2 shangm2 requested a review from presto-oss February 13, 2025 01:13
@prestodb-ci prestodb-ci added the from:Meta PR from Meta label Feb 13, 2025
swapsmagic
swapsmagic previously approved these changes Feb 13, 2025
jainxrohit
jainxrohit previously approved these changes Feb 13, 2025
Copy link
Contributor

@jainxrohit jainxrohit left a comment

Choose a reason for hiding this comment

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

Thank you Shang!

rschlussel
rschlussel previously approved these changes Feb 13, 2025
@rschlussel
Copy link
Contributor

thanks @shangm2 can you fix the merge conflict?

FYI @ZacBlanco this change was in the 0.291 release. We're still investigating, but that release may need to be patched.

Copy link
Member

@arhimondr arhimondr left a comment

Choose a reason for hiding this comment

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

We are still investigating the problem. There is a chance it could be related to differences in configuration on our side. Let's hold on with the revert for now.

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

Successfully merging this pull request may close these issues.

6 participants