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

[KYUUBI #6236] Throw connectionClosed KyuubiSQLException when engine died #6237

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

Conversation

beryllw
Copy link
Contributor

@beryllw beryllw commented Apr 2, 2024

🔍 Description

Issue References 🔗

This pull request fixes #6236

Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklist 📝

Be nice. Be informative.

@beryllw beryllw changed the title Throw connectionClosed KyuubiSQLException when engine died [KYUUBI #6236]Throw connectionClosed KyuubiSQLException when engine died Apr 3, 2024
@cxzl25 cxzl25 changed the title [KYUUBI #6236]Throw connectionClosed KyuubiSQLException when engine died [KYUUBI #6236] Throw connectionClosed KyuubiSQLException when engine died Apr 5, 2024
e)
case te: TTransportException =>
warn(s"Error $action $opType: Socket for ${session.handle} is closed", e)
KyuubiSQLException.connectionClosed(session.handle, e)
Copy link
Member

Choose a reason for hiding this comment

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

socket closed may be more appropriate. and cc @yaooqinn, it was introduced in #375.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong option here, both seem fine to me

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.

[Improvement] Throw connectionClosed KyuubiSQLException when engine died
3 participants