You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2023/02/26 19:46:41 UTC
[GitHub] [kyuubi] pan3793 opened a new pull request, #4418: Metadata operation supports sync retry mode
pan3793 opened a new pull request, #4418:
URL: https://github.com/apache/kyuubi/pull/4418
<!--
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/CONTRIBUTING.html
2. If the PR is related to an issue in https://github.com/apache/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.
-->
Currently, we only support async retry for metadata operation, it's valuable since it can tolerate long-time metadata store outages w/o blocking the submission request, but it can not guarantee metadata consistency eventually.
This PR introduces a sync mode for metadata operation retry, which means when a metadata request failed, block the request thread and retry synchronously until success or reaches the threshold, it makes metadata consistent, and is necessary for some cases, e.g. in async mode, when inserting data violate the unique restriction, the user got success response but the retry will never succeed.
### _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.readthedocs.io/en/master/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] pan3793 commented on a diff in pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1119974021
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/MetadataManager.scala:
##########
@@ -37,44 +37,55 @@ class MetadataManager extends AbstractService("MetadataManager") {
private var _metadataStore: MetadataStore = _
// Visible for testing.
- private[metadata] val identifierRequestsRetryRefs =
+ private[metadata] val identifierRequestsAsyncRetryRefs =
new ConcurrentHashMap[String, MetadataRequestsRetryRef]()
// Visible for testing.
- private[metadata] val identifierRequestsRetryingCounts =
+ private[metadata] val identifierRequestsAsyncRetryingCounts =
new ConcurrentHashMap[String, AtomicInteger]()
- private val requestsRetryTrigger =
- ThreadUtils.newDaemonSingleThreadScheduledExecutor("metadata-requests-retry-trigger")
+ private lazy val requestsRetryInterval =
+ conf.get(KyuubiConf.METADATA_REQUEST_RETRY_INTERVAL)
- private var requestsRetryExecutor: ThreadPoolExecutor = _
+ private lazy val requestsAsyncRetryEnabled =
+ conf.get(KyuubiConf.METADATA_REQUEST_ASYNC_RETRY_ENABLED)
Review Comment:
yea, the key changes are
1. allow to disable retry
2. always fail fast on "duplicated key on unique index" error
--
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] ulysses-you commented on pull request #4418: Metadata operation supports sync retry
Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#issuecomment-1447692576
retry synchronously may introduce state consistent issue. say, the first action udates state to running and second action (different thread) upadates state to cancel, and the first action fails but retry successfully.
--
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 a diff in pull request #4418: Metadata operation supports sync retry mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1118248902
##########
docs/deployment/settings.md:
##########
@@ -403,9 +403,11 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
| kyuubi.metadata.cleaner.interval | PT30M | The interval to check and clean expired metadata. | duration | 1.6.0 |
| kyuubi.metadata.max.age | PT72H | The maximum age of metadata, the metadata exceeding the age will be cleaned. | duration | 1.6.0 |
| kyuubi.metadata.recovery.threads | 10 | The number of threads for recovery from the metadata store when the Kyuubi server restarts. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.interval | PT5S | The interval to check and trigger the metadata request retry tasks. | duration | 1.6.0 |
-| kyuubi.metadata.request.retry.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.threads | 10 | Number of threads in the metadata request retry manager thread pool. The metadata store might be unavailable sometimes and the requests will fail, tolerant for this case and unblock the main thread, we support retrying the failed requests in an async way. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.enabled | false | Whether to retry in async when metadata request failed. If false, when metadata request failed, block the request thread and retry synchronously until success or reach the threshold; if true, return immediately even the metadata request failed, and schedule it in background until success. In async mode, we can not guarantee metadata consistency eventually, but it's still valuable since we can tolerate long-time metadata store outages w/o blocking the submission request. | boolean | 1.7.0 |
+| kyuubi.metadata.request.retry.async.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. Only take affect when kyuubi.metadata.request.retry.async.enabled is `true`. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.threads | 10 | Number of threads in the metadata request retry manager thread pool. Only take affect when kyuubi.metadata.request.retry.async.enabled is `true`. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.interval | PT5S | The interval for metadata request retry. | duration | 1.6.0 |
+| kyuubi.metadata.request.retry.maxAttempts | 3 | The max attempts for metadata request retry. | int | 1.7.0 |
Review Comment:
is it possible to lose metadata for async mode if the metastore does not recover in-time.
--
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] pan3793 commented on pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#issuecomment-1448161031
Thanks, merged to master/1.7
--
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] pan3793 commented on pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#issuecomment-1448077891
> can you update the outdate pr description ?
updated
--
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] pan3793 commented on a diff in pull request #4418: Metadata operation supports sync retry mode
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1118541610
##########
docs/deployment/settings.md:
##########
@@ -403,9 +403,11 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
| kyuubi.metadata.cleaner.interval | PT30M | The interval to check and clean expired metadata. | duration | 1.6.0 |
| kyuubi.metadata.max.age | PT72H | The maximum age of metadata, the metadata exceeding the age will be cleaned. | duration | 1.6.0 |
| kyuubi.metadata.recovery.threads | 10 | The number of threads for recovery from the metadata store when the Kyuubi server restarts. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.interval | PT5S | The interval to check and trigger the metadata request retry tasks. | duration | 1.6.0 |
-| kyuubi.metadata.request.retry.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.threads | 10 | Number of threads in the metadata request retry manager thread pool. The metadata store might be unavailable sometimes and the requests will fail, tolerant for this case and unblock the main thread, we support retrying the failed requests in an async way. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.enabled | false | Whether to retry in async when metadata request failed. If false, when metadata request failed, block the request thread and retry synchronously until success or reach the threshold; if true, return immediately even the metadata request failed, and schedule it in background until success. In async mode, we can not guarantee metadata consistency eventually, but it's still valuable since we can tolerate long-time metadata store outages w/o blocking the submission request. | boolean | 1.7.0 |
+| kyuubi.metadata.request.retry.async.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. Only take affect when kyuubi.metadata.request.retry.async.enabled is `true`. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.threads | 10 | Number of threads in the metadata request retry manager thread pool. Only take affect when kyuubi.metadata.request.retry.async.enabled is `true`. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.interval | PT5S | The interval for metadata request retry. | duration | 1.6.0 |
+| kyuubi.metadata.request.retry.maxAttempts | 3 | The max attempts for metadata request retry. | int | 1.7.0 |
Review Comment:
case 1:
- t1: metadata store outage
- t2: client opens batch, kyuubi inserts metadata failed, but returns a successful response
- t3: server done
- t4: metadata store recovered.
case 2: kyuubi inserts metadata record but encounters "duplicated unique key" error, which means the insert will never succeed, it blocks all following update operations of this batch job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: 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] pan3793 commented on a diff in pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1119974021
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/MetadataManager.scala:
##########
@@ -37,44 +37,55 @@ class MetadataManager extends AbstractService("MetadataManager") {
private var _metadataStore: MetadataStore = _
// Visible for testing.
- private[metadata] val identifierRequestsRetryRefs =
+ private[metadata] val identifierRequestsAsyncRetryRefs =
new ConcurrentHashMap[String, MetadataRequestsRetryRef]()
// Visible for testing.
- private[metadata] val identifierRequestsRetryingCounts =
+ private[metadata] val identifierRequestsAsyncRetryingCounts =
new ConcurrentHashMap[String, AtomicInteger]()
- private val requestsRetryTrigger =
- ThreadUtils.newDaemonSingleThreadScheduledExecutor("metadata-requests-retry-trigger")
+ private lazy val requestsRetryInterval =
+ conf.get(KyuubiConf.METADATA_REQUEST_RETRY_INTERVAL)
- private var requestsRetryExecutor: ThreadPoolExecutor = _
+ private lazy val requestsAsyncRetryEnabled =
+ conf.get(KyuubiConf.METADATA_REQUEST_ASYNC_RETRY_ENABLED)
Review Comment:
yea
--
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] ulysses-you commented on pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#issuecomment-1448067501
can you update the outdate pr description ?
--
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] pan3793 closed pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
URL: https://github.com/apache/kyuubi/pull/4418
--
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] ulysses-you commented on a diff in pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1119965819
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/MetadataManager.scala:
##########
@@ -37,44 +37,55 @@ class MetadataManager extends AbstractService("MetadataManager") {
private var _metadataStore: MetadataStore = _
// Visible for testing.
- private[metadata] val identifierRequestsRetryRefs =
+ private[metadata] val identifierRequestsAsyncRetryRefs =
new ConcurrentHashMap[String, MetadataRequestsRetryRef]()
// Visible for testing.
- private[metadata] val identifierRequestsRetryingCounts =
+ private[metadata] val identifierRequestsAsyncRetryingCounts =
new ConcurrentHashMap[String, AtomicInteger]()
- private val requestsRetryTrigger =
- ThreadUtils.newDaemonSingleThreadScheduledExecutor("metadata-requests-retry-trigger")
+ private lazy val requestsRetryInterval =
+ conf.get(KyuubiConf.METADATA_REQUEST_RETRY_INTERVAL)
- private var requestsRetryExecutor: ThreadPoolExecutor = _
+ private lazy val requestsAsyncRetryEnabled =
+ conf.get(KyuubiConf.METADATA_REQUEST_ASYNC_RETRY_ENABLED)
Review Comment:
so this is the key change that introduce a config to control if we should 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 a diff in pull request #4418: Metadata operation supports sync retry mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1118248706
##########
docs/deployment/settings.md:
##########
@@ -403,9 +403,11 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
| kyuubi.metadata.cleaner.interval | PT30M | The interval to check and clean expired metadata. | duration | 1.6.0 |
| kyuubi.metadata.max.age | PT72H | The maximum age of metadata, the metadata exceeding the age will be cleaned. | duration | 1.6.0 |
| kyuubi.metadata.recovery.threads | 10 | The number of threads for recovery from the metadata store when the Kyuubi server restarts. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.interval | PT5S | The interval to check and trigger the metadata request retry tasks. | duration | 1.6.0 |
-| kyuubi.metadata.request.retry.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.threads | 10 | Number of threads in the metadata request retry manager thread pool. The metadata store might be unavailable sometimes and the requests will fail, tolerant for this case and unblock the main thread, we support retrying the failed requests in an async way. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.enabled | false | Whether to retry in async when metadata request failed. If false, when metadata request failed, block the request thread and retry synchronously until success or reach the threshold; if true, return immediately even the metadata request failed, and schedule it in background until success. In async mode, we can not guarantee metadata consistency eventually, but it's still valuable since we can tolerate long-time metadata store outages w/o blocking the submission request. | boolean | 1.7.0 |
+| kyuubi.metadata.request.retry.async.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. Only take affect when kyuubi.metadata.request.retry.async.enabled is `true`. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.threads | 10 | Number of threads in the metadata request retry manager thread pool. Only take affect when kyuubi.metadata.request.retry.async.enabled is `true`. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.interval | PT5S | The interval for metadata request retry. | duration | 1.6.0 |
+| kyuubi.metadata.request.retry.maxAttempts | 3 | The max attempts for metadata request retry. | int | 1.7.0 |
Review Comment:
behavior change for async mode.
--
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] pan3793 commented on a diff in pull request #4418: Metadata operation supports sync retry mode
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1118541610
##########
docs/deployment/settings.md:
##########
@@ -403,9 +403,11 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
| kyuubi.metadata.cleaner.interval | PT30M | The interval to check and clean expired metadata. | duration | 1.6.0 |
| kyuubi.metadata.max.age | PT72H | The maximum age of metadata, the metadata exceeding the age will be cleaned. | duration | 1.6.0 |
| kyuubi.metadata.recovery.threads | 10 | The number of threads for recovery from the metadata store when the Kyuubi server restarts. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.interval | PT5S | The interval to check and trigger the metadata request retry tasks. | duration | 1.6.0 |
-| kyuubi.metadata.request.retry.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.threads | 10 | Number of threads in the metadata request retry manager thread pool. The metadata store might be unavailable sometimes and the requests will fail, tolerant for this case and unblock the main thread, we support retrying the failed requests in an async way. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.enabled | false | Whether to retry in async when metadata request failed. If false, when metadata request failed, block the request thread and retry synchronously until success or reach the threshold; if true, return immediately even the metadata request failed, and schedule it in background until success. In async mode, we can not guarantee metadata consistency eventually, but it's still valuable since we can tolerate long-time metadata store outages w/o blocking the submission request. | boolean | 1.7.0 |
+| kyuubi.metadata.request.retry.async.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. Only take affect when kyuubi.metadata.request.retry.async.enabled is `true`. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.threads | 10 | Number of threads in the metadata request retry manager thread pool. Only take affect when kyuubi.metadata.request.retry.async.enabled is `true`. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.interval | PT5S | The interval for metadata request retry. | duration | 1.6.0 |
+| kyuubi.metadata.request.retry.maxAttempts | 3 | The max attempts for metadata request retry. | int | 1.7.0 |
Review Comment:
case 1:
- t1: metadata store outage
- t2: client opens batch, kyuubi inserts metadata failed, but returns a successful response
- t3: server down
- t4: metadata store recovered.
case 2: kyuubi inserts metadata record but encounters "duplicated unique key" error, which means the insert will never succeed, it blocks all following update operations of this batch job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: 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] codecov-commenter commented on pull request #4418: Metadata operation supports sync retry
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#issuecomment-1447657416
# [Codecov](https://codecov.io/gh/apache/kyuubi/pull/4418?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 [#4418](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d3e142) into [master](https://codecov.io/gh/apache/kyuubi/commit/59c1875bc16a4fa5a1f8e3e9d1c0c0ad77431e7e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (59c1875) will **decrease** coverage by `0.61%`.
> The diff coverage is `71.96%`.
```diff
@@ Coverage Diff @@
## master #4418 +/- ##
============================================
- Coverage 53.75% 53.15% -0.61%
Complexity 13 13
============================================
Files 564 569 +5
Lines 30978 31103 +125
Branches 4170 4205 +35
============================================
- Hits 16653 16533 -120
- Misses 12767 12997 +230
- Partials 1558 1573 +15
```
| [Impacted Files](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pache/kyuubi/server/metadata/MetadataRequest.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbWV0YWRhdGEvTWV0YWRhdGFSZXF1ZXN0LnNjYWxh) | `0.00% <ø> (ø)` | |
| [...ommon/src/main/scala/org/apache/kyuubi/Utils.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9VdGlscy5zY2FsYQ==) | `72.25% <60.00%> (-0.76%)` | :arrow_down: |
| [...pache/kyuubi/server/metadata/MetadataManager.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbWV0YWRhdGEvTWV0YWRhdGFNYW5hZ2VyLnNjYWxh) | `77.22% <67.01%> (-7.06%)` | :arrow_down: |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?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.33% <96.00%> (-0.22%)` | :arrow_down: |
| [.../kyuubi/engine/spark/operation/ExecutePython.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVB5dGhvbi5zY2FsYQ==) | `0.00% <0.00%> (-81.47%)` | :arrow_down: |
| [...e/spark/api/python/KyuubiPythonGatewayServer.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3BhcmsvYXBpL3B5dGhvbi9LeXV1YmlQeXRob25HYXRld2F5U2VydmVyLnNjYWxh) | `0.00% <0.00%> (-73.92%)` | :arrow_down: |
| [...ache/kyuubi/plugin/spark/authz/serde/package.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L3NlcmRlL3BhY2thZ2Uuc2NhbGE=) | `65.62% <0.00%> (-18.75%)` | :arrow_down: |
| [.../authentication/EngineSecuritySecretProvider.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0VuZ2luZVNlY3VyaXR5U2VjcmV0UHJvdmlkZXIuc2NhbGE=) | `84.61% <0.00%> (-15.39%)` | :arrow_down: |
| [...ine/spark/operation/SparkSQLOperationManager.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vU3BhcmtTUUxPcGVyYXRpb25NYW5hZ2VyLnNjYWxh) | `79.79% <0.00%> (-6.07%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/spark/SparkSQLEngine.scala](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9TcGFya1NRTEVuZ2luZS5zY2FsYQ==) | `68.65% <0.00%> (-1.50%)` | :arrow_down: |
| ... and [23 more](https://codecov.io/gh/apache/kyuubi/pull/4418?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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 a diff in pull request #4418: Metadata operation supports sync retry mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1118248591
##########
docs/deployment/settings.md:
##########
@@ -403,9 +403,11 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
| kyuubi.metadata.cleaner.interval | PT30M | The interval to check and clean expired metadata. | duration | 1.6.0 |
| kyuubi.metadata.max.age | PT72H | The maximum age of metadata, the metadata exceeding the age will be cleaned. | duration | 1.6.0 |
| kyuubi.metadata.recovery.threads | 10 | The number of threads for recovery from the metadata store when the Kyuubi server restarts. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.interval | PT5S | The interval to check and trigger the metadata request retry tasks. | duration | 1.6.0 |
-| kyuubi.metadata.request.retry.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.threads | 10 | Number of threads in the metadata request retry manager thread pool. The metadata store might be unavailable sometimes and the requests will fail, tolerant for this case and unblock the main thread, we support retrying the failed requests in an async way. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.enabled | false | Whether to retry in async when metadata request failed. If false, when metadata request failed, block the request thread and retry synchronously until success or reach the threshold; if true, return immediately even the metadata request failed, and schedule it in background until success. In async mode, we can not guarantee metadata consistency eventually, but it's still valuable since we can tolerate long-time metadata store outages w/o blocking the submission request. | boolean | 1.7.0 |
Review Comment:
looks like behavior change
--
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] pan3793 commented on a diff in pull request #4418: Metadata operation supports sync retry mode
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#discussion_r1119565429
##########
docs/deployment/settings.md:
##########
@@ -403,9 +403,11 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
| kyuubi.metadata.cleaner.interval | PT30M | The interval to check and clean expired metadata. | duration | 1.6.0 |
| kyuubi.metadata.max.age | PT72H | The maximum age of metadata, the metadata exceeding the age will be cleaned. | duration | 1.6.0 |
| kyuubi.metadata.recovery.threads | 10 | The number of threads for recovery from the metadata store when the Kyuubi server restarts. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.interval | PT5S | The interval to check and trigger the metadata request retry tasks. | duration | 1.6.0 |
-| kyuubi.metadata.request.retry.queue.size | 65536 | The maximum queue size for buffering metadata requests in memory when the external metadata storage is down. Requests will be dropped if the queue exceeds. | int | 1.6.0 |
-| kyuubi.metadata.request.retry.threads | 10 | Number of threads in the metadata request retry manager thread pool. The metadata store might be unavailable sometimes and the requests will fail, tolerant for this case and unblock the main thread, we support retrying the failed requests in an async way. | int | 1.6.0 |
+| kyuubi.metadata.request.retry.async.enabled | false | Whether to retry in async when metadata request failed. If false, when metadata request failed, block the request thread and retry synchronously until success or reach the threshold; if true, return immediately even the metadata request failed, and schedule it in background until success. In async mode, we can not guarantee metadata consistency eventually, but it's still valuable since we can tolerate long-time metadata store outages w/o blocking the submission request. | boolean | 1.7.0 |
Review Comment:
mentioned in migration guide
--
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] ulysses-you commented on pull request #4418: Allow disable metadata operation async retry and fail fast on unrecoverable DB error
Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #4418:
URL: https://github.com/apache/kyuubi/pull/4418#issuecomment-1448087613
it looks fine to me, can you double check if it breaks anything with you ? @turboFei
--
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