Close the thrift connection of KyuubiServer when backend engine is determined as dead by EngineAliveProbe #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
risyomei
force-pushed
the
feature/kill-when-engine-fail
branch
from
72cf5aa to
a4725d4
Compare
risyomei
changed the title
risyomei
force-pushed
the
feature/kill-when-engine-fail
branch
from
a4725d4 to
f7bce43
Compare
|
hi @risyomei the target branch should be apache/kyuubi:master |
|
@turboFei |
risyomei
force-pushed
the
feature/kill-when-engine-fail
branch
from
f7bce43 to
e40fcfc
Compare
|
Hi, @turboFei |
risyomei
pushed a commit
that referenced
this pull request
Mar 23, 2023
### _Why are the changes needed?_ Introduce a brand new CHAT engine, it's supposed to support different backends, e.g. ChatGPT, 文心一言, etc. This PR implements the following providers: - ECHO, simply replies a welcome message. - GPT: a.k.a ChatGPT, powered by OpenAI, which requires a API key for authentication. https://platform.openai.com/account/api-keys Add the following configurations in `kyuubi-defaults.conf` ``` kyuubi.engine.chat.provider=[ECHO|GPT] kyuubi.engine.chat.gpt.apiKey=<chat-gpt-api-key> ``` Open an ECHO beeline chat engine. ``` beeline -u 'jdbc:hive2://localhost:10009/?kyuubi.engine.type=CHAT;kyuubi.engine.chat.provider=ECHO' ``` ``` Connecting to jdbc:hive2://localhost:10009/ Connected to: Kyuubi Chat Engine (version 1.8.0-SNAPSHOT) Driver: Kyuubi Project Hive JDBC Client (version 1.7.0) Beeline version 1.7.0 by Apache Kyuubi 0: jdbc:hive2://localhost:10009/> Hello, Kyuubi!; +----------------------------------------+ | reply | +----------------------------------------+ | This is ChatKyuubi, nice to meet you! | +----------------------------------------+ 1 row selected (0.397 seconds) ``` Open a ChatGPT beeline chat engine. (make sure your network can connect the open API and configure the API key) ``` beeline -u 'jdbc:hive2://localhost:10009/?kyuubi.engine.type=CHAT;kyuubi.engine.chat.provider=GPT' ``` <img width="1109" alt="image" src="https://user-images.githubusercontent.com/26535726/225813625-a002e6e2-3b0d-4194-b061-2e215d58ba94.png"> ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [x] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request Closes apache#4544 from pan3793/chatgpt. Closes apache#4544 87bdebb [Cheng Pan] nit f7dee18 [Cheng Pan] Update docs 9beb551 [cxzl25] chat api (#1) af38bdc [Cheng Pan] update docs 9aa6d83 [Cheng Pan] Initial implement Kyuubi Chat Engine Lead-authored-by: Cheng Pan <chengpan@apache.org> Co-authored-by: cxzl25 <cxzl25@users.noreply.github.com> Signed-off-by: Cheng Pan <chengpan@apache.org>
risyomei
pushed a commit
that referenced
this pull request
Apr 1, 2024
* Implemented classloader-based approach
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are the changes needed?
As described in apache#4457, when the backend engine becomes unresponsive,
the beeline will hang because
client.getOperationStatus(_remoteOpHandle)inExecuteStatement #waitStatementComplete()would never receive any response.https://github.com/apache/kyuubi/blob/43309b86f1997b028e8fde5cb4e6449d818f4f73/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/ExecuteStatement.scala#L101-L105
While the EngineAliveProbe can identify engine failure, it does not resolve the underlying issue of the thrift client waiting for a response, causing the beeline to remain unresponsive. The only way to resolve this state of suspension is by interrupting the thrift client thread.
https://github.com/apache/kyuubi/blob/3d65f2711faa5dc9173130557e2d33adab04b5c7/kyuubi-server/src/main/scala/org/apache/kyuubi/client/KyuubiSyncThriftClient.scala#L84
This pull request introduces a new logic to the EngineAliveProbe feature, whereby upon detecting an engine failure, it deliberately closes the thrift connection. This creates an exception that can then be handled by the error handling mechanism introduced in apache#646, thereby allowing for a graceful shutdown of the session handle.
How was this patch tested?
I am very happy to add a test case but I am not aware of how to simulate a state of unresponsive.
Otherwise, I can implement test like this:
https://github.com/apache/kyuubi/blob/43309b86f1997b028e8fde5cb4e6449d818f4f73/kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerUserSuite.scala#L169
kyuubi-default.confAfter 3 consecutive alive probe fails, Engine is marked dead.
Now force closing the thrift client to generate an exception.
OperationManager try
closeOperationbut failed because the connection is already closedAliveProbeClient try to close the remote handler but failed.
It's expected as the backend Engine is dead already.
KyuubiSyncThriftClient#closeSession failed
It's expected as the backend Engine is dead already.
Beeline finished with errors (as expected)
The kyuubi-server suite wasn't working even before this PR.