You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "Kwafoor (via GitHub)" <gi...@apache.org> on 2023/12/18 14:04:24 UTC
[PR] [kyuubi #5797]Support to describe and register/de-register engine wit… [kyuubi]
Kwafoor opened a new pull request, #5871:
URL: https://github.com/apache/kyuubi/pull/5871
…h command in current session
# :mag: Description
## Issue References 🔗
<!-- Append the issue number after #. If there is no issue for you to link create one or -->
<!-- If there are no issues to link, please provide details here. -->
This pull request fixes #5797
## Describe Your Solution 🔧
Support to describe and register/de-register engine with command in current session
## Types of changes :bookmark:
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] 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 :coffin:
#### Behavior With This Pull Request :tada:
#### Related Unit Tests
---
# Checklists
## 📝 Author Self Checklist
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project
- [x] I have performed a self-review
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
## 📝 Committer Pre-Merge Checklist
- [ ] Pull request title is okay.
- [ ] No license issues.
- [ ] Milestone correctly set?
- [ ] Test coverage is ok
- [ ] Assignees are selected.
- [ ] Minimum number of approvals
- [ ] No changes are requested
**Be nice. Be informative.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435904184
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/events/KyuubiSessionEvent.scala:
##########
@@ -34,7 +34,7 @@ import org.apache.kyuubi.session.KyuubiSession
* @param startTime session create time
* @param remoteSessionId remote engine session id
* @param engineId engine id. For engine on yarn, it is applicationId.
- * @param openedTime session opened time
+ * @param openedTime session engine opened time
Review Comment:
Do not change this behavior
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435906535
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/command/RunnableCommand.scala:
##########
@@ -30,6 +30,10 @@ trait RunnableCommand extends KyuubiTreeNode {
protected var isAdmin: Boolean = false
+ val statement: String
Review Comment:
Why is the new `statement` attribute needed? It seems that the `name() `method is enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435262274
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/parser/DescribeEngineSuite.scala:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.operation.parser
+
+class DescribeEngineSuite extends ExecutedCommandExecSuite {
+
+ test("desc kyuubi session") {
Review Comment:
desc kyuubi engine
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/parser/DescribeEngineSuite.scala:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.operation.parser
+
+class DescribeEngineSuite extends ExecutedCommandExecSuite {
+
+ test("desc kyuubi session") {
Review Comment:
desc kyuubi engine
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and restart engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1436042213
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/parser/RestartEngineSuite.scala:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.operation.parser
+
+class RestartEngineSuite extends ExecutedCommandExecSuite {
+
+ test("Restart kyuubi engine") {
+ withJdbcStatement() { statement =>
+ var resultSet = statement.executeQuery("KYUUBI DESC ENGINE")
+ assert(resultSet.next())
+ assert(resultSet.getMetaData.getColumnCount == 3)
+ assert(resultSet.getMetaData.getColumnName(1) == "ENGINE_ID")
+ assert(resultSet.getMetaData.getColumnName(2) == "ENGINE_NAME")
+ assert(resultSet.getMetaData.getColumnName(3) == "ENGINE_URL")
+ val engineDied = resultSet.getString("ENGINE_ID")
+
+ resultSet = statement.executeQuery("KYUUBI RESTART ENGINE")
+ assert(resultSet.next())
+ assert(resultSet.getMetaData.getColumnCount == 3)
+ assert(resultSet.getMetaData.getColumnName(1) == "ENGINE_ID")
+ assert(resultSet.getMetaData.getColumnName(2) == "ENGINE_NAME")
+ assert(resultSet.getMetaData.getColumnName(3) == "ENGINE_URL")
+
+ val rs = statement.getConnection.createStatement().executeQuery("KYUUBI DESC ENGINE")
Review Comment:
We should execute the engine-side statements after restarting instead of the server-side, like `select 1`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1862123924
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `16 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`667b5ab`)](https://app.codecov.io/gh/apache/kyuubi/commit/667b5ab6b82f1188d53b37e2fa1d86faf4f49e21?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.36% compared to head [(`756c8a8`)](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.36%.
> Report is 2 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...cala/org/apache/kyuubi/operation/CloseEngine.scala](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vQ2xvc2VFbmdpbmUuc2NhbGE=) | 50.00% | [8 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pache/kyuubi/sql/plan/command/RegisterEngine.scala](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zcWwvcGxhbi9jb21tYW5kL1JlZ2lzdGVyRW5naW5lLnNjYWxh) | 89.47% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | 92.85% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...g/apache/kyuubi/session/KyuubiSessionManager.scala](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25NYW5hZ2VyLnNjYWxh) | 0.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...che/kyuubi/sql/plan/command/DeregisterEngine.scala](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zcWwvcGxhbi9jb21tYW5kL0RlcmVnaXN0ZXJFbmdpbmUuc2NhbGE=) | 94.44% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pache/kyuubi/sql/plan/command/DescribeEngine.scala](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zcWwvcGxhbi9jb21tYW5kL0Rlc2NyaWJlRW5naW5lLnNjYWxh) | 94.11% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #5871 +/- ##
============================================
- Coverage 61.36% 61.36% -0.01%
Complexity 23 23
============================================
Files 612 616 +4
Lines 36239 36332 +93
Branches 4967 4968 +1
============================================
+ Hits 22237 22294 +57
- Misses 11606 11629 +23
- Partials 2396 2409 +13
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/5871?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435056414
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -80,7 +80,7 @@ class KyuubiSessionImpl(
handle.identifier.toString,
sessionManager.applicationManager,
sessionManager.engineStartupProcessSemaphore)
- private[kyuubi] val launchEngineOp = sessionManager.operationManager
+ private[kyuubi] var launchEngineOp = sessionManager.operationManager
Review Comment:
add `@volatile`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435908399
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
We may not be able to use the `lock` of `_client` because `_client` will be assigned a new object after this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1431389608
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -118,6 +118,14 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
If in non-CONNECTION mode, this might still be routed to the same engine since we didn't kill it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1868896805
> 3. describe the design and reason why we have to have `restarting engine`, which is not that reasonable to me
There are some related discussions in https://github.com/apache/kyuubi/issues/5797#issuecomment-1862190442
> is that reslaunching engine a sync operation
I think we should aim to make this a synchronous operation
> and what happen if the restaring engine breaks the timeouts for either launching or session
I think it's similar to the error that occurs when opening a session
> 5. authorization first. What happens if the current engine is used by other sessions even after deregisted. In what case and combination situation allows one to restart the engine?
The engine will go offline gracefully without affecting other sessions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and restart engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor closed pull request #5871: [KYUUBI #5797] Support to describe engine and restart engine with command in current session
URL: https://github.com/apache/kyuubi/pull/5871
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435908782
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
> in case that other operations access the `_client` during restarting engine.
This seems difficult to avoid
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1432240293
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/command/DescribeEngine.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.plan.command
+
+import scala.collection.mutable.ListBuffer
+
+import org.apache.kyuubi.operation.IterableFetchIterator
+import org.apache.kyuubi.session.{KyuubiSession, KyuubiSessionImpl}
+import org.apache.kyuubi.shaded.hive.service.rpc.thrift.TTypeId
+import org.apache.kyuubi.sql.schema.{Column, Row, Schema}
+
+/**
+ * A runnable node for description the current session.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * [DESC|DESCRIBE] ENGINE;
+ * }}}
+ */
+case class DescribeEngine() extends RunnableCommand {
+
+ override def run(kyuubiSession: KyuubiSession): Unit = {
+ val rows = Seq(kyuubiSession).map { session =>
+ val client = session.asInstanceOf[KyuubiSessionImpl].client
+ val values = new ListBuffer[String]()
+ values += client.engineId.getOrElse("")
+ values += client.engineName.getOrElse("")
+ values += client.engineUrl.getOrElse("")
+ Row(values.toList)
+ }
+ iter = new IterableFetchIterator(rows)
+ }
+
+ override def resultSchema: Schema = {
+ Schema(DescribeEngine.outputCols().toList)
+ }
+
+ override val statement: String = "Describe Engine"
+}
+
+object DescribeEngine {
+
+ def outputCols(): Seq[Column] = {
+ Seq(
+ Column("engine id", TTypeId.STRING_TYPE, Some("Kyuubi engine identify")),
+ Column("name", TTypeId.STRING_TYPE, Some("Kyuubi engine name")),
+ Column("url", TTypeId.STRING_TYPE, Some("Kyuubi engine url")))
Review Comment:
> How about adding `ENGINE_` prefix for clearification as `ENGINE_ID`, `ENGINE_NAME`, `ENGINE_URL` ?
Sounds good.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/command/DescribeEngine.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.plan.command
+
+import scala.collection.mutable.ListBuffer
+
+import org.apache.kyuubi.operation.IterableFetchIterator
+import org.apache.kyuubi.session.{KyuubiSession, KyuubiSessionImpl}
+import org.apache.kyuubi.shaded.hive.service.rpc.thrift.TTypeId
+import org.apache.kyuubi.sql.schema.{Column, Row, Schema}
+
+/**
+ * A runnable node for description the current session.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * [DESC|DESCRIBE] ENGINE;
+ * }}}
+ */
+case class DescribeEngine() extends RunnableCommand {
+
+ override def run(kyuubiSession: KyuubiSession): Unit = {
+ val rows = Seq(kyuubiSession).map { session =>
+ val client = session.asInstanceOf[KyuubiSessionImpl].client
+ val values = new ListBuffer[String]()
+ values += client.engineId.getOrElse("")
+ values += client.engineName.getOrElse("")
+ values += client.engineUrl.getOrElse("")
+ Row(values.toList)
+ }
+ iter = new IterableFetchIterator(rows)
+ }
+
+ override def resultSchema: Schema = {
+ Schema(DescribeEngine.outputCols().toList)
+ }
+
+ override val statement: String = "Describe Engine"
+}
+
+object DescribeEngine {
+
+ def outputCols(): Seq[Column] = {
+ Seq(
+ Column("engine id", TTypeId.STRING_TYPE, Some("Kyuubi engine identify")),
+ Column("name", TTypeId.STRING_TYPE, Some("Kyuubi engine name")),
+ Column("url", TTypeId.STRING_TYPE, Some("Kyuubi engine url")))
Review Comment:
> How about adding `ENGINE_` prefix for clearification as `ENGINE_ID`, `ENGINE_NAME`, `ENGINE_URL` ?
SGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435056197
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
This will re-execute `openEngineSession` and we need to consider whether some states are as expected.
+ `openedTime` will be reset: `sessionEvent.openedTime = System.currentTimeMillis()`
+ do we need to update `engineLastAlive`?
+ `EngineRef#builder` is changed, is it thread safe?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435065995
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala:
##########
@@ -316,7 +316,7 @@ private[kyuubi] class EngineRef(
def deregister(discoveryClient: DiscoveryClient, hostPort: (String, Int)): Unit =
tryWithLock(discoveryClient) {
if (discoveryClient.getServerHost(engineSpace) == Option(hostPort)) {
- discoveryClient.delete(engineSpace)
+ discoveryClient.delete(engineSpace, deleteChildren = true)
Review Comment:
`deleteChildren = true` Is this required?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435254187
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala:
##########
@@ -316,7 +316,7 @@ private[kyuubi] class EngineRef(
def deregister(discoveryClient: DiscoveryClient, hostPort: (String, Int)): Unit =
tryWithLock(discoveryClient) {
if (discoveryClient.getServerHost(engineSpace) == Option(hostPort)) {
- discoveryClient.delete(engineSpace)
+ discoveryClient.delete(engineSpace, deleteChildren = true)
Review Comment:
I make a followup for this: https://github.com/apache/kyuubi/pull/5910
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1432181012
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -118,6 +118,14 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
In non-CONNECTION mode, how about current session does not kill engine but start a new one？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1868425192
> please rebase your code.
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and restart engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1871664411
> 1. separate two features in each PR, which are not coupled with each other.
For `restart engine` is not simple, i will separate two features in each PR.
When multi-thread uses a session to submit operations, `restart engine` will close the current `KyuubiSyncThriftClient` session, and this will cause other operations of the client to fail.
Should we give more detailed descriptions and usage cases?
like https://github.com/apache/kyuubi/issues/5797#issuecomment-1862190442
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1432181012
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -118,6 +118,14 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
In non-CONNECTION mode, how about current session not kill engine but start a new one？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435262797
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/parser/DescribeEngineSuite.scala:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.operation.parser
+
+class DescribeEngineSuite extends ExecutedCommandExecSuite {
+
+ test("desc kyuubi session") {
Review Comment:
```
test("desc/describe kyuubi engine") {
Seq("desc", "describe").foreach { desc =>
.....
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1436036641
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/parser/RestartEngineSuite.scala:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.operation.parser
+
+class RestartEngineSuite extends ExecutedCommandExecSuite {
+
+ test("Restart kyuubi engine") {
+ withJdbcStatement() { statement =>
+ var resultSet = statement.executeQuery("KYUUBI DESC ENGINE")
+ assert(resultSet.next())
+ assert(resultSet.getMetaData.getColumnCount == 3)
+ assert(resultSet.getMetaData.getColumnName(1) == "ENGINE_ID")
+ assert(resultSet.getMetaData.getColumnName(2) == "ENGINE_NAME")
+ assert(resultSet.getMetaData.getColumnName(3) == "ENGINE_URL")
+ val engineDied = resultSet.getString("ENGINE_ID")
+
+ resultSet = statement.executeQuery("KYUUBI RESTART ENGINE")
+ assert(resultSet.next())
+ assert(resultSet.getMetaData.getColumnCount == 3)
+ assert(resultSet.getMetaData.getColumnName(1) == "ENGINE_ID")
+ assert(resultSet.getMetaData.getColumnName(2) == "ENGINE_NAME")
+ assert(resultSet.getMetaData.getColumnName(3) == "ENGINE_URL")
+
+ val engineAlive = resultSet.getString("ENGINE_ID")
+ assert(engineDied != engineAlive)
+ }
Review Comment:
Please execute a select statement again after restarting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and restart engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1868898873
> clarify the gap between description and implementation , as the title and description says de-register engine while it implements re-start engine
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435914113
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/command/RunnableCommand.scala:
##########
@@ -30,6 +30,10 @@ trait RunnableCommand extends KyuubiTreeNode {
protected var isAdmin: Boolean = false
+ val statement: String
Review Comment:
`Execute xxx Command` sounds good, I will fix it in another issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435056197
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
This will re-execute `openEngineSession` and we need to consider whether some states are as expected.
+ `openedTime` will be reset: `sessionEvent.openedTime = System.currentTimeMillis()`
+ do we need to update `engineLastAlive`?
+ `EngineRef#builder` is changed, volatile required?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and restart engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1869178570
Some errors occurred in the `BatchesResource` test, try to understand the relevant code to solve it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435260592
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
Review Comment:
you can save the original `host port` info in the KyuubiSessionImpl.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1436036044
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/parser/RestartEngineSuite.scala:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.operation.parser
+
+class RestartEngineSuite extends ExecutedCommandExecSuite {
+
+ test("Restart kyuubi engine") {
+ withJdbcStatement() { statement =>
+ var resultSet = statement.executeQuery("KYUUBI DESC ENGINE")
+ assert(resultSet.next())
+ assert(resultSet.getMetaData.getColumnCount == 3)
+ assert(resultSet.getMetaData.getColumnName(1) == "ENGINE_ID")
+ assert(resultSet.getMetaData.getColumnName(2) == "ENGINE_NAME")
+ assert(resultSet.getMetaData.getColumnName(3) == "ENGINE_URL")
+ val engineDied = resultSet.getString("ENGINE_ID")
+
+ resultSet = statement.executeQuery("KYUUBI RESTART ENGINE")
+ assert(resultSet.next())
+ assert(resultSet.getMetaData.getColumnCount == 3)
+ assert(resultSet.getMetaData.getColumnName(1) == "ENGINE_ID")
+ assert(resultSet.getMetaData.getColumnName(2) == "ENGINE_NAME")
+ assert(resultSet.getMetaData.getColumnName(3) == "ENGINE_URL")
+
+ val engineAlive = resultSet.getString("ENGINE_ID")
+ assert(engineDied != engineAlive)
+ }
+ }
+
+ test("Restart kyuubi engine multiple threads ") {
Review Comment:
This implementation does not seem to reflect multiple threads.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435763336
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
in case that other operations access the `_client` during restarting engine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435763199
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
You shall need to wrap this method with `_client.withLockAcquired`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor closed pull request #5871: [KYUUBI #5797] Support to describe engine and de-register engine with command in current session
URL: https://github.com/apache/kyuubi/pull/5871
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435056197
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
This will re-execute `openEngineSession` and we need to consider whether some states are as expected.
+ `openedTime` will be reset: `sessionEvent.openedTime = System.currentTimeMillis()`
+ do we need to update `engineLastAlive`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435759268
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
fixed `engineLastAlive `, `EngineRef#builder`.
I think `openedTime ` means engine openedTime, so we don't need to update `openedTime`.And i changed the `org.apache.kyuubi.events.KyuubiSessionEvent#openedTime` comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435913289
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
> Or we need to add `ReadWriteLock` for the `client` method of `KyuubiSessionImpl`.
submit `Operation` from multiple threads will meet problem , I will fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "wForget (via GitHub)" <gi...@apache.org>.
wForget commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435909937
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
+ }
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
Or we need to add `ReadWriteLock` for the `client` method of `KyuubiSessionImpl`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1867671122
@turboFei @pan3793 cc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1435260592
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -119,6 +119,19 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ withDiscoveryClient(sessionConf) { discoveryClient =>
+ engine.deregister(
+ discoveryClient,
+ engine.getOrCreate(discoveryClient, launchEngineOp.getOperationLog))
Review Comment:
you can save the `host port` info in the KyuubiSessionImpl.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1868876282
I have some concerns and suggestions,
1. separate two features in each PR, which are not coupled with each other.
2. clarify the gap between description and implementation , as the title and description says `de-register engine` while it implements `re-start engine`
3. describe the design and reason why we have to have `restarting engine`, which is not that reasonable to me
4. is that reslaunching engine a sync operation, and what happen if the restaring engine breaks the timeouts for either launching or session
5. authorization first. What happens if the current engine is used by other sessions even after deregisted. In what case and combination situation allows one to restart the engine?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#issuecomment-1868416404
please rebase your code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1431396034
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/command/DescribeEngine.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.sql.plan.command
+
+import scala.collection.mutable.ListBuffer
+
+import org.apache.kyuubi.operation.IterableFetchIterator
+import org.apache.kyuubi.session.{KyuubiSession, KyuubiSessionImpl}
+import org.apache.kyuubi.shaded.hive.service.rpc.thrift.TTypeId
+import org.apache.kyuubi.sql.schema.{Column, Row, Schema}
+
+/**
+ * A runnable node for description the current session.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * [DESC|DESCRIBE] ENGINE;
+ * }}}
+ */
+case class DescribeEngine() extends RunnableCommand {
+
+ override def run(kyuubiSession: KyuubiSession): Unit = {
+ val rows = Seq(kyuubiSession).map { session =>
+ val client = session.asInstanceOf[KyuubiSessionImpl].client
+ val values = new ListBuffer[String]()
+ values += client.engineId.getOrElse("")
+ values += client.engineName.getOrElse("")
+ values += client.engineUrl.getOrElse("")
+ Row(values.toList)
+ }
+ iter = new IterableFetchIterator(rows)
+ }
+
+ override def resultSchema: Schema = {
+ Schema(DescribeEngine.outputCols().toList)
+ }
+
+ override val statement: String = "Describe Engine"
+}
+
+object DescribeEngine {
+
+ def outputCols(): Seq[Column] = {
+ Seq(
+ Column("engine id", TTypeId.STRING_TYPE, Some("Kyuubi engine identify")),
+ Column("name", TTypeId.STRING_TYPE, Some("Kyuubi engine name")),
+ Column("url", TTypeId.STRING_TYPE, Some("Kyuubi engine url")))
Review Comment:
How about adding `ENGINE_` prefix for clearification as `ENGINE_ID`, `ENGINE_NAME`, `ENGINE_URL` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5797] Support to describe engine and de-register engine with command in current session [kyuubi]
Posted by "Kwafoor (via GitHub)" <gi...@apache.org>.
Kwafoor commented on code in PR #5871:
URL: https://github.com/apache/kyuubi/pull/5871#discussion_r1432181012
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -118,6 +118,14 @@ class KyuubiSessionImpl(
engineLastAlive = System.currentTimeMillis()
}
+ def reLaunchEngine(): Unit = handleSessionException {
+ _client.closeSession()
+ engineLaunched = false
+ launchEngineOp = sessionManager.operationManager
+ .newLaunchEngineOperation(this, shouldRunAsync = false)
+ runOperation(launchEngineOp)
Review Comment:
In non-CONNECTION mode, how about current session not kill but start a new engine？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org