You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/03/15 14:11:03 UTC
[GitHub] [kafka] dajac opened a new pull request #10318: KAFKA-12330; FetchSessionCache may cause starvation for partitions when FetchResponse is full
dajac opened a new pull request #10318:
URL: https://github.com/apache/kafka/pull/10318
The incremental FetchSessionCache sessions deprioritizes partitions where a response is returned. This may happen if log metadata such as log start offset, hwm, etc is returned, or if data for that partition is returned.
When a fetch response fills to maxBytes, data may not be returned for partitions even if the fetch offset is lower than the fetch upper bound. However, the fetch response will still contain updates to metadata such as hwm if that metadata has changed. This can lead to degenerate behavior where a partition's hwm or log start offset is updated resulting in the next fetch being unnecessarily skipped for that partition. At first this appeared to be worse, as hwm updates occur frequently, but starvation should result in hwm movement becoming blocked, allowing a fetch to go through and then becoming unstuck. However, it'll still require one more fetch request than necessary to do so. Consumers may be affected more than replica fetchers, however they often remove partitions with fetched data from the next fetch request and this may be helping prevent starvation.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] lbradstreet commented on a change in pull request #10318: KAFKA-12330; FetchSessionCache may cause starvation for partitions when FetchResponse is full
Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #10318:
URL: https://github.com/apache/kafka/pull/10318#discussion_r595047309
##########
File path: core/src/main/scala/kafka/server/FetchSession.scala
##########
@@ -428,7 +428,7 @@ class IncrementalFetchContext(private val time: Time,
val mustRespond = cachedPart.maybeUpdateResponseData(respData, updateFetchContextAndRemoveUnselected)
if (mustRespond) {
nextElement = element
- if (updateFetchContextAndRemoveUnselected) {
+ if (updateFetchContextAndRemoveUnselected && FetchResponse.recordsSize(respData) > 0) {
Review comment:
@chia7712 I believe it only affects the order of the fetching on the following fetch. Previously those partitions would be penalized in fetch order if their hwm or log start offset updated and there was no data to be returned.
All of the partitions where the fetch metadata had been updated without any data to be returned will end up behind the partitions ordered prior to them that were part of the full fetch 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10318: KAFKA-12330; FetchSessionCache may cause starvation for partitions when FetchResponse is full
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10318:
URL: https://github.com/apache/kafka/pull/10318#discussion_r595414671
##########
File path: core/src/main/scala/kafka/server/FetchSession.scala
##########
@@ -428,7 +428,7 @@ class IncrementalFetchContext(private val time: Time,
val mustRespond = cachedPart.maybeUpdateResponseData(respData, updateFetchContextAndRemoveUnselected)
if (mustRespond) {
nextElement = element
- if (updateFetchContextAndRemoveUnselected) {
+ if (updateFetchContextAndRemoveUnselected && FetchResponse.recordsSize(respData) > 0) {
Review comment:
Thanks for nice explanation!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10318: KAFKA-12330; FetchSessionCache may cause starvation for partitions when FetchResponse is full
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10318:
URL: https://github.com/apache/kafka/pull/10318#discussion_r595003883
##########
File path: core/src/main/scala/kafka/server/FetchSession.scala
##########
@@ -428,7 +428,7 @@ class IncrementalFetchContext(private val time: Time,
val mustRespond = cachedPart.maybeUpdateResponseData(respData, updateFetchContextAndRemoveUnselected)
if (mustRespond) {
nextElement = element
- if (updateFetchContextAndRemoveUnselected) {
+ if (updateFetchContextAndRemoveUnselected && FetchResponse.recordsSize(respData) > 0) {
Review comment:
Does it result in the side effect that we always fetch the no-data partitions?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rajinisivaram commented on pull request #10318: KAFKA-12330; FetchSessionCache may cause starvation for partitions when FetchResponse is full
Posted by GitBox <gi...@apache.org>.
rajinisivaram commented on pull request #10318:
URL: https://github.com/apache/kafka/pull/10318#issuecomment-800540196
@dajac Thanks for the PR, LGTM. Merging to trunk.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rajinisivaram merged pull request #10318: KAFKA-12330; FetchSessionCache may cause starvation for partitions when FetchResponse is full
Posted by GitBox <gi...@apache.org>.
rajinisivaram merged pull request #10318:
URL: https://github.com/apache/kafka/pull/10318
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org