You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/01/24 01:00:36 UTC
[GitHub] [incubator-kyuubi] hddong opened a new pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
hddong opened a new pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
Add trino ExecuteStatement
### _How was this patch tested?_
- [X] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [X] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f4d6cbb) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.33%`.
> The diff coverage is `67.55%`.
> :exclamation: Current head f4d6cbb differs from pull request most recent head 067bde7. Consider uploading reports for the commit 067bde7 to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.88% +0.33%
- Complexity 247 317 +70
============================================
Files 274 288 +14
Lines 13525 14174 +649
Branches 1738 1800 +62
============================================
+ Hits 8054 8488 +434
- Misses 4777 4965 +188
- Partials 694 721 +27
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `36.36% <36.36%> (ø)` | |
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `51.72% <50.00%> (+51.72%)` | :arrow_up: |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `66.26% <72.00%> (+0.38%)` | :arrow_up: |
| [.../org/apache/kyuubi/engine/trino/TrinoContext.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29udGV4dC5zY2FsYQ==) | `83.33% <75.00%> (-16.67%)` | :arrow_down: |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `76.19% <76.19%> (ø)` | |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `97.61% <97.61%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `100.00% <100.00%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| ... and [65 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...067bde7](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1020751276
Took a rough look overall, the trinoContext/httpClient is singleton?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1020751276
Took a rough look overall, the trinoContext/httpClient is singleton?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802302244
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoContext.scala
##########
@@ -17,24 +17,9 @@
package org.apache.kyuubi.engine.trino
-import java.util.concurrent.atomic.AtomicReference
-
import io.trino.client.ClientSession
import okhttp3.OkHttpClient
-class TrinoContext(
- val httpClient: OkHttpClient,
- val clientSession: AtomicReference[ClientSession]) {
-
- def getClientSession: ClientSession = clientSession.get
-
- def setCurrentSchema(schema: String): Unit = {
- clientSession.set(ClientSession.builder(clientSession.get).withSchema(schema).build())
- }
-
-}
-
-object TrinoContext {
- def apply(httpClient: OkHttpClient, clientSession: ClientSession): TrinoContext =
- new TrinoContext(httpClient, new AtomicReference(clientSession))
-}
+case class TrinoContext(
+ httpClient: OkHttpClient,
+ var clientSession: ClientSession)
Review comment:
The new configs was store in `StatementClient`, we need to ues it to update session's `ClientSession`. And `ClientSession` can not update directly, so we reset it for each ExcuteStatement.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802368390
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoStatement.scala
##########
@@ -99,50 +99,45 @@ class TrinoStatement(trinoContext: TrinoContext, kyuubiConf: KyuubiConf, sql: St
val rowBuffer = new ArrayList[List[Any]](MAX_BUFFERED_ROWS)
var bufferStart = System.nanoTime()
val result = ArrayBuffer[List[Any]]()
- try {
- breakable {
- while (!dataProcessing.isCompleted) {
- val atEnd = drainDetectingEnd(rowQueue, rowBuffer, MAX_BUFFERED_ROWS, END_TOKEN)
- if (!atEnd) {
- // Flush if needed
- if (rowBuffer.size() >= MAX_BUFFERED_ROWS ||
- Duration.fromNanos(bufferStart).compareTo(MAX_BUFFER_TIME) >= 0) {
- result ++= rowBuffer.asScala
- rowBuffer.clear()
- bufferStart = System.nanoTime()
- }
-
- val row = rowQueue.poll(MAX_BUFFER_TIME.toMillis, duration.MILLISECONDS)
- row match {
- case END_TOKEN => break
- case null =>
- case _ => rowBuffer.add(row)
- }
+
+ breakable {
Review comment:
can we not use breakable to end a while loop?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (351e2bc) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.24%`.
> The diff coverage is `67.48%`.
> :exclamation: Current head 351e2bc differs from pull request most recent head f4d6cbb. Consider uploading reports for the commit f4d6cbb to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.79% +0.24%
- Complexity 247 321 +74
============================================
Files 274 281 +7
Lines 13525 13971 +446
Branches 1738 1781 +43
============================================
+ Hits 8054 8354 +300
- Misses 4777 4906 +129
- Partials 694 711 +17
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `36.36% <36.36%> (ø)` | |
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `51.72% <50.00%> (+51.72%)` | :arrow_up: |
| [.../org/apache/kyuubi/engine/trino/TrinoContext.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29udGV4dC5zY2FsYQ==) | `66.66% <66.66%> (-33.34%)` | :arrow_down: |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `76.19% <76.19%> (ø)` | |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `97.61% <97.61%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `100.00% <100.00%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `71.76% <100.00%> (+5.88%)` | :arrow_up: |
| ... and [43 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...f4d6cbb](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791413606
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionManager.scala
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.engine.trino.session
+
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+import org.apache.hive.service.rpc.thrift.TProtocolVersion
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Utils
+import org.apache.kyuubi.Utils.currentUser
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.ENGINE_OPERATION_LOG_DIR_ROOT
+import org.apache.kyuubi.config.KyuubiConf.ENGINE_SHARE_LEVEL
+import org.apache.kyuubi.engine.ShareLevel
+import org.apache.kyuubi.engine.trino.TrinoConf
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine
+import org.apache.kyuubi.engine.trino.operation.TrinoOperationManager
+import org.apache.kyuubi.session.SessionHandle
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoSessionManager
+ extends SessionManager("TrinoSessionManager") {
+
+ val operationManager = new TrinoOperationManager()
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ val absPath = Utils.getAbsolutePathFromWork(conf.get(ENGINE_OPERATION_LOG_DIR_ROOT))
+ _operationLogRoot = Some(absPath.toAbsolutePath.toString)
+ super.initialize(conf)
+ }
+
+ override def openSession(
+ protocol: TProtocolVersion,
+ user: String,
+ password: String,
+ ipAddress: String,
+ conf: Map[String, String]): SessionHandle = {
+ info(s"Opening session for $user@$ipAddress")
+ val trinoContext = createTrinoContext()
+ val sessionImpl =
+ new TrinoSessionImpl(protocol, user, password, ipAddress, conf, this, trinoContext)
+
+ try {
+ val handle = sessionImpl.handle
+ sessionImpl.open()
+ setSession(handle, sessionImpl)
+ info(s"$user's trino session with $handle is opened, current opening sessions" +
+ s" $getOpenSessionCount")
+ handle
+ } catch {
+ case e: Exception =>
+ sessionImpl.close()
+ throw KyuubiSQLException(e)
+ }
+ }
+
+ def createTrinoContext(): TrinoContext = {
+ val connectionUrl = conf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_URL).getOrElse(
+ throw KyuubiSQLException("Trino server url can not be null!"))
+ val catalog = conf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_CATALOG).getOrElse(
+ throw KyuubiSQLException("Trino default catalog can not be null!"))
+ val user = conf.getOption("kyuubi.trino.user").getOrElse(currentUser)
+ val clientRequestTimeout = conf.get(TrinoConf.CLIENT_REQUEST_TIMEOUT)
+
+ val session = new ClientSession(
+ URI.create(connectionUrl),
+ user,
+ Optional.empty(),
+ "kyuubi",
+ Optional.empty(),
+ Set[String]().asJava,
+ null,
+ catalog,
+ null,
+ null,
+ ZoneId.systemDefault(),
+ Locale.getDefault,
+ Map[String, String]().asJava,
+ Map[String, String]().asJava,
+ Map[String, String]().asJava,
+ Map[String, ClientSelectedRole]().asJava,
+ Map[String, String]().asJava,
Review comment:
Collections.emptyMap()?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c5072db) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.44%`.
> The diff coverage is `68.52%`.
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.99% +0.44%
- Complexity 247 313 +66
============================================
Files 274 280 +6
Lines 13525 13856 +331
Branches 1738 1759 +21
============================================
+ Hits 8054 8313 +259
- Misses 4777 4833 +56
- Partials 694 710 +16
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `41.66% <41.66%> (ø)` | |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `78.26% <78.26%> (ø)` | |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `80.00% <80.00%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `82.75% <83.63%> (+82.75%)` | :arrow_up: |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `87.50% <87.50%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `66.99% <100.00%> (+0.32%)` | :arrow_up: |
| [...ache/kyuubi/ha/client/KyuubiServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9LeXV1YmlTZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [31 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...c5072db](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791304761
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
Review comment:
we can set the name = “ TrinoBackendService” directly
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791306045
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoConf.scala
##########
@@ -30,4 +32,11 @@ object TrinoConf {
.version("1.5.0")
.intConf
.createWithDefault(3)
+
+ val CLIENT_REQUEST_TIMEOUT: ConfigEntry[Long] =
+ buildConf("trino.client.request.timeout")
+ .doc("Client request will be closed when it's not accessed for this duration of time")
Review comment:
the `client` here is very ambiguous
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791410483
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/TrinoOperation.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.engine.trino.operation
+
+import java.io.IOException
+
+import io.trino.client.Column
+import io.trino.client.StatementClient
+import org.apache.hive.service.rpc.thrift.TRowSet
+import org.apache.hive.service.rpc.thrift.TTableSchema
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Utils
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.engine.trino.schema.RowSet
+import org.apache.kyuubi.engine.trino.schema.SchemaHelper
+import org.apache.kyuubi.engine.trino.session.TrinoSessionImpl
+import org.apache.kyuubi.operation.AbstractOperation
+import org.apache.kyuubi.operation.FetchIterator
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_FIRST
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_NEXT
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_PRIOR
+import org.apache.kyuubi.operation.FetchOrientation.FetchOrientation
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.operation.OperationState.OperationState
+import org.apache.kyuubi.operation.OperationType.OperationType
+import org.apache.kyuubi.operation.log.OperationLog
+import org.apache.kyuubi.session.Session
+
+abstract class TrinoOperation(opType: OperationType, session: Session)
+ extends AbstractOperation(opType, session) {
+
+ protected val trinoContext: TrinoContext = session.asInstanceOf[TrinoSessionImpl].trinoContext
+
+ protected var trino: StatementClient = _
+
+ protected var schema: List[Column] = _
+
+ protected var iter: FetchIterator[List[Any]] = _
+
+ override def getResultSetSchema: TTableSchema = SchemaHelper.toTTableSchema(schema)
+
+ override def getNextRowSet(order: FetchOrientation, rowSetSize: Int): TRowSet = {
+ validateDefaultFetchOrientation(order)
+ assertState(OperationState.FINISHED)
+ setHasResultSet(true)
+ order match {
+ case FETCH_NEXT => iter.fetchNext()
+ case FETCH_PRIOR => iter.fetchPrior(rowSetSize);
+ case FETCH_FIRST => iter.fetchAbsolute(0);
+ }
+ val taken = iter.take(rowSetSize)
+ val resultRowSet = RowSet.toTRowSet(taken.toList, schema, getProtocolVersion)
+ resultRowSet.setStartRowOffset(iter.getPosition)
+ resultRowSet
+ }
+
+ override protected def beforeRun(): Unit = {
+ setHasResultSet(true)
+ setState(OperationState.RUNNING)
+ }
+
+ override protected def afterRun(): Unit = {
+ state.synchronized {
+ if (!isTerminalState(state)) {
+ setState(OperationState.FINISHED)
+ }
+ }
+ OperationLog.removeCurrentOperationLog()
+ }
+
+ override def cancel(): Unit = {
+ cleanup(OperationState.CANCELED)
+ }
+
+ protected def cleanup(targetState: OperationState): Unit = state.synchronized {
+ if (!isTerminalState(state)) {
+ setState(targetState)
+ Option(getBackgroundHandle).foreach(_.cancel(true))
+ }
+ }
+
+ override def close(): Unit = {
+ cleanup(OperationState.CLOSED)
+ try {
+ trino.close()
+ getOperationLog.foreach(_.close())
+ } catch {
+ case e: IOException =>
+ error(e.getMessage, e)
+ }
+ }
+
+ override def shouldRunAsync: Boolean = false
+
+ protected def onError(cancel: Boolean = false): PartialFunction[Throwable, Unit] = {
+ // We should use Throwable instead of Exception since `java.lang.NoClassDefFoundError`
+ // could be thrown.
+ case e: Throwable =>
+ if (cancel && trino.isRunning) trino.cancelLeafStage()
+ state.synchronized {
+ val errMsg = Utils.stringifyException(e)
+ if (state == OperationState.TIMEOUT) {
+ val ke = KyuubiSQLException(s"Timeout operating $opType: $errMsg")
+ setOperationException(ke)
+ throw ke
+ } else if (isTerminalState(state)) {
+ setOperationException(KyuubiSQLException(errMsg))
+ warn(s"Ignore exception in terminal state with $statementId: $errMsg")
+ } else {
+ setState(OperationState.ERROR)
Review comment:
move to L130？
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802294094
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/ExecuteStatement.scala
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.engine.trino.operation
+
+import java.util.concurrent.RejectedExecutionException
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Logging
+import org.apache.kyuubi.engine.trino.TrinoStatement
+import org.apache.kyuubi.operation.ArrayFetchIterator
+import org.apache.kyuubi.operation.IterableFetchIterator
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.operation.OperationType
+import org.apache.kyuubi.operation.log.OperationLog
+import org.apache.kyuubi.session.Session
+
+class ExecuteStatement(
+ session: Session,
+ override val statement: String,
+ override val shouldRunAsync: Boolean,
+ incrementalCollect: Boolean)
+ extends TrinoOperation(OperationType.EXECUTE_STATEMENT, session) with Logging {
+
+ private val operationLog: OperationLog = OperationLog.createOperationLog(session, getHandle)
+ override def getOperationLog: Option[OperationLog] = Option(operationLog)
+
+ override protected def beforeRun(): Unit = {
+ OperationLog.setCurrentOperationLog(operationLog)
+ setState(OperationState.PENDING)
+ setHasResultSet(true)
+ }
+
+ override protected def afterRun(): Unit = {
+ OperationLog.removeCurrentOperationLog()
+ }
+
+ override protected def runInternal(): Unit = {
+ val trinoStatement = TrinoStatement(trinoContext, session.sessionManager.getConf, statement)
+ trino = trinoStatement.getTrinoClient
+ if (shouldRunAsync) {
+ val asyncOperation = new Runnable {
+ override def run(): Unit = {
+ OperationLog.setCurrentOperationLog(operationLog)
+ executeStatement(trinoStatement)
+ }
+ }
+
+ try {
+ val trinoSessionManager = session.sessionManager
+ val backgroundHandle = trinoSessionManager.submitBackgroundOperation(asyncOperation)
+ setBackgroundHandle(backgroundHandle)
+ } catch {
+ case rejected: RejectedExecutionException =>
+ setState(OperationState.ERROR)
+ val ke =
+ KyuubiSQLException("Error submitting query in background, query rejected", rejected)
+ setOperationException(ke)
+ throw ke
+ }
+ } else {
+ executeStatement(trinoStatement)
+ }
+ }
+
+ private def executeStatement(trinoStatement: TrinoStatement): Unit = {
+ setState(OperationState.RUNNING)
+ try {
+ schema = trinoStatement.getColumns
+ val resultSet = trinoStatement.execute()
+ iter =
+ if (incrementalCollect) {
+ info("Execute in incremental collect mode")
+ new IterableFetchIterator(resultSet)
Review comment:
Yes, it's different from spark.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802304134
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoContext.scala
##########
@@ -17,24 +17,9 @@
package org.apache.kyuubi.engine.trino
-import java.util.concurrent.atomic.AtomicReference
-
import io.trino.client.ClientSession
import okhttp3.OkHttpClient
-class TrinoContext(
- val httpClient: OkHttpClient,
- val clientSession: AtomicReference[ClientSession]) {
-
- def getClientSession: ClientSession = clientSession.get
-
- def setCurrentSchema(schema: String): Unit = {
- clientSession.set(ClientSession.builder(clientSession.get).withSchema(schema).build())
- }
-
-}
-
-object TrinoContext {
- def apply(httpClient: OkHttpClient, clientSession: ClientSession): TrinoContext =
- new TrinoContext(httpClient, new AtomicReference(clientSession))
-}
+case class TrinoContext(
+ httpClient: OkHttpClient,
+ var clientSession: ClientSession)
Review comment:
After `use schema` execute, we can get the new schame by `StatementClient.getSetSchema` and set it to session's `ClientSession`, so the next stetment will run with correct `schema`.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802314344
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/ExecuteStatement.scala
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.engine.trino.operation
+
+import java.util.concurrent.RejectedExecutionException
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Logging
+import org.apache.kyuubi.engine.trino.TrinoStatement
+import org.apache.kyuubi.operation.ArrayFetchIterator
+import org.apache.kyuubi.operation.IterableFetchIterator
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.operation.OperationType
+import org.apache.kyuubi.operation.log.OperationLog
+import org.apache.kyuubi.session.Session
+
+class ExecuteStatement(
+ session: Session,
+ override val statement: String,
+ override val shouldRunAsync: Boolean,
+ incrementalCollect: Boolean)
+ extends TrinoOperation(OperationType.EXECUTE_STATEMENT, session) with Logging {
+
+ private val operationLog: OperationLog = OperationLog.createOperationLog(session, getHandle)
+ override def getOperationLog: Option[OperationLog] = Option(operationLog)
+
+ override protected def beforeRun(): Unit = {
+ OperationLog.setCurrentOperationLog(operationLog)
+ setState(OperationState.PENDING)
+ setHasResultSet(true)
+ }
+
+ override protected def afterRun(): Unit = {
+ OperationLog.removeCurrentOperationLog()
+ }
+
+ override protected def runInternal(): Unit = {
+ val trinoStatement = TrinoStatement(trinoContext, session.sessionManager.getConf, statement)
+ trino = trinoStatement.getTrinoClient
+ if (shouldRunAsync) {
+ val asyncOperation = new Runnable {
+ override def run(): Unit = {
+ OperationLog.setCurrentOperationLog(operationLog)
+ executeStatement(trinoStatement)
+ }
+ }
+
+ try {
+ val trinoSessionManager = session.sessionManager
+ val backgroundHandle = trinoSessionManager.submitBackgroundOperation(asyncOperation)
+ setBackgroundHandle(backgroundHandle)
+ } catch {
+ case rejected: RejectedExecutionException =>
+ setState(OperationState.ERROR)
+ val ke =
+ KyuubiSQLException("Error submitting query in background, query rejected", rejected)
+ setOperationException(ke)
+ throw ke
+ }
+ } else {
+ executeStatement(trinoStatement)
+ }
+ }
+
+ private def executeStatement(trinoStatement: TrinoStatement): Unit = {
+ setState(OperationState.RUNNING)
+ try {
+ schema = trinoStatement.getColumns
+ val resultSet = trinoStatement.execute()
Review comment:
we can remove the error handling inside `trinoStatement.execute()`
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1034416039
@yaooqinn @pan3793 : Thanks for review, merging to master.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong closed pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong closed pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] cody195 commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
cody195 commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r790554649
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoConf.scala
##########
@@ -30,4 +30,11 @@ object TrinoConf {
.version("1.5.0")
.intConf
.createWithDefault(3)
+
+ val CLIENT_REQUEST_TIMEOUT: ConfigEntry[Int] =
+ buildConf("trino.client.request.timeout")
+ .doc("Client request will be closed when it's not accessed for this duration of time")
+ .version("1.5.0")
+ .intConf
+ .createWithDefault(2)
Review comment:
hello, what's the unit about this key?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a20f2d0) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.44%`.
> The diff coverage is `68.52%`.
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.99% +0.44%
- Complexity 247 314 +67
============================================
Files 274 280 +6
Lines 13525 13856 +331
Branches 1738 1759 +21
============================================
+ Hits 8054 8313 +259
- Misses 4777 4833 +56
- Partials 694 710 +16
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `41.66% <41.66%> (ø)` | |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `78.26% <78.26%> (ø)` | |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `80.00% <80.00%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `82.75% <83.63%> (+82.75%)` | :arrow_up: |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `87.50% <87.50%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `66.99% <100.00%> (+0.32%)` | :arrow_up: |
| [...ache/kyuubi/ha/client/KyuubiServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9LeXV1YmlTZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [32 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...a20f2d0](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c5072db) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.44%`.
> The diff coverage is `68.52%`.
> :exclamation: Current head c5072db differs from pull request most recent head a20f2d0. Consider uploading reports for the commit a20f2d0 to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.99% +0.44%
- Complexity 247 313 +66
============================================
Files 274 280 +6
Lines 13525 13856 +331
Branches 1738 1759 +21
============================================
+ Hits 8054 8313 +259
- Misses 4777 4833 +56
- Partials 694 710 +16
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `41.66% <41.66%> (ø)` | |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `78.26% <78.26%> (ø)` | |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `80.00% <80.00%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `82.75% <83.63%> (+82.75%)` | :arrow_up: |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `87.50% <87.50%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `66.99% <100.00%> (+0.32%)` | :arrow_up: |
| [...ache/kyuubi/ha/client/KyuubiServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9LeXV1YmlTZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [31 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...a20f2d0](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791310061
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
##########
@@ -17,19 +17,128 @@
package org.apache.kyuubi.engine.trino
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+
+import org.apache.kyuubi.KyuubiSQLException
import org.apache.kyuubi.Logging
+import org.apache.kyuubi.Utils.TRINO_ENGINE_SHUTDOWN_PRIORITY
+import org.apache.kyuubi.Utils.addShutdownHook
+import org.apache.kyuubi.Utils.currentUser
import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.countDownLatch
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.currentEngine
+import org.apache.kyuubi.ha.HighAvailabilityConf.HA_ZK_CONN_RETRY_POLICY
+import org.apache.kyuubi.ha.client.RetryPolicies
+import org.apache.kyuubi.service.Serverable
import org.apache.kyuubi.util.SignalRegister
+case class TrinoSqlEngine(trino: TrinoContext)
+ extends Serverable("TrinoSQLEngine") {
+
+ override val backendService = new TrinoBackendService(trino)
+
+ override val frontendServices = Seq(new TrinoTBinaryFrontendService(this))
+
+ override def start(): Unit = {
+ super.start()
+ // Start engine self-terminating checker after all services are ready and it can be reached by
+ // all servers in engine spaces.
+ backendService.sessionManager.startTerminatingChecker(() => {
+ assert(currentEngine.isDefined)
+ currentEngine.get.stop()
+ })
+ }
+
+ override protected def stopServer(): Unit = {
+ countDownLatch.countDown()
+ }
+}
+
object TrinoSqlEngine extends Logging {
+ private val countDownLatch = new CountDownLatch(1)
val kyuubiConf: KyuubiConf = KyuubiConf()
+ var currentEngine: Option[TrinoSqlEngine] = None
+
+ def createTrinoContext(): TrinoContext = {
Review comment:
why we need a trinoContext here?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791305562
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
+ def this(trino: TrinoContext) =
+ this(classOf[TrinoBackendService].getSimpleName, trino)
+
+ override val sessionManager: SessionManager = new TrinoSessionManager(trinoContext)
+
+ def trinoSession: TrinoContext = trinoContext
Review comment:
or simply make `val trinoContext: TrinoContext`?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791392575
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
Review comment:
Tanks, done.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r801514242
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoContext.scala
##########
@@ -17,24 +17,9 @@
package org.apache.kyuubi.engine.trino
-import java.util.concurrent.atomic.AtomicReference
-
import io.trino.client.ClientSession
import okhttp3.OkHttpClient
-class TrinoContext(
- val httpClient: OkHttpClient,
- val clientSession: AtomicReference[ClientSession]) {
-
- def getClientSession: ClientSession = clientSession.get
-
- def setCurrentSchema(schema: String): Unit = {
- clientSession.set(ClientSession.builder(clientSession.get).withSchema(schema).build())
- }
-
-}
-
-object TrinoContext {
- def apply(httpClient: OkHttpClient, clientSession: ClientSession): TrinoContext =
- new TrinoContext(httpClient, new AtomicReference(clientSession))
-}
+case class TrinoContext(
+ httpClient: OkHttpClient,
+ var clientSession: ClientSession)
Review comment:
How about we make `clientSession` to TrinoStatement? or somewhere in TrinoOperation? It's not a session level context.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791391845
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
+ def this(trino: TrinoContext) =
+ this(classOf[TrinoBackendService].getSimpleName, trino)
+
+ override val sessionManager: SessionManager = new TrinoSessionManager(trinoContext)
+
+ def trinoSession: TrinoContext = trinoContext
Review comment:
Done.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (69d7d9b) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.21%`.
> The diff coverage is `66.14%`.
> :exclamation: Current head 69d7d9b differs from pull request most recent head 351e2bc. Consider uploading reports for the commit 351e2bc to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.76% +0.21%
- Complexity 247 320 +73
============================================
Files 274 281 +7
Lines 13525 13969 +444
Branches 1738 1780 +42
============================================
+ Hits 8054 8349 +295
- Misses 4777 4908 +131
- Partials 694 712 +18
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `36.36% <36.36%> (ø)` | |
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `51.72% <50.00%> (+51.72%)` | :arrow_up: |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `87.50% <87.50%> (ø)` | |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `90.19% <90.19%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `100.00% <100.00%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `64.48% <100.00%> (-2.19%)` | :arrow_down: |
| [...ache/kyuubi/ha/client/KyuubiServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9LeXV1YmlTZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [38 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...351e2bc](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a20f2d0) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.44%`.
> The diff coverage is `68.52%`.
> :exclamation: Current head a20f2d0 differs from pull request most recent head 9cb757a. Consider uploading reports for the commit 9cb757a to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.99% +0.44%
- Complexity 247 314 +67
============================================
Files 274 280 +6
Lines 13525 13856 +331
Branches 1738 1759 +21
============================================
+ Hits 8054 8313 +259
- Misses 4777 4833 +56
- Partials 694 710 +16
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `41.66% <41.66%> (ø)` | |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `78.26% <78.26%> (ø)` | |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `80.00% <80.00%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `82.75% <83.63%> (+82.75%)` | :arrow_up: |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `87.50% <87.50%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `66.99% <100.00%> (+0.32%)` | :arrow_up: |
| [...ache/kyuubi/ha/client/KyuubiServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9LeXV1YmlTZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [32 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...9cb757a](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (351e2bc) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.24%`.
> The diff coverage is `67.48%`.
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.79% +0.24%
- Complexity 247 321 +74
============================================
Files 274 281 +7
Lines 13525 13971 +446
Branches 1738 1781 +43
============================================
+ Hits 8054 8354 +300
- Misses 4777 4906 +129
- Partials 694 711 +17
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `36.36% <36.36%> (ø)` | |
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `51.72% <50.00%> (+51.72%)` | :arrow_up: |
| [.../org/apache/kyuubi/engine/trino/TrinoContext.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29udGV4dC5zY2FsYQ==) | `66.66% <66.66%> (-33.34%)` | :arrow_down: |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `76.19% <76.19%> (ø)` | |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `97.61% <97.61%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `100.00% <100.00%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `71.76% <100.00%> (+5.88%)` | :arrow_up: |
| ... and [43 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...351e2bc](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791287118
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoConf.scala
##########
@@ -30,4 +30,11 @@ object TrinoConf {
.version("1.5.0")
.intConf
.createWithDefault(3)
+
+ val CLIENT_REQUEST_TIMEOUT: ConfigEntry[Int] =
+ buildConf("trino.client.request.timeout")
+ .doc("Client request will be closed when it's not accessed for this duration of time")
+ .version("1.5.0")
+ .intConf
+ .createWithDefault(2)
Review comment:
> hello, what's the unit about this key?
Thanks, addressed.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791391690
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoConf.scala
##########
@@ -30,4 +32,11 @@ object TrinoConf {
.version("1.5.0")
.intConf
.createWithDefault(3)
+
+ val CLIENT_REQUEST_TIMEOUT: ConfigEntry[Long] =
+ buildConf("trino.client.request.timeout")
+ .doc("Client request will be closed when it's not accessed for this duration of time")
Review comment:
Not sure only doc or key is included, had optimized doc first.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1020850711
> Took a rough look overall, the trinoContext/httpClient is singleton?
Not singleton, create for each session.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1032420562
@yaooqinn @pan3793 : Any more comments?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802312922
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoContext.scala
##########
@@ -17,24 +17,9 @@
package org.apache.kyuubi.engine.trino
-import java.util.concurrent.atomic.AtomicReference
-
import io.trino.client.ClientSession
import okhttp3.OkHttpClient
-class TrinoContext(
- val httpClient: OkHttpClient,
- val clientSession: AtomicReference[ClientSession]) {
-
- def getClientSession: ClientSession = clientSession.get
-
- def setCurrentSchema(schema: String): Unit = {
- clientSession.set(ClientSession.builder(clientSession.get).withSchema(schema).build())
- }
-
-}
-
-object TrinoContext {
- def apply(httpClient: OkHttpClient, clientSession: ClientSession): TrinoContext =
- new TrinoContext(httpClient, new AtomicReference(clientSession))
-}
+case class TrinoContext(
+ httpClient: OkHttpClient,
+ var clientSession: ClientSession)
Review comment:
I see, then the original AtomicRef makes sense to me.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791305441
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
+ def this(trino: TrinoContext) =
+ this(classOf[TrinoBackendService].getSimpleName, trino)
+
+ override val sessionManager: SessionManager = new TrinoSessionManager(trinoContext)
+
+ def trinoSession: TrinoContext = trinoContext
Review comment:
where do we use this?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791310061
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
##########
@@ -17,19 +17,128 @@
package org.apache.kyuubi.engine.trino
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+
+import org.apache.kyuubi.KyuubiSQLException
import org.apache.kyuubi.Logging
+import org.apache.kyuubi.Utils.TRINO_ENGINE_SHUTDOWN_PRIORITY
+import org.apache.kyuubi.Utils.addShutdownHook
+import org.apache.kyuubi.Utils.currentUser
import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.countDownLatch
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.currentEngine
+import org.apache.kyuubi.ha.HighAvailabilityConf.HA_ZK_CONN_RETRY_POLICY
+import org.apache.kyuubi.ha.client.RetryPolicies
+import org.apache.kyuubi.service.Serverable
import org.apache.kyuubi.util.SignalRegister
+case class TrinoSqlEngine(trino: TrinoContext)
+ extends Serverable("TrinoSQLEngine") {
+
+ override val backendService = new TrinoBackendService(trino)
+
+ override val frontendServices = Seq(new TrinoTBinaryFrontendService(this))
+
+ override def start(): Unit = {
+ super.start()
+ // Start engine self-terminating checker after all services are ready and it can be reached by
+ // all servers in engine spaces.
+ backendService.sessionManager.startTerminatingChecker(() => {
+ assert(currentEngine.isDefined)
+ currentEngine.get.stop()
+ })
+ }
+
+ override protected def stopServer(): Unit = {
+ countDownLatch.countDown()
+ }
+}
+
object TrinoSqlEngine extends Logging {
+ private val countDownLatch = new CountDownLatch(1)
val kyuubiConf: KyuubiConf = KyuubiConf()
+ var currentEngine: Option[TrinoSqlEngine] = None
+
+ def createTrinoContext(): TrinoContext = {
Review comment:
why we need a trinoContext here? Looks only SessionManger needs 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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1020850711
> Took a rough look overall, the trinoContext/httpClient is singleton?
Not singleton, create for each session.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791499849
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/TrinoOperation.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.engine.trino.operation
+
+import java.io.IOException
+
+import io.trino.client.Column
+import io.trino.client.StatementClient
+import org.apache.hive.service.rpc.thrift.TRowSet
+import org.apache.hive.service.rpc.thrift.TTableSchema
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Utils
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.engine.trino.schema.RowSet
+import org.apache.kyuubi.engine.trino.schema.SchemaHelper
+import org.apache.kyuubi.engine.trino.session.TrinoSessionImpl
+import org.apache.kyuubi.operation.AbstractOperation
+import org.apache.kyuubi.operation.FetchIterator
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_FIRST
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_NEXT
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_PRIOR
+import org.apache.kyuubi.operation.FetchOrientation.FetchOrientation
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.operation.OperationState.OperationState
+import org.apache.kyuubi.operation.OperationType.OperationType
+import org.apache.kyuubi.operation.log.OperationLog
+import org.apache.kyuubi.session.Session
+
+abstract class TrinoOperation(opType: OperationType, session: Session)
+ extends AbstractOperation(opType, session) {
+
+ protected val trinoContext: TrinoContext = session.asInstanceOf[TrinoSessionImpl].trinoContext
+
+ protected var trino: StatementClient = _
+
+ protected var schema: List[Column] = _
+
+ protected var iter: FetchIterator[List[Any]] = _
+
+ override def getResultSetSchema: TTableSchema = SchemaHelper.toTTableSchema(schema)
+
+ override def getNextRowSet(order: FetchOrientation, rowSetSize: Int): TRowSet = {
+ validateDefaultFetchOrientation(order)
+ assertState(OperationState.FINISHED)
+ setHasResultSet(true)
+ order match {
+ case FETCH_NEXT => iter.fetchNext()
+ case FETCH_PRIOR => iter.fetchPrior(rowSetSize);
+ case FETCH_FIRST => iter.fetchAbsolute(0);
+ }
+ val taken = iter.take(rowSetSize)
+ val resultRowSet = RowSet.toTRowSet(taken.toList, schema, getProtocolVersion)
+ resultRowSet.setStartRowOffset(iter.getPosition)
+ resultRowSet
+ }
+
+ override protected def beforeRun(): Unit = {
+ setHasResultSet(true)
+ setState(OperationState.RUNNING)
+ }
+
+ override protected def afterRun(): Unit = {
+ state.synchronized {
+ if (!isTerminalState(state)) {
+ setState(OperationState.FINISHED)
+ }
+ }
+ OperationLog.removeCurrentOperationLog()
+ }
+
+ override def cancel(): Unit = {
+ cleanup(OperationState.CANCELED)
+ }
+
+ protected def cleanup(targetState: OperationState): Unit = state.synchronized {
+ if (!isTerminalState(state)) {
+ setState(targetState)
+ Option(getBackgroundHandle).foreach(_.cancel(true))
+ }
+ }
+
+ override def close(): Unit = {
+ cleanup(OperationState.CLOSED)
+ try {
+ trino.close()
+ getOperationLog.foreach(_.close())
+ } catch {
+ case e: IOException =>
+ error(e.getMessage, e)
+ }
+ }
+
+ override def shouldRunAsync: Boolean = false
+
+ protected def onError(cancel: Boolean = false): PartialFunction[Throwable, Unit] = {
+ // We should use Throwable instead of Exception since `java.lang.NoClassDefFoundError`
+ // could be thrown.
+ case e: Throwable =>
+ if (cancel && trino.isRunning) trino.cancelLeafStage()
+ state.synchronized {
+ val errMsg = Utils.stringifyException(e)
+ if (state == OperationState.TIMEOUT) {
+ val ke = KyuubiSQLException(s"Timeout operating $opType: $errMsg")
+ setOperationException(ke)
+ throw ke
+ } else if (isTerminalState(state)) {
+ setOperationException(KyuubiSQLException(errMsg))
+ warn(s"Ignore exception in terminal state with $statementId: $errMsg")
+ } else {
+ setState(OperationState.ERROR)
Review comment:
Done
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791500827
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.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.engine.trino.session
+
+import org.apache.hive.service.rpc.thrift.TProtocolVersion
+
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.session.AbstractSession
+import org.apache.kyuubi.session.SessionHandle
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoSessionImpl(
+ protocol: TProtocolVersion,
+ user: String,
+ password: String,
+ ipAddress: String,
+ conf: Map[String, String],
+ sessionManager: SessionManager,
+ val trinoContext: TrinoContext)
+ extends AbstractSession(protocol, user, password, ipAddress, conf, sessionManager) {
+ override val handle: SessionHandle = SessionHandle(protocol)
+
+ override def open(): Unit = {
+ normalizedConf.foreach {
+ case ("use:database", database) => trinoContext.setCurrentSchema(database)
Review comment:
Done
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791304761
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
Review comment:
we can set the name = “ TrinoBackendService” directly
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
+ def this(trino: TrinoContext) =
+ this(classOf[TrinoBackendService].getSimpleName, trino)
+
+ override val sessionManager: SessionManager = new TrinoSessionManager(trinoContext)
+
+ def trinoSession: TrinoContext = trinoContext
Review comment:
where do we use this?
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
+ def this(trino: TrinoContext) =
+ this(classOf[TrinoBackendService].getSimpleName, trino)
+
+ override val sessionManager: SessionManager = new TrinoSessionManager(trinoContext)
+
+ def trinoSession: TrinoContext = trinoContext
Review comment:
or simply make `val trinoContext: TrinoContext`?
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoConf.scala
##########
@@ -30,4 +32,11 @@ object TrinoConf {
.version("1.5.0")
.intConf
.createWithDefault(3)
+
+ val CLIENT_REQUEST_TIMEOUT: ConfigEntry[Long] =
+ buildConf("trino.client.request.timeout")
+ .doc("Client request will be closed when it's not accessed for this duration of time")
Review comment:
the `client` here is very ambiguous
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
##########
@@ -17,19 +17,128 @@
package org.apache.kyuubi.engine.trino
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+
+import org.apache.kyuubi.KyuubiSQLException
import org.apache.kyuubi.Logging
+import org.apache.kyuubi.Utils.TRINO_ENGINE_SHUTDOWN_PRIORITY
+import org.apache.kyuubi.Utils.addShutdownHook
+import org.apache.kyuubi.Utils.currentUser
import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.countDownLatch
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.currentEngine
+import org.apache.kyuubi.ha.HighAvailabilityConf.HA_ZK_CONN_RETRY_POLICY
+import org.apache.kyuubi.ha.client.RetryPolicies
+import org.apache.kyuubi.service.Serverable
import org.apache.kyuubi.util.SignalRegister
+case class TrinoSqlEngine(trino: TrinoContext)
+ extends Serverable("TrinoSQLEngine") {
+
+ override val backendService = new TrinoBackendService(trino)
+
+ override val frontendServices = Seq(new TrinoTBinaryFrontendService(this))
+
+ override def start(): Unit = {
+ super.start()
+ // Start engine self-terminating checker after all services are ready and it can be reached by
+ // all servers in engine spaces.
+ backendService.sessionManager.startTerminatingChecker(() => {
+ assert(currentEngine.isDefined)
+ currentEngine.get.stop()
+ })
+ }
+
+ override protected def stopServer(): Unit = {
+ countDownLatch.countDown()
+ }
+}
+
object TrinoSqlEngine extends Logging {
+ private val countDownLatch = new CountDownLatch(1)
val kyuubiConf: KyuubiConf = KyuubiConf()
+ var currentEngine: Option[TrinoSqlEngine] = None
+
+ def createTrinoContext(): TrinoContext = {
Review comment:
why we need a trinoContext here?
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
##########
@@ -17,19 +17,128 @@
package org.apache.kyuubi.engine.trino
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+
+import org.apache.kyuubi.KyuubiSQLException
import org.apache.kyuubi.Logging
+import org.apache.kyuubi.Utils.TRINO_ENGINE_SHUTDOWN_PRIORITY
+import org.apache.kyuubi.Utils.addShutdownHook
+import org.apache.kyuubi.Utils.currentUser
import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.countDownLatch
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.currentEngine
+import org.apache.kyuubi.ha.HighAvailabilityConf.HA_ZK_CONN_RETRY_POLICY
+import org.apache.kyuubi.ha.client.RetryPolicies
+import org.apache.kyuubi.service.Serverable
import org.apache.kyuubi.util.SignalRegister
+case class TrinoSqlEngine(trino: TrinoContext)
+ extends Serverable("TrinoSQLEngine") {
+
+ override val backendService = new TrinoBackendService(trino)
+
+ override val frontendServices = Seq(new TrinoTBinaryFrontendService(this))
+
+ override def start(): Unit = {
+ super.start()
+ // Start engine self-terminating checker after all services are ready and it can be reached by
+ // all servers in engine spaces.
+ backendService.sessionManager.startTerminatingChecker(() => {
+ assert(currentEngine.isDefined)
+ currentEngine.get.stop()
+ })
+ }
+
+ override protected def stopServer(): Unit = {
+ countDownLatch.countDown()
+ }
+}
+
object TrinoSqlEngine extends Logging {
+ private val countDownLatch = new CountDownLatch(1)
val kyuubiConf: KyuubiConf = KyuubiConf()
+ var currentEngine: Option[TrinoSqlEngine] = None
+
+ def createTrinoContext(): TrinoContext = {
Review comment:
why we need a trinoContext here? Looks only SessionManger needs it?
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/TrinoOperation.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.engine.trino.operation
+
+import java.io.IOException
+
+import io.trino.client.Column
+import io.trino.client.StatementClient
+import org.apache.hive.service.rpc.thrift.TRowSet
+import org.apache.hive.service.rpc.thrift.TTableSchema
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Utils
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.engine.trino.schema.RowSet
+import org.apache.kyuubi.engine.trino.schema.SchemaHelper
+import org.apache.kyuubi.engine.trino.session.TrinoSessionImpl
+import org.apache.kyuubi.operation.AbstractOperation
+import org.apache.kyuubi.operation.FetchIterator
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_FIRST
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_NEXT
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_PRIOR
+import org.apache.kyuubi.operation.FetchOrientation.FetchOrientation
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.operation.OperationState.OperationState
+import org.apache.kyuubi.operation.OperationType.OperationType
+import org.apache.kyuubi.operation.log.OperationLog
+import org.apache.kyuubi.session.Session
+
+abstract class TrinoOperation(opType: OperationType, session: Session)
+ extends AbstractOperation(opType, session) {
+
+ protected val trinoContext: TrinoContext = session.asInstanceOf[TrinoSessionImpl].trinoContext
+
+ protected var trino: StatementClient = _
+
+ protected var schema: List[Column] = _
+
+ protected var iter: FetchIterator[List[Any]] = _
+
+ override def getResultSetSchema: TTableSchema = SchemaHelper.toTTableSchema(schema)
+
+ override def getNextRowSet(order: FetchOrientation, rowSetSize: Int): TRowSet = {
+ validateDefaultFetchOrientation(order)
+ assertState(OperationState.FINISHED)
+ setHasResultSet(true)
+ order match {
+ case FETCH_NEXT => iter.fetchNext()
+ case FETCH_PRIOR => iter.fetchPrior(rowSetSize);
+ case FETCH_FIRST => iter.fetchAbsolute(0);
+ }
+ val taken = iter.take(rowSetSize)
+ val resultRowSet = RowSet.toTRowSet(taken.toList, schema, getProtocolVersion)
+ resultRowSet.setStartRowOffset(iter.getPosition)
+ resultRowSet
+ }
+
+ override protected def beforeRun(): Unit = {
+ setHasResultSet(true)
+ setState(OperationState.RUNNING)
+ }
+
+ override protected def afterRun(): Unit = {
+ state.synchronized {
+ if (!isTerminalState(state)) {
+ setState(OperationState.FINISHED)
+ }
+ }
+ OperationLog.removeCurrentOperationLog()
+ }
+
+ override def cancel(): Unit = {
+ cleanup(OperationState.CANCELED)
+ }
+
+ protected def cleanup(targetState: OperationState): Unit = state.synchronized {
+ if (!isTerminalState(state)) {
+ setState(targetState)
+ Option(getBackgroundHandle).foreach(_.cancel(true))
+ }
+ }
+
+ override def close(): Unit = {
+ cleanup(OperationState.CLOSED)
+ try {
+ trino.close()
+ getOperationLog.foreach(_.close())
+ } catch {
+ case e: IOException =>
+ error(e.getMessage, e)
+ }
+ }
+
+ override def shouldRunAsync: Boolean = false
+
+ protected def onError(cancel: Boolean = false): PartialFunction[Throwable, Unit] = {
+ // We should use Throwable instead of Exception since `java.lang.NoClassDefFoundError`
+ // could be thrown.
+ case e: Throwable =>
+ if (cancel && trino.isRunning) trino.cancelLeafStage()
+ state.synchronized {
+ val errMsg = Utils.stringifyException(e)
+ if (state == OperationState.TIMEOUT) {
+ val ke = KyuubiSQLException(s"Timeout operating $opType: $errMsg")
+ setOperationException(ke)
+ throw ke
+ } else if (isTerminalState(state)) {
+ setOperationException(KyuubiSQLException(errMsg))
+ warn(s"Ignore exception in terminal state with $statementId: $errMsg")
+ } else {
+ setState(OperationState.ERROR)
Review comment:
move to L130？
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionManager.scala
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.engine.trino.session
+
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+import org.apache.hive.service.rpc.thrift.TProtocolVersion
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Utils
+import org.apache.kyuubi.Utils.currentUser
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.ENGINE_OPERATION_LOG_DIR_ROOT
+import org.apache.kyuubi.config.KyuubiConf.ENGINE_SHARE_LEVEL
+import org.apache.kyuubi.engine.ShareLevel
+import org.apache.kyuubi.engine.trino.TrinoConf
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine
+import org.apache.kyuubi.engine.trino.operation.TrinoOperationManager
+import org.apache.kyuubi.session.SessionHandle
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoSessionManager
+ extends SessionManager("TrinoSessionManager") {
+
+ val operationManager = new TrinoOperationManager()
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ val absPath = Utils.getAbsolutePathFromWork(conf.get(ENGINE_OPERATION_LOG_DIR_ROOT))
+ _operationLogRoot = Some(absPath.toAbsolutePath.toString)
+ super.initialize(conf)
+ }
+
+ override def openSession(
+ protocol: TProtocolVersion,
+ user: String,
+ password: String,
+ ipAddress: String,
+ conf: Map[String, String]): SessionHandle = {
+ info(s"Opening session for $user@$ipAddress")
+ val trinoContext = createTrinoContext()
+ val sessionImpl =
+ new TrinoSessionImpl(protocol, user, password, ipAddress, conf, this, trinoContext)
+
+ try {
+ val handle = sessionImpl.handle
+ sessionImpl.open()
+ setSession(handle, sessionImpl)
+ info(s"$user's trino session with $handle is opened, current opening sessions" +
+ s" $getOpenSessionCount")
+ handle
+ } catch {
+ case e: Exception =>
+ sessionImpl.close()
+ throw KyuubiSQLException(e)
+ }
+ }
+
+ def createTrinoContext(): TrinoContext = {
+ val connectionUrl = conf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_URL).getOrElse(
+ throw KyuubiSQLException("Trino server url can not be null!"))
+ val catalog = conf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_CATALOG).getOrElse(
+ throw KyuubiSQLException("Trino default catalog can not be null!"))
+ val user = conf.getOption("kyuubi.trino.user").getOrElse(currentUser)
+ val clientRequestTimeout = conf.get(TrinoConf.CLIENT_REQUEST_TIMEOUT)
+
+ val session = new ClientSession(
+ URI.create(connectionUrl),
+ user,
+ Optional.empty(),
+ "kyuubi",
+ Optional.empty(),
+ Set[String]().asJava,
+ null,
+ catalog,
+ null,
+ null,
+ ZoneId.systemDefault(),
+ Locale.getDefault,
+ Map[String, String]().asJava,
+ Map[String, String]().asJava,
+ Map[String, String]().asJava,
+ Map[String, ClientSelectedRole]().asJava,
+ Map[String, String]().asJava,
Review comment:
Collections.emptyMap()?
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.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.engine.trino.session
+
+import org.apache.hive.service.rpc.thrift.TProtocolVersion
+
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.session.AbstractSession
+import org.apache.kyuubi.session.SessionHandle
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoSessionImpl(
+ protocol: TProtocolVersion,
+ user: String,
+ password: String,
+ ipAddress: String,
+ conf: Map[String, String],
+ sessionManager: SessionManager,
+ val trinoContext: TrinoContext)
+ extends AbstractSession(protocol, user, password, ipAddress, conf, sessionManager) {
+ override val handle: SessionHandle = SessionHandle(protocol)
+
+ override def open(): Unit = {
+ normalizedConf.foreach {
+ case ("use:database", database) => trinoContext.setCurrentSchema(database)
Review comment:
why not delay ClientSession here? and an `AtomicReference` in TrinoContext also seems unnecessary to me
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (69d7d9b) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.21%`.
> The diff coverage is `66.14%`.
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.76% +0.21%
- Complexity 247 320 +73
============================================
Files 274 281 +7
Lines 13525 13969 +444
Branches 1738 1780 +42
============================================
+ Hits 8054 8349 +295
- Misses 4777 4908 +131
- Partials 694 712 +18
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `36.36% <36.36%> (ø)` | |
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `51.72% <50.00%> (+51.72%)` | :arrow_up: |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `87.50% <87.50%> (ø)` | |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `90.19% <90.19%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `100.00% <100.00%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `64.48% <100.00%> (-2.19%)` | :arrow_down: |
| [...ache/kyuubi/ha/client/KyuubiServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9LeXV1YmlTZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [38 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...69d7d9b](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791287118
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoConf.scala
##########
@@ -30,4 +30,11 @@ object TrinoConf {
.version("1.5.0")
.intConf
.createWithDefault(3)
+
+ val CLIENT_REQUEST_TIMEOUT: ConfigEntry[Int] =
+ buildConf("trino.client.request.timeout")
+ .doc("Client request will be closed when it's not accessed for this duration of time")
+ .version("1.5.0")
+ .intConf
+ .createWithDefault(2)
Review comment:
> hello, what's the unit about this key?
Thanks, addressed.
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
##########
@@ -17,19 +17,128 @@
package org.apache.kyuubi.engine.trino
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+
+import org.apache.kyuubi.KyuubiSQLException
import org.apache.kyuubi.Logging
+import org.apache.kyuubi.Utils.TRINO_ENGINE_SHUTDOWN_PRIORITY
+import org.apache.kyuubi.Utils.addShutdownHook
+import org.apache.kyuubi.Utils.currentUser
import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.countDownLatch
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.currentEngine
+import org.apache.kyuubi.ha.HighAvailabilityConf.HA_ZK_CONN_RETRY_POLICY
+import org.apache.kyuubi.ha.client.RetryPolicies
+import org.apache.kyuubi.service.Serverable
import org.apache.kyuubi.util.SignalRegister
+case class TrinoSqlEngine(trino: TrinoContext)
+ extends Serverable("TrinoSQLEngine") {
+
+ override val backendService = new TrinoBackendService(trino)
+
+ override val frontendServices = Seq(new TrinoTBinaryFrontendService(this))
+
+ override def start(): Unit = {
+ super.start()
+ // Start engine self-terminating checker after all services are ready and it can be reached by
+ // all servers in engine spaces.
+ backendService.sessionManager.startTerminatingChecker(() => {
+ assert(currentEngine.isDefined)
+ currentEngine.get.stop()
+ })
+ }
+
+ override protected def stopServer(): Unit = {
+ countDownLatch.countDown()
+ }
+}
+
object TrinoSqlEngine extends Logging {
+ private val countDownLatch = new CountDownLatch(1)
val kyuubiConf: KyuubiConf = KyuubiConf()
+ var currentEngine: Option[TrinoSqlEngine] = None
+
+ def createTrinoContext(): TrinoContext = {
Review comment:
Yep, move it to SessionManager.
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoConf.scala
##########
@@ -30,4 +32,11 @@ object TrinoConf {
.version("1.5.0")
.intConf
.createWithDefault(3)
+
+ val CLIENT_REQUEST_TIMEOUT: ConfigEntry[Long] =
+ buildConf("trino.client.request.timeout")
+ .doc("Client request will be closed when it's not accessed for this duration of time")
Review comment:
Not sure only doc or key is included, had optimized doc first.
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
+ def this(trino: TrinoContext) =
+ this(classOf[TrinoBackendService].getSimpleName, trino)
+
+ override val sessionManager: SessionManager = new TrinoSessionManager(trinoContext)
+
+ def trinoSession: TrinoContext = trinoContext
Review comment:
Done.
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoBackendService.scala
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.engine.trino
+
+import org.apache.kyuubi.engine.trino.session.TrinoSessionManager
+import org.apache.kyuubi.service.AbstractBackendService
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoBackendService(name: String, trinoContext: TrinoContext)
+ extends AbstractBackendService(name) {
Review comment:
Tanks, done.
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/TrinoOperation.scala
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.engine.trino.operation
+
+import java.io.IOException
+
+import io.trino.client.Column
+import io.trino.client.StatementClient
+import org.apache.hive.service.rpc.thrift.TRowSet
+import org.apache.hive.service.rpc.thrift.TTableSchema
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Utils
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.engine.trino.schema.RowSet
+import org.apache.kyuubi.engine.trino.schema.SchemaHelper
+import org.apache.kyuubi.engine.trino.session.TrinoSessionImpl
+import org.apache.kyuubi.operation.AbstractOperation
+import org.apache.kyuubi.operation.FetchIterator
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_FIRST
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_NEXT
+import org.apache.kyuubi.operation.FetchOrientation.FETCH_PRIOR
+import org.apache.kyuubi.operation.FetchOrientation.FetchOrientation
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.operation.OperationState.OperationState
+import org.apache.kyuubi.operation.OperationType.OperationType
+import org.apache.kyuubi.operation.log.OperationLog
+import org.apache.kyuubi.session.Session
+
+abstract class TrinoOperation(opType: OperationType, session: Session)
+ extends AbstractOperation(opType, session) {
+
+ protected val trinoContext: TrinoContext = session.asInstanceOf[TrinoSessionImpl].trinoContext
+
+ protected var trino: StatementClient = _
+
+ protected var schema: List[Column] = _
+
+ protected var iter: FetchIterator[List[Any]] = _
+
+ override def getResultSetSchema: TTableSchema = SchemaHelper.toTTableSchema(schema)
+
+ override def getNextRowSet(order: FetchOrientation, rowSetSize: Int): TRowSet = {
+ validateDefaultFetchOrientation(order)
+ assertState(OperationState.FINISHED)
+ setHasResultSet(true)
+ order match {
+ case FETCH_NEXT => iter.fetchNext()
+ case FETCH_PRIOR => iter.fetchPrior(rowSetSize);
+ case FETCH_FIRST => iter.fetchAbsolute(0);
+ }
+ val taken = iter.take(rowSetSize)
+ val resultRowSet = RowSet.toTRowSet(taken.toList, schema, getProtocolVersion)
+ resultRowSet.setStartRowOffset(iter.getPosition)
+ resultRowSet
+ }
+
+ override protected def beforeRun(): Unit = {
+ setHasResultSet(true)
+ setState(OperationState.RUNNING)
+ }
+
+ override protected def afterRun(): Unit = {
+ state.synchronized {
+ if (!isTerminalState(state)) {
+ setState(OperationState.FINISHED)
+ }
+ }
+ OperationLog.removeCurrentOperationLog()
+ }
+
+ override def cancel(): Unit = {
+ cleanup(OperationState.CANCELED)
+ }
+
+ protected def cleanup(targetState: OperationState): Unit = state.synchronized {
+ if (!isTerminalState(state)) {
+ setState(targetState)
+ Option(getBackgroundHandle).foreach(_.cancel(true))
+ }
+ }
+
+ override def close(): Unit = {
+ cleanup(OperationState.CLOSED)
+ try {
+ trino.close()
+ getOperationLog.foreach(_.close())
+ } catch {
+ case e: IOException =>
+ error(e.getMessage, e)
+ }
+ }
+
+ override def shouldRunAsync: Boolean = false
+
+ protected def onError(cancel: Boolean = false): PartialFunction[Throwable, Unit] = {
+ // We should use Throwable instead of Exception since `java.lang.NoClassDefFoundError`
+ // could be thrown.
+ case e: Throwable =>
+ if (cancel && trino.isRunning) trino.cancelLeafStage()
+ state.synchronized {
+ val errMsg = Utils.stringifyException(e)
+ if (state == OperationState.TIMEOUT) {
+ val ke = KyuubiSQLException(s"Timeout operating $opType: $errMsg")
+ setOperationException(ke)
+ throw ke
+ } else if (isTerminalState(state)) {
+ setOperationException(KyuubiSQLException(errMsg))
+ warn(s"Ignore exception in terminal state with $statementId: $errMsg")
+ } else {
+ setState(OperationState.ERROR)
Review comment:
Done
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionManager.scala
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.engine.trino.session
+
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+import org.apache.hive.service.rpc.thrift.TProtocolVersion
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Utils
+import org.apache.kyuubi.Utils.currentUser
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.ENGINE_OPERATION_LOG_DIR_ROOT
+import org.apache.kyuubi.config.KyuubiConf.ENGINE_SHARE_LEVEL
+import org.apache.kyuubi.engine.ShareLevel
+import org.apache.kyuubi.engine.trino.TrinoConf
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine
+import org.apache.kyuubi.engine.trino.operation.TrinoOperationManager
+import org.apache.kyuubi.session.SessionHandle
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoSessionManager
+ extends SessionManager("TrinoSessionManager") {
+
+ val operationManager = new TrinoOperationManager()
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ val absPath = Utils.getAbsolutePathFromWork(conf.get(ENGINE_OPERATION_LOG_DIR_ROOT))
+ _operationLogRoot = Some(absPath.toAbsolutePath.toString)
+ super.initialize(conf)
+ }
+
+ override def openSession(
+ protocol: TProtocolVersion,
+ user: String,
+ password: String,
+ ipAddress: String,
+ conf: Map[String, String]): SessionHandle = {
+ info(s"Opening session for $user@$ipAddress")
+ val trinoContext = createTrinoContext()
+ val sessionImpl =
+ new TrinoSessionImpl(protocol, user, password, ipAddress, conf, this, trinoContext)
+
+ try {
+ val handle = sessionImpl.handle
+ sessionImpl.open()
+ setSession(handle, sessionImpl)
+ info(s"$user's trino session with $handle is opened, current opening sessions" +
+ s" $getOpenSessionCount")
+ handle
+ } catch {
+ case e: Exception =>
+ sessionImpl.close()
+ throw KyuubiSQLException(e)
+ }
+ }
+
+ def createTrinoContext(): TrinoContext = {
+ val connectionUrl = conf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_URL).getOrElse(
+ throw KyuubiSQLException("Trino server url can not be null!"))
+ val catalog = conf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_CATALOG).getOrElse(
+ throw KyuubiSQLException("Trino default catalog can not be null!"))
+ val user = conf.getOption("kyuubi.trino.user").getOrElse(currentUser)
+ val clientRequestTimeout = conf.get(TrinoConf.CLIENT_REQUEST_TIMEOUT)
+
+ val session = new ClientSession(
+ URI.create(connectionUrl),
+ user,
+ Optional.empty(),
+ "kyuubi",
+ Optional.empty(),
+ Set[String]().asJava,
+ null,
+ catalog,
+ null,
+ null,
+ ZoneId.systemDefault(),
+ Locale.getDefault,
+ Map[String, String]().asJava,
+ Map[String, String]().asJava,
+ Map[String, String]().asJava,
+ Map[String, ClientSelectedRole]().asJava,
+ Map[String, String]().asJava,
Review comment:
Done
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.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.engine.trino.session
+
+import org.apache.hive.service.rpc.thrift.TProtocolVersion
+
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.session.AbstractSession
+import org.apache.kyuubi.session.SessionHandle
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoSessionImpl(
+ protocol: TProtocolVersion,
+ user: String,
+ password: String,
+ ipAddress: String,
+ conf: Map[String, String],
+ sessionManager: SessionManager,
+ val trinoContext: TrinoContext)
+ extends AbstractSession(protocol, user, password, ipAddress, conf, sessionManager) {
+ override val handle: SessionHandle = SessionHandle(protocol)
+
+ override def open(): Unit = {
+ normalizedConf.foreach {
+ case ("use:database", database) => trinoContext.setCurrentSchema(database)
Review comment:
Done
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791421658
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionImpl.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.engine.trino.session
+
+import org.apache.hive.service.rpc.thrift.TProtocolVersion
+
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.session.AbstractSession
+import org.apache.kyuubi.session.SessionHandle
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoSessionImpl(
+ protocol: TProtocolVersion,
+ user: String,
+ password: String,
+ ipAddress: String,
+ conf: Map[String, String],
+ sessionManager: SessionManager,
+ val trinoContext: TrinoContext)
+ extends AbstractSession(protocol, user, password, ipAddress, conf, sessionManager) {
+ override val handle: SessionHandle = SessionHandle(protocol)
+
+ override def open(): Unit = {
+ normalizedConf.foreach {
+ case ("use:database", database) => trinoContext.setCurrentSchema(database)
Review comment:
why not delay ClientSession here? and an `AtomicReference` in TrinoContext also seems unnecessary to me
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802294986
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoContext.scala
##########
@@ -17,24 +17,9 @@
package org.apache.kyuubi.engine.trino
-import java.util.concurrent.atomic.AtomicReference
-
import io.trino.client.ClientSession
import okhttp3.OkHttpClient
-class TrinoContext(
- val httpClient: OkHttpClient,
- val clientSession: AtomicReference[ClientSession]) {
-
- def getClientSession: ClientSession = clientSession.get
-
- def setCurrentSchema(schema: String): Unit = {
- clientSession.set(ClientSession.builder(clientSession.get).withSchema(schema).build())
- }
-
-}
-
-object TrinoContext {
- def apply(httpClient: OkHttpClient, clientSession: ClientSession): TrinoContext =
- new TrinoContext(httpClient, new AtomicReference(clientSession))
-}
+case class TrinoContext(
+ httpClient: OkHttpClient,
+ var clientSession: ClientSession)
Review comment:
so why do we reset it for each ExcuteStatement?
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791499941
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/session/TrinoSessionManager.scala
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.engine.trino.session
+
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+import org.apache.hive.service.rpc.thrift.TProtocolVersion
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Utils
+import org.apache.kyuubi.Utils.currentUser
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.ENGINE_OPERATION_LOG_DIR_ROOT
+import org.apache.kyuubi.config.KyuubiConf.ENGINE_SHARE_LEVEL
+import org.apache.kyuubi.engine.ShareLevel
+import org.apache.kyuubi.engine.trino.TrinoConf
+import org.apache.kyuubi.engine.trino.TrinoContext
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine
+import org.apache.kyuubi.engine.trino.operation.TrinoOperationManager
+import org.apache.kyuubi.session.SessionHandle
+import org.apache.kyuubi.session.SessionManager
+
+class TrinoSessionManager
+ extends SessionManager("TrinoSessionManager") {
+
+ val operationManager = new TrinoOperationManager()
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ val absPath = Utils.getAbsolutePathFromWork(conf.get(ENGINE_OPERATION_LOG_DIR_ROOT))
+ _operationLogRoot = Some(absPath.toAbsolutePath.toString)
+ super.initialize(conf)
+ }
+
+ override def openSession(
+ protocol: TProtocolVersion,
+ user: String,
+ password: String,
+ ipAddress: String,
+ conf: Map[String, String]): SessionHandle = {
+ info(s"Opening session for $user@$ipAddress")
+ val trinoContext = createTrinoContext()
+ val sessionImpl =
+ new TrinoSessionImpl(protocol, user, password, ipAddress, conf, this, trinoContext)
+
+ try {
+ val handle = sessionImpl.handle
+ sessionImpl.open()
+ setSession(handle, sessionImpl)
+ info(s"$user's trino session with $handle is opened, current opening sessions" +
+ s" $getOpenSessionCount")
+ handle
+ } catch {
+ case e: Exception =>
+ sessionImpl.close()
+ throw KyuubiSQLException(e)
+ }
+ }
+
+ def createTrinoContext(): TrinoContext = {
+ val connectionUrl = conf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_URL).getOrElse(
+ throw KyuubiSQLException("Trino server url can not be null!"))
+ val catalog = conf.get(KyuubiConf.ENGINE_TRINO_CONNECTION_CATALOG).getOrElse(
+ throw KyuubiSQLException("Trino default catalog can not be null!"))
+ val user = conf.getOption("kyuubi.trino.user").getOrElse(currentUser)
+ val clientRequestTimeout = conf.get(TrinoConf.CLIENT_REQUEST_TIMEOUT)
+
+ val session = new ClientSession(
+ URI.create(connectionUrl),
+ user,
+ Optional.empty(),
+ "kyuubi",
+ Optional.empty(),
+ Set[String]().asJava,
+ null,
+ catalog,
+ null,
+ null,
+ ZoneId.systemDefault(),
+ Locale.getDefault,
+ Map[String, String]().asJava,
+ Map[String, String]().asJava,
+ Map[String, String]().asJava,
+ Map[String, ClientSelectedRole]().asJava,
+ Map[String, String]().asJava,
Review comment:
Done
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1019640105
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1830](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a20f2d0) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/61e6022162e4ffe5622f30f8d23d8bd17ae3947d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61e6022) will **increase** coverage by `0.44%`.
> The diff coverage is `68.52%`.
> :exclamation: Current head a20f2d0 differs from pull request most recent head 69d7d9b. Consider uploading reports for the commit 69d7d9b to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1830 +/- ##
============================================
+ Coverage 59.54% 59.99% +0.44%
- Complexity 247 314 +67
============================================
Files 274 280 +6
Lines 13525 13856 +331
Branches 1738 1759 +21
============================================
+ Hits 8054 8313 +259
- Misses 4777 4833 +56
- Partials 694 710 +16
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...kyuubi/engine/trino/operation/TrinoOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbi5zY2FsYQ==) | `39.21% <39.21%> (ø)` | |
| [...engine/trino/operation/TrinoOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9Ucmlub09wZXJhdGlvbk1hbmFnZXIuc2NhbGE=) | `41.66% <41.66%> (ø)` | |
| [...ubi/engine/trino/session/TrinoSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uTWFuYWdlci5zY2FsYQ==) | `78.26% <78.26%> (ø)` | |
| [...uubi/engine/trino/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL29wZXJhdGlvbi9FeGVjdXRlU3RhdGVtZW50LnNjYWxh) | `78.37% <78.37%> (ø)` | |
| [...ache/kyuubi/engine/trino/TrinoBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `80.00% <80.00%> (ø)` | |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `82.75% <83.63%> (+82.75%)` | :arrow_up: |
| [...kyuubi/engine/trino/session/TrinoSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL3Nlc3Npb24vVHJpbm9TZXNzaW9uSW1wbC5zY2FsYQ==) | `87.50% <87.50%> (ø)` | |
| [...ala/org/apache/kyuubi/engine/trino/TrinoConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vQ29uZi5zY2FsYQ==) | `100.00% <100.00%> (ø)` | |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `66.99% <100.00%> (+0.32%)` | :arrow_up: |
| [...ache/kyuubi/ha/client/KyuubiServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9LeXV1YmlTZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [32 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [61e6022...69d7d9b](https://codecov.io/gh/apache/incubator-kyuubi/pull/1830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r791388070
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
##########
@@ -17,19 +17,128 @@
package org.apache.kyuubi.engine.trino
+import java.net.URI
+import java.time.ZoneId
+import java.util.Locale
+import java.util.Optional
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import io.airlift.units.Duration
+import io.trino.client.ClientSelectedRole
+import io.trino.client.ClientSession
+import okhttp3.OkHttpClient
+
+import org.apache.kyuubi.KyuubiSQLException
import org.apache.kyuubi.Logging
+import org.apache.kyuubi.Utils.TRINO_ENGINE_SHUTDOWN_PRIORITY
+import org.apache.kyuubi.Utils.addShutdownHook
+import org.apache.kyuubi.Utils.currentUser
import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.countDownLatch
+import org.apache.kyuubi.engine.trino.TrinoSqlEngine.currentEngine
+import org.apache.kyuubi.ha.HighAvailabilityConf.HA_ZK_CONN_RETRY_POLICY
+import org.apache.kyuubi.ha.client.RetryPolicies
+import org.apache.kyuubi.service.Serverable
import org.apache.kyuubi.util.SignalRegister
+case class TrinoSqlEngine(trino: TrinoContext)
+ extends Serverable("TrinoSQLEngine") {
+
+ override val backendService = new TrinoBackendService(trino)
+
+ override val frontendServices = Seq(new TrinoTBinaryFrontendService(this))
+
+ override def start(): Unit = {
+ super.start()
+ // Start engine self-terminating checker after all services are ready and it can be reached by
+ // all servers in engine spaces.
+ backendService.sessionManager.startTerminatingChecker(() => {
+ assert(currentEngine.isDefined)
+ currentEngine.get.stop()
+ })
+ }
+
+ override protected def stopServer(): Unit = {
+ countDownLatch.countDown()
+ }
+}
+
object TrinoSqlEngine extends Logging {
+ private val countDownLatch = new CountDownLatch(1)
val kyuubiConf: KyuubiConf = KyuubiConf()
+ var currentEngine: Option[TrinoSqlEngine] = None
+
+ def createTrinoContext(): TrinoContext = {
Review comment:
Yep, move it to SessionManager.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
pan3793 commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r801459745
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/operation/ExecuteStatement.scala
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.engine.trino.operation
+
+import java.util.concurrent.RejectedExecutionException
+
+import org.apache.kyuubi.KyuubiSQLException
+import org.apache.kyuubi.Logging
+import org.apache.kyuubi.engine.trino.TrinoStatement
+import org.apache.kyuubi.operation.ArrayFetchIterator
+import org.apache.kyuubi.operation.IterableFetchIterator
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.operation.OperationType
+import org.apache.kyuubi.operation.log.OperationLog
+import org.apache.kyuubi.session.Session
+
+class ExecuteStatement(
+ session: Session,
+ override val statement: String,
+ override val shouldRunAsync: Boolean,
+ incrementalCollect: Boolean)
+ extends TrinoOperation(OperationType.EXECUTE_STATEMENT, session) with Logging {
+
+ private val operationLog: OperationLog = OperationLog.createOperationLog(session, getHandle)
+ override def getOperationLog: Option[OperationLog] = Option(operationLog)
+
+ override protected def beforeRun(): Unit = {
+ OperationLog.setCurrentOperationLog(operationLog)
+ setState(OperationState.PENDING)
+ setHasResultSet(true)
+ }
+
+ override protected def afterRun(): Unit = {
+ OperationLog.removeCurrentOperationLog()
+ }
+
+ override protected def runInternal(): Unit = {
+ val trinoStatement = TrinoStatement(trinoContext, session.sessionManager.getConf, statement)
+ trino = trinoStatement.getTrinoClient
+ if (shouldRunAsync) {
+ val asyncOperation = new Runnable {
+ override def run(): Unit = {
+ OperationLog.setCurrentOperationLog(operationLog)
+ executeStatement(trinoStatement)
+ }
+ }
+
+ try {
+ val trinoSessionManager = session.sessionManager
+ val backgroundHandle = trinoSessionManager.submitBackgroundOperation(asyncOperation)
+ setBackgroundHandle(backgroundHandle)
+ } catch {
+ case rejected: RejectedExecutionException =>
+ setState(OperationState.ERROR)
+ val ke =
+ KyuubiSQLException("Error submitting query in background, query rejected", rejected)
+ setOperationException(ke)
+ throw ke
+ }
+ } else {
+ executeStatement(trinoStatement)
+ }
+ }
+
+ private def executeStatement(trinoStatement: TrinoStatement): Unit = {
+ setState(OperationState.RUNNING)
+ try {
+ schema = trinoStatement.getColumns
+ val resultSet = trinoStatement.execute()
+ iter =
+ if (incrementalCollect) {
+ info("Execute in incremental collect mode")
+ new IterableFetchIterator(resultSet)
Review comment:
Looks like Trino works better in incremental collect mode? In Spark incremental collect will significantly downgrade the perf.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#issuecomment-1033481045
LGTM, thanks for your great job!
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802294262
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoContext.scala
##########
@@ -17,24 +17,9 @@
package org.apache.kyuubi.engine.trino
-import java.util.concurrent.atomic.AtomicReference
-
import io.trino.client.ClientSession
import okhttp3.OkHttpClient
-class TrinoContext(
- val httpClient: OkHttpClient,
- val clientSession: AtomicReference[ClientSession]) {
-
- def getClientSession: ClientSession = clientSession.get
-
- def setCurrentSchema(schema: String): Unit = {
- clientSession.set(ClientSession.builder(clientSession.get).withSchema(schema).build())
- }
-
-}
-
-object TrinoContext {
- def apply(httpClient: OkHttpClient, clientSession: ClientSession): TrinoContext =
- new TrinoContext(httpClient, new AtomicReference(clientSession))
-}
+case class TrinoContext(
+ httpClient: OkHttpClient,
+ var clientSession: ClientSession)
Review comment:
I think it's a session level variable. `ClientSession` stores session configs, such as schema and configs which set by `set` command. If we move it to `TrinoOperation `, `use` and `set` will not work.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] hddong commented on a change in pull request #1830: [KYUUBI #1821] Add trino ExecuteStatement
Posted by GitBox <gi...@apache.org>.
hddong commented on a change in pull request #1830:
URL: https://github.com/apache/incubator-kyuubi/pull/1830#discussion_r802407408
##########
File path: externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoStatement.scala
##########
@@ -99,50 +99,45 @@ class TrinoStatement(trinoContext: TrinoContext, kyuubiConf: KyuubiConf, sql: St
val rowBuffer = new ArrayList[List[Any]](MAX_BUFFERED_ROWS)
var bufferStart = System.nanoTime()
val result = ArrayBuffer[List[Any]]()
- try {
- breakable {
- while (!dataProcessing.isCompleted) {
- val atEnd = drainDetectingEnd(rowQueue, rowBuffer, MAX_BUFFERED_ROWS, END_TOKEN)
- if (!atEnd) {
- // Flush if needed
- if (rowBuffer.size() >= MAX_BUFFERED_ROWS ||
- Duration.fromNanos(bufferStart).compareTo(MAX_BUFFER_TIME) >= 0) {
- result ++= rowBuffer.asScala
- rowBuffer.clear()
- bufferStart = System.nanoTime()
- }
-
- val row = rowQueue.poll(MAX_BUFFER_TIME.toMillis, duration.MILLISECONDS)
- row match {
- case END_TOKEN => break
- case null =>
- case _ => rowBuffer.add(row)
- }
+
+ breakable {
Review comment:
Sure, use a boolean flag instead.
--
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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org