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/05/19 19:15:15 UTC
[GitHub] [incubator-kyuubi] cxzl25 opened a new pull request, #2710: [KYUUBI #2594][FOLLOWUP] Fix flaky Test - support engine alive probe to fast fail on engine broken
cxzl25 opened a new pull request, #2710:
URL: https://github.com/apache/incubator-kyuubi/pull/2710
### _Why are the changes needed?_
#2594
In this test case, `ENGINE_REQUEST_TIMEOUT` is set to 100 milliseconds, `KyuubiSyncThriftClient#createTProtocol` sometimes exits directly due to `Read timed out`, session closes, and the test fails.
```
2022-05-19T13:59:50.9049182Z - KYUUBI #2102 - support engine alive probe to fast fail on engine broken *** FAILED ***
2022-05-19T13:59:50.9053749Z 2 was not greater than 3000 (KyuubiOperationPerConnectionSuite.scala:253)
```
```
13:59:50.401 KyuubiSessionManager-exec-pool: Thread-3608 ERROR KyuubiSessionImpl: Opening engine [kyuubi_CONNECTION_SPARK_SQL_runner_8df442d1-a6b0-4694-ac3a-bb6c758fe02b fv-az83-137:37443] for runner session failed
org.apache.thrift.transport.TTransportException: java.net.SocketTimeoutException: Read timed out
at org.apache.thrift.transport.TIOStreamTransport.read(TIOStreamTransport.java:129) ~[libthrift-0.9.3.jar:0.9.3]
at org.apache.thrift.transport.TTransport.readAll(TTransport.java:86) ~[libthrift-0.9.3.jar:0.9.3]
at org.apache.thrift.transport.TSaslTransport.receiveSaslMessage(TSaslTransport.java:178) ~[libthrift-0.9.3.jar:0.9.3]
at org.apache.thrift.transport.TSaslTransport.open(TSaslTransport.java:307) ~[libthrift-0.9.3.jar:0.9.3]
at org.apache.thrift.transport.TSaslClientTransport.open(TSaslClientTransport.java:37) ~[libthrift-0.9.3.jar:0.9.3]
at org.apache.kyuubi.client.KyuubiSyncThriftClient$.createTProtocol(KyuubiSyncThriftClient.scala:399) ~[classes/:?]
at org.apache.kyuubi.client.KyuubiSyncThriftClient$.createClient(KyuubiSyncThriftClient.scala:417) ~[classes/:?]
at org.apache.kyuubi.session.KyuubiSessionImpl.$anonfun$openEngineSession$1(KyuubiSessionImpl.scala:106) ~[classes/:?]
```
### _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] turboFei commented on a diff in pull request #2710: [KYUUBI #2594][FOLLOWUP] Fix flaky Test - support engine alive probe to fast fail on engine broken
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2710:
URL: https://github.com/apache/incubator-kyuubi/pull/2710#discussion_r877709526
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerConnectionSuite.scala:
##########
@@ -250,7 +250,7 @@ class KyuubiOperationPerConnectionSuite extends WithKyuubiServer with HiveJDBCTe
val executeStmtResp = client.ExecuteStatement(executeStmtReq)
assert(executeStmtResp.getStatus.getStatusCode === TStatusCode.ERROR_STATUS)
val elapsedTime = System.currentTimeMillis() - startTime
- assert(elapsedTime > 3 * 1000 && elapsedTime < 20 * 1000)
+ assert(elapsedTime > 10 * 1000 && elapsedTime < 20 * 1000)
Review Comment:
keep it the same with before?
--
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] turboFei closed pull request #2710: [KYUUBI #2594][FOLLOWUP] Fix flaky Test - support engine alive probe to fast fail on engine broken
Posted by GitBox <gi...@apache.org>.
turboFei closed pull request #2710: [KYUUBI #2594][FOLLOWUP] Fix flaky Test - support engine alive probe to fast fail on engine broken
URL: https://github.com/apache/incubator-kyuubi/pull/2710
--
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] turboFei commented on a diff in pull request #2710: [KYUUBI #2594][FOLLOWUP] Fix flaky Test - support engine alive probe to fast fail on engine broken
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2710:
URL: https://github.com/apache/incubator-kyuubi/pull/2710#discussion_r877709313
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerConnectionSuite.scala:
##########
@@ -231,10 +231,10 @@ class KyuubiOperationPerConnectionSuite extends WithKyuubiServer with HiveJDBCTe
test("KYUUBI #2102 - support engine alive probe to fast fail on engine broken") {
withSessionConf(Map(
KyuubiConf.ENGINE_ALIVE_PROBE_ENABLED.key -> "true",
- KyuubiConf.ENGINE_ALIVE_PROBE_INTERVAL.key -> "100",
- KyuubiConf.ENGINE_ALIVE_TIMEOUT.key -> "3000",
+ KyuubiConf.ENGINE_ALIVE_PROBE_INTERVAL.key -> "1000",
+ KyuubiConf.ENGINE_ALIVE_TIMEOUT.key -> "10000",
Review Comment:
how about reserve it as `3000`?
--
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] turboFei commented on pull request #2710: [KYUUBI #2594][FOLLOWUP] Fix flaky Test - support engine alive probe to fast fail on engine broken
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2710:
URL: https://github.com/apache/incubator-kyuubi/pull/2710#issuecomment-1132596027
thanks, merged 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 #2710: [KYUUBI #2594][FOLLOWUP] Fix flaky Test - support engine alive probe to fast fail on engine broken
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2710:
URL: https://github.com/apache/incubator-kyuubi/pull/2710#issuecomment-1132432154
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2710?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 [#2710](https://codecov.io/gh/apache/incubator-kyuubi/pull/2710?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2af2657) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/93753292957e248cd81dbc2696df0e896dd7a2a9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9375329) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.
> :exclamation: Current head 2af2657 differs from pull request most recent head d445790. Consider uploading reports for the commit d445790 to get more accurate results
```diff
@@ Coverage Diff @@
## master #2710 +/- ##
============================================
+ Coverage 65.00% 65.01% +0.01%
Complexity 284 284
============================================
Files 407 407
Lines 19571 19571
Branches 2575 2575
============================================
+ Hits 12722 12724 +2
+ Misses 5697 5693 -4
- Partials 1152 1154 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2710?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2710/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=) | `84.27% <0.00%> (-0.63%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2710/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==) | `88.12% <0.00%> (-0.46%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2710/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) | `59.64% <0.00%> (+7.01%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2710?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/2710?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 [9375329...d445790](https://codecov.io/gh/apache/incubator-kyuubi/pull/2710?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 diff in pull request #2710: [KYUUBI #2594][FOLLOWUP] Fix flaky Test - support engine alive probe to fast fail on engine broken
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on code in PR #2710:
URL: https://github.com/apache/incubator-kyuubi/pull/2710#discussion_r877710787
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/operation/KyuubiOperationPerConnectionSuite.scala:
##########
@@ -250,7 +250,7 @@ class KyuubiOperationPerConnectionSuite extends WithKyuubiServer with HiveJDBCTe
val executeStmtResp = client.ExecuteStatement(executeStmtReq)
assert(executeStmtResp.getStatus.getStatusCode === TStatusCode.ERROR_STATUS)
val elapsedTime = System.currentTimeMillis() - startTime
- assert(elapsedTime > 3 * 1000 && elapsedTime < 20 * 1000)
+ assert(elapsedTime > 10 * 1000 && elapsedTime < 20 * 1000)
Review Comment:
If we modify `ENGINE_ALIVE_TIMEOUT`, it should also be modified here.
I now change it to 3000.
--
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