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/06/27 04:52:17 UTC
[GitHub] [incubator-kyuubi] turboFei opened a new pull request, #2948: remove request timeout
turboFei opened a new pull request, #2948:
URL: https://github.com/apache/incubator-kyuubi/pull/2948
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
### _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] [kyuubi] risyomei commented on pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by "risyomei (via GitHub)" <gi...@apache.org>.
risyomei commented on PR #2948:
URL: https://github.com/apache/kyuubi/pull/2948#issuecomment-1455064755
@yaooqinn @turboFei
Hello, Sorry to disturb you while you are busy.
May I know the motivation for deleting the 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] codecov-commenter commented on pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2948:
URL: https://github.com/apache/incubator-kyuubi/pull/2948#issuecomment-1166894925
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948?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 [#2948](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9c5c08) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/37c0d4258c5c05cd31dcdaa4cd44fa0e97f607f8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37c0d42) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #2948 +/- ##
============================================
- Coverage 62.89% 62.88% -0.02%
Complexity 6 6
============================================
Files 418 418
Lines 20283 20252 -31
Branches 2594 2589 -5
============================================
- Hits 12757 12735 -22
+ Misses 6285 6283 -2
+ Partials 1241 1234 -7
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948?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/2948/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==) | `97.18% <ø> (-0.02%)` | :arrow_down: |
| [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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.28% <100.00%> (-0.05%)` | :arrow_down: |
| [...e/authentication/KyuubiAuthenticationFactory.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0t5dXViaUF1dGhlbnRpY2F0aW9uRmFjdG9yeS5zY2FsYQ==) | `77.94% <0.00%> (-1.48%)` | :arrow_down: |
| [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `68.00% <0.00%> (-1.34%)` | :arrow_down: |
| [...che/kyuubi/server/KyuubiTHttpFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvS3l1dWJpVEh0dHBGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `63.97% <0.00%> (-0.48%)` | :arrow_down: |
| [...ver/http/authentication/AuthenticationFilter.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvaHR0cC9hdXRoZW50aWNhdGlvbi9BdXRoZW50aWNhdGlvbkZpbHRlci5zY2FsYQ==) | `91.52% <0.00%> (-0.15%)` | :arrow_down: |
| [.../apache/kyuubi/server/http/ThriftHttpServlet.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvaHR0cC9UaHJpZnRIdHRwU2VydmxldC5zY2FsYQ==) | `69.53% <0.00%> (+0.30%)` | :arrow_up: |
| [.../kyuubi/plugin/spark/authz/PrivilegesBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L1ByaXZpbGVnZXNCdWlsZGVyLnNjYWxh) | `69.44% <0.00%> (+0.30%)` | :arrow_up: |
| [...ache/kyuubi/plugin/spark/authz/OperationType.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L09wZXJhdGlvblR5cGUuc2NhbGE=) | `58.82% <0.00%> (+1.17%)` | :arrow_up: |
| [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `80.00% <0.00%> (+2.00%)` | :arrow_up: |
| ... and [1 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948/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/2948?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/2948?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 [37c0d42...a9c5c08](https://codecov.io/gh/apache/incubator-kyuubi/pull/2948?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] [kyuubi] turboFei commented on pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #2948:
URL: https://github.com/apache/kyuubi/pull/2948#issuecomment-1455276102
The thrift protocol does not support retry.
--
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] [kyuubi] turboFei commented on pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #2948:
URL: https://github.com/apache/kyuubi/pull/2948#issuecomment-1455555311
> Do you have a ticket to describe the "out of sequence issue"?
about `out of sequence issue`.
```
In the context of Apache Thrift, "out of sequence issue" can refer to a situation where a Thrift server receives a message from a client that is out of order, meaning the server receives a message with a sequence number that is lower than expected.
This can happen if there is a delay or a disruption in the network communication between the client and the server, causing messages to be delivered out of order. If the server is not able to handle such out of sequence messages properly, it can result in incorrect behavior or even crashes.
To address this issue, Thrift includes a sequence number in each message to help the server determine if a message is out of order. If a server receives an out of sequence message, it can respond with an error message indicating the issue, or it can buffer the message until the expected sequence number is received.
Thrift also provides an option for clients to request ordered delivery of messages, which instructs the server to buffer and deliver messages in the correct order, even if they arrive out of sequence. This option can be useful in situations where network disruptions are common, but it can also impact performance, as buffering and reordering messages can introduce additional latency.
```
For example:
1. client send fetchResult request -> timeout , then client does not receive the fetchResult response.
2. client send getOpStatus request -> then receive the last `fetchResult response`(out of sequence because expected result is getOpStatus response`
--
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 #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2948:
URL: https://github.com/apache/incubator-kyuubi/pull/2948#issuecomment-1166945312
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] [kyuubi] risyomei commented on pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by "risyomei (via GitHub)" <gi...@apache.org>.
risyomei commented on PR #2948:
URL: https://github.com/apache/kyuubi/pull/2948#issuecomment-1455515919
@turboFei
Thank you very much for your response.
Do you have a ticket to describe the "out of sequence issue"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] risyomei commented on pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by "risyomei (via GitHub)" <gi...@apache.org>.
risyomei commented on PR #2948:
URL: https://github.com/apache/kyuubi/pull/2948#issuecomment-1456091696
@turboFei
Thank you very much for your explanation.
It's very easy to follow!
> client send fetchResult request -> timeout , then client does not receive the fetchResult response.
client send getOpStatus request -> then receive the last fetchResult response(out of sequence because expected result is getOpStatus response`
By the way, I have found a possible issue (https://github.com/apache/kyuubi/issues/4457) that are kind of related to this issue.
It will be very helpful if you can take a look at that as well 🙇♂️
--
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] [kyuubi] turboFei commented on pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #2948:
URL: https://github.com/apache/kyuubi/pull/2948#issuecomment-1455119160
> @yaooqinn @turboFei
>
> Hello, Sorry to disturb you while you are busy.
>
>
>
> May I know the motivation for deleting the timeout?
Remove it to prevent out of sequence issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei closed pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
Posted by GitBox <gi...@apache.org>.
turboFei closed pull request #2948: Remove thrift request timeout for KyuubiSyncThriftClient
URL: https://github.com/apache/incubator-kyuubi/pull/2948
--
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