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鈥檒l 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