You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/03/12 13:09:23 UTC
[GitHub] [incubator-kyuubi] cxzl25 opened a new pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
cxzl25 opened a new pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113
### _Why are the changes needed?_
https://github.com/apache/incubator-kyuubi/issues/2112
Clients less than version 2.1 have no [HIVE-4924](https://issues.apache.org/jira/browse/HIVE-4924) Patch, no queryTimeout parameter and no TIMEOUT status. When the server enables kyuubi.operation.query.timeout, this will cause the client of the lower version to get stuck.
Introduce a parameter, when it is turned on, no matter what the version of the client is, the queryTimeout parameter is not set, and the TIMEOUT state is converted to CANCELED. If queryTimeout is set, the TIMEOUT state is still maintained.
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065927953
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 [#2113](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (46307b1) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8f7b2c66401034b3238dc40238f42b0afe2de31d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8f7b2c6) will **increase** coverage by `0.45%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 #2113 +/- ##
============================================
+ Coverage 60.73% 61.19% +0.45%
Complexity 108 108
============================================
Files 320 318 -2
Lines 15481 15474 -7
Branches 1991 1991
============================================
+ Hits 9403 9469 +66
+ Misses 5273 5197 -76
- Partials 805 808 +3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.58% <100.00%> (+0.14%)` | :arrow_up: |
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `88.17% <100.00%> (+7.27%)` | :arrow_up: |
| [...ubi/engine/flink/operation/PlanOnlyStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vUGxhbk9ubHlTdGF0ZW1lbnQuc2NhbGE=) | `64.70% <0.00%> (-13.87%)` | :arrow_down: |
| [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `76.38% <0.00%> (-5.71%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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) | `67.46% <0.00%> (-2.41%)` | :arrow_down: |
| [...ache/kyuubi/engine/trino/TrinoProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvdHJpbm8vVHJpbm9Qcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `42.59% <0.00%> (-2.24%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `82.75% <0.00%> (-0.88%)` | :arrow_down: |
| [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `85.12% <0.00%> (-0.83%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `84.15% <0.00%> (-0.31%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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/2113?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/2113?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 [8f7b2c6...46307b1](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cxzl25 commented on a change in pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#discussion_r825923560
##########
File path: kyuubi-server/src/main/scala/org/apache/kyuubi/operation/ExecuteStatement.scala
##########
@@ -124,6 +124,11 @@ class ExecuteStatement(
case CANCELED_STATE =>
setState(OperationState.CANCELED)
+ case TIMEDOUT_STATE
+ if getSession.protocol.getValue <=
+ TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V8.getValue =>
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1067467594
thanks, 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065927953
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 [#2113](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce9d75f) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8f7b2c66401034b3238dc40238f42b0afe2de31d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8f7b2c6) will **increase** coverage by `0.49%`.
> The diff coverage is `96.29%`.
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 #2113 +/- ##
============================================
+ Coverage 60.73% 61.23% +0.49%
Complexity 108 108
============================================
Files 320 317 -3
Lines 15481 15461 -20
Branches 1991 1990 -1
============================================
+ Hits 9403 9468 +65
+ Misses 5273 5186 -87
- Partials 805 807 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `88.46% <94.11%> (+7.56%)` | :arrow_up: |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.45% <100.00%> (+0.01%)` | :arrow_up: |
| [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `97.05% <100.00%> (+4.55%)` | :arrow_up: |
| [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `85.12% <0.00%> (-0.83%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `91.66% <0.00%> (ø)` | |
| [...he/hadoop/hive/thrift/TUGIContainingTransport.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hpdmUvdGhyaWZ0L1RVR0lDb250YWluaW5nVHJhbnNwb3J0LmphdmE=) | | |
| [...doop/hive/thrift/client/TUGIAssumingTransport.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hpdmUvdGhyaWZ0L2NsaWVudC9UVUdJQXNzdW1pbmdUcmFuc3BvcnQuamF2YQ==) | | |
| [...rg/apache/hadoop/hive/thrift/TFilterTransport.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hpdmUvdGhyaWZ0L1RGaWx0ZXJUcmFuc3BvcnQuamF2YQ==) | | |
| [...a/org/apache/kyuubi/service/TFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `92.77% <0.00%> (+0.66%)` | :arrow_up: |
| ... and [5 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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/2113?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/2113?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 [8f7b2c6...ce9d75f](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a change in pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on a change in pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#discussion_r825850175
##########
File path: kyuubi-server/src/main/scala/org/apache/kyuubi/operation/ExecuteStatement.scala
##########
@@ -124,6 +124,11 @@ class ExecuteStatement(
case CANCELED_STATE =>
setState(OperationState.CANCELED)
+ case TIMEDOUT_STATE
+ if getSession.protocol.getValue <=
+ TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V8.getValue =>
Review comment:
can we also add some comments to describe the reason why we do 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cxzl25 commented on a change in pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#discussion_r825597677
##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationManagerTimeoutCompatibleSuite.scala
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.operation
+
+import java.sql.{SQLException, SQLTimeoutException}
+
+import org.apache.kyuubi.WithKyuubiServer
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{OPERATION_QUERY_TIMEOUT, OPERATION_QUERY_TIMEOUT_COMPATIBLE_STATE}
+
+class KyuubiOperationManagerTimeoutCompatibleSuite extends WithKyuubiServer
+ with HiveJDBCTestHelper {
+ override protected val conf: KyuubiConf = {
+ KyuubiConf().set(OPERATION_QUERY_TIMEOUT.key, "PT1S").set(
+ OPERATION_QUERY_TIMEOUT_COMPATIBLE_STATE.key,
+ "true")
+ }
+
+ test("clientTimeout is not set, timeout state switches to cancel state") {
Review comment:
After using `withSessionConf` to set parameters, the session of `ExecuteStatement` needs to be converted into `KyuubiSessionImpl` to get `sessionConf`.
So i did not modify.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065927953
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 [#2113](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (46307b1) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8f7b2c66401034b3238dc40238f42b0afe2de31d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8f7b2c6) will **increase** coverage by `0.45%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 46307b1 differs from pull request most recent head d07c945. Consider uploading reports for the commit d07c945 to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 #2113 +/- ##
============================================
+ Coverage 60.73% 61.19% +0.45%
Complexity 108 108
============================================
Files 320 318 -2
Lines 15481 15474 -7
Branches 1991 1991
============================================
+ Hits 9403 9469 +66
+ Misses 5273 5197 -76
- Partials 805 808 +3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.58% <100.00%> (+0.14%)` | :arrow_up: |
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `88.17% <100.00%> (+7.27%)` | :arrow_up: |
| [...ubi/engine/flink/operation/PlanOnlyStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vUGxhbk9ubHlTdGF0ZW1lbnQuc2NhbGE=) | `64.70% <0.00%> (-13.87%)` | :arrow_down: |
| [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `76.38% <0.00%> (-5.71%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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) | `67.46% <0.00%> (-2.41%)` | :arrow_down: |
| [...ache/kyuubi/engine/trino/TrinoProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvdHJpbm8vVHJpbm9Qcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `42.59% <0.00%> (-2.24%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `82.75% <0.00%> (-0.88%)` | :arrow_down: |
| [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `85.12% <0.00%> (-0.83%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `84.15% <0.00%> (-0.31%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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/2113?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/2113?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 [8f7b2c6...d07c945](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cxzl25 commented on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1066627951
> is it possible change operation state dynamically through thrift protocol version ?
>
> It seems we can check if the thrift protocol version <= [HIVE_CLI_SERVICE_PROTOCOL_V8](https://github.com/apache/hive/blob/branch-2.0/service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TProtocolVersion.java) and disable timeout, since Hive-2.1 introduce [HIVE_CLI_SERVICE_PROTOCOL_V9](https://github.com/apache/hive/commit/cfb11f8039c8d9176a77c2c1d7f3624f8d378474)
Now I change to directly use the thrift protocol version to decide whether to convert `TIMEDOUT_STATE`.
<= HIVE_CLI_SERVICE_PROTOCOL_V8 and disable timeout
`HIVE_CLI_SERVICE_PROTOCOL_V8` in Hive 2.1.0
`HIVE_CLI_SERVICE_PROTOCOL_V9` in Hive 2.1.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065927953
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 [#2113](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (46307b1) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8f7b2c66401034b3238dc40238f42b0afe2de31d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8f7b2c6) will **increase** coverage by `0.45%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 46307b1 differs from pull request most recent head c50ab2c. Consider uploading reports for the commit c50ab2c to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 #2113 +/- ##
============================================
+ Coverage 60.73% 61.19% +0.45%
Complexity 108 108
============================================
Files 320 318 -2
Lines 15481 15474 -7
Branches 1991 1991
============================================
+ Hits 9403 9469 +66
+ Misses 5273 5197 -76
- Partials 805 808 +3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.58% <100.00%> (+0.14%)` | :arrow_up: |
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `88.17% <100.00%> (+7.27%)` | :arrow_up: |
| [...ubi/engine/flink/operation/PlanOnlyStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vUGxhbk9ubHlTdGF0ZW1lbnQuc2NhbGE=) | `64.70% <0.00%> (-13.87%)` | :arrow_down: |
| [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `76.38% <0.00%> (-5.71%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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) | `67.46% <0.00%> (-2.41%)` | :arrow_down: |
| [...ache/kyuubi/engine/trino/TrinoProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvdHJpbm8vVHJpbm9Qcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `42.59% <0.00%> (-2.24%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `82.75% <0.00%> (-0.88%)` | :arrow_down: |
| [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `85.12% <0.00%> (-0.83%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `84.15% <0.00%> (-0.31%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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/2113?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/2113?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 [8f7b2c6...c50ab2c](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cxzl25 commented on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065886296
When the client has no `TIMEOUT` enumeration value, isSetOperationState is always false, and the loop will not exit.
org.apache.hive.jdbc.HiveStatement#execute
```java
while (!operationComplete) {
if (statusResp.isSetOperationState()) {
switch (statusResp.getOperationState()) {
case XXX:
```
## Fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1066299642
is it possible change operation state dynamically through thrift protocol version ?
It seems we can check if the thrift protocol version <= [HIVE_CLI_SERVICE_PROTOCOL_V8](https://github.com/apache/hive/blob/branch-2.0/service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TProtocolVersion.java) and disable timeout, since Hive-2.1 introduce [HIVE_CLI_SERVICE_PROTOCOL_V9](https://github.com/apache/hive/commit/cfb11f8039c8d9176a77c2c1d7f3624f8d378474)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065927953
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 [#2113](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (46307b1) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8f7b2c66401034b3238dc40238f42b0afe2de31d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8f7b2c6) will **increase** coverage by `0.45%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 46307b1 differs from pull request most recent head dbaeb1f. Consider uploading reports for the commit dbaeb1f to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 #2113 +/- ##
============================================
+ Coverage 60.73% 61.19% +0.45%
Complexity 108 108
============================================
Files 320 318 -2
Lines 15481 15474 -7
Branches 1991 1991
============================================
+ Hits 9403 9469 +66
+ Misses 5273 5197 -76
- Partials 805 808 +3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.58% <100.00%> (+0.14%)` | :arrow_up: |
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `88.17% <100.00%> (+7.27%)` | :arrow_up: |
| [...ubi/engine/flink/operation/PlanOnlyStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vUGxhbk9ubHlTdGF0ZW1lbnQuc2NhbGE=) | `64.70% <0.00%> (-13.87%)` | :arrow_down: |
| [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `76.38% <0.00%> (-5.71%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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) | `67.46% <0.00%> (-2.41%)` | :arrow_down: |
| [...ache/kyuubi/engine/trino/TrinoProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvdHJpbm8vVHJpbm9Qcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `42.59% <0.00%> (-2.24%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `82.75% <0.00%> (-0.88%)` | :arrow_down: |
| [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `85.12% <0.00%> (-0.83%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `84.15% <0.00%> (-0.31%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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/2113?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/2113?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 [8f7b2c6...dbaeb1f](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#discussion_r825559134
##########
File path: kyuubi-server/src/main/scala/org/apache/kyuubi/operation/ExecuteStatement.scala
##########
@@ -125,7 +144,17 @@ class ExecuteStatement(
setState(OperationState.CANCELED)
case TIMEDOUT_STATE =>
- setState(OperationState.TIMEOUT)
+ val state =
+ if (operationQueryTimeoutCompatibleState) {
+ if (queryTimeout > 0) {
Review comment:
do we need this？TIMEDOUT_STATE only happens when `queryTimeout>0` is satisfied on the engine side.
this may be much simpler
case TIMEDOUT_STATE if operationQueryTimeoutCompatibleState => OperationState.CANCELED
case TIMEDOUT_STATE => OperationState.TIMEOUT
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you closed pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
ulysses-you closed pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1066069949
Is there possible to use protocol version to dynamic change the state?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cxzl25 edited a comment on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
cxzl25 edited a comment on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065886296
When the client has no `TIMEOUT` enumeration value, isSetOperationState is always false, and the loop will not exit.
org.apache.hive.jdbc.HiveStatement#execute
```java
while (!operationComplete) {
if (statusResp.isSetOperationState()) {
switch (statusResp.getOperationState()) {
...
case CANCELED_STATE:
// 01000 -> warning
throw new SQLException("Query was cancelled", "01000");
case TIMEDOUT_STATE:
throw new SQLTimeoutException("Query timed out after " + queryTimeout + " seconds");
```
## Fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065927953
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 [#2113](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce9d75f) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8f7b2c66401034b3238dc40238f42b0afe2de31d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8f7b2c6) will **increase** coverage by `0.49%`.
> The diff coverage is `96.29%`.
> :exclamation: Current head ce9d75f differs from pull request most recent head 46307b1. Consider uploading reports for the commit 46307b1 to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 #2113 +/- ##
============================================
+ Coverage 60.73% 61.23% +0.49%
Complexity 108 108
============================================
Files 320 317 -3
Lines 15481 15461 -20
Branches 1991 1990 -1
============================================
+ Hits 9403 9468 +65
+ Misses 5273 5186 -87
- Partials 805 807 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `88.46% <94.11%> (+7.56%)` | :arrow_up: |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.45% <100.00%> (+0.01%)` | :arrow_up: |
| [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `97.05% <100.00%> (+4.55%)` | :arrow_up: |
| [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `85.12% <0.00%> (-0.83%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `91.66% <0.00%> (ø)` | |
| [...he/hadoop/hive/thrift/TUGIContainingTransport.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hpdmUvdGhyaWZ0L1RVR0lDb250YWluaW5nVHJhbnNwb3J0LmphdmE=) | | |
| [...rg/apache/hadoop/hive/thrift/TFilterTransport.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hpdmUvdGhyaWZ0L1RGaWx0ZXJUcmFuc3BvcnQuamF2YQ==) | | |
| [...doop/hive/thrift/client/TUGIAssumingTransport.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hpdmUvdGhyaWZ0L2NsaWVudC9UVUdJQXNzdW1pbmdUcmFuc3BvcnQuamF2YQ==) | | |
| [...a/org/apache/kyuubi/service/TFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `92.77% <0.00%> (+0.66%)` | :arrow_up: |
| ... and [5 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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/2113?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/2113?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 [8f7b2c6...46307b1](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cxzl25 commented on a change in pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on a change in pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#discussion_r825596925
##########
File path: kyuubi-server/src/main/scala/org/apache/kyuubi/operation/ExecuteStatement.scala
##########
@@ -125,7 +144,17 @@ class ExecuteStatement(
setState(OperationState.CANCELED)
case TIMEDOUT_STATE =>
- setState(OperationState.TIMEOUT)
+ val state =
+ if (operationQueryTimeoutCompatibleState) {
+ if (queryTimeout > 0) {
Review comment:
It was considered before that the server has enabled `queryTimeout`, but although the client supports this feature, `queryTimeout` is not set.
Now change it a little easier, turn it on and convert it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1066534563
> is it possible change operation state dynamically through thrift protocol version ?
>
> It seems we can check if the thrift protocol version <= [HIVE_CLI_SERVICE_PROTOCOL_V8](https://github.com/apache/hive/blob/branch-2.0/service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TProtocolVersion.java) and disable timeout, since Hive-2.1 introduce [HIVE_CLI_SERVICE_PROTOCOL_V9](https://github.com/apache/hive/commit/cfb11f8039c8d9176a77c2c1d7f3624f8d378474)
Oh I missed this comment, seems reasonable to me, We can return corresponding state by hive client protocol
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1065927953
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 [#2113](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (46307b1) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8f7b2c66401034b3238dc40238f42b0afe2de31d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8f7b2c6) will **increase** coverage by `0.45%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 46307b1 differs from pull request most recent head 7622470. Consider uploading reports for the commit 7622470 to get more accurate results
[](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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 #2113 +/- ##
============================================
+ Coverage 60.73% 61.19% +0.45%
Complexity 108 108
============================================
Files 320 318 -2
Lines 15481 15474 -7
Branches 1991 1991
============================================
+ Hits 9403 9469 +66
+ Misses 5273 5197 -76
- Partials 805 808 +3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.58% <100.00%> (+0.14%)` | :arrow_up: |
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `88.17% <100.00%> (+7.27%)` | :arrow_up: |
| [...ubi/engine/flink/operation/PlanOnlyStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vUGxhbk9ubHlTdGF0ZW1lbnQuc2NhbGE=) | `64.70% <0.00%> (-13.87%)` | :arrow_down: |
| [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `76.38% <0.00%> (-5.71%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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) | `67.46% <0.00%> (-2.41%)` | :arrow_down: |
| [...ache/kyuubi/engine/trino/TrinoProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvdHJpbm8vVHJpbm9Qcm9jZXNzQnVpbGRlci5zY2FsYQ==) | `42.59% <0.00%> (-2.24%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `82.75% <0.00%> (-0.88%)` | :arrow_down: |
| [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `85.12% <0.00%> (-0.83%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `84.15% <0.00%> (-0.31%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113/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/2113?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/2113?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 [8f7b2c6...7622470](https://codecov.io/gh/apache/incubator-kyuubi/pull/2113?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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#discussion_r825560331
##########
File path: kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationManagerTimeoutCompatibleSuite.scala
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.operation
+
+import java.sql.{SQLException, SQLTimeoutException}
+
+import org.apache.kyuubi.WithKyuubiServer
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{OPERATION_QUERY_TIMEOUT, OPERATION_QUERY_TIMEOUT_COMPATIBLE_STATE}
+
+class KyuubiOperationManagerTimeoutCompatibleSuite extends WithKyuubiServer
+ with HiveJDBCTestHelper {
+ override protected val conf: KyuubiConf = {
+ KyuubiConf().set(OPERATION_QUERY_TIMEOUT.key, "PT1S").set(
+ OPERATION_QUERY_TIMEOUT_COMPATIBLE_STATE.key,
+ "true")
+ }
+
+ test("clientTimeout is not set, timeout state switches to cancel state") {
Review comment:
shall we move this test to KyuubiOperationManagerSuite
we can use org.apache.kyuubi.operation.HiveJDBCTestHelper#withSessionConf to set session-level configurations for a single test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cxzl25 commented on pull request #2113: [KYUUBI #2112] Improve the compatibility of queryTimeout in more version clients
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #2113:
URL: https://github.com/apache/incubator-kyuubi/pull/2113#issuecomment-1066110729
> Is there possible to use protocol version to dynamic change the state?
In the statement, we can get the protocol version through `getSession.protocol` .
`HIVE_CLI_SERVICE_PROTOCOL_V8` was introduced by [HIVE-14191](https://issues.apache.org/jira/browse/HIVE-14191) and took effect in Hive 2.1.1 and 2.2.0, but querytimeout was introduced by [HIVE-4924](https://issues.apache.org/jira/browse/HIVE-4924) and was available in Hive 2.1.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org