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 2020/05/17 19:12:33 UTC
[GitHub] [kafka] abbccdda opened a new pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
abbccdda opened a new pull request #8682:
URL: https://github.com/apache/kafka/pull/8682
As stated, we couldn't wait for handleRebalanceComplete in the case of handleLostAll, as we already closed the active task as dirty, and could potentially require its offset in the next thread.runOnce call.
### 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] guozhangwang commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-631070764
> @guozhangwang Why would we warn? You mean if we have any tasks leftover? These are just tasks that are currently assigned
I think we would only have leftover tasks if they are closed due to exception during the rebalance.
----------------------------------------------------------------
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] ableegoldman edited a comment on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
ableegoldman edited a comment on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-630522346
I also think we should reset/clear the set at the beginning of `tryToLockAllNonEmptyTaskDirectories`, so basically we're always dealing with the current set of actually-locked tasks and don't need to worry about removing them during `handleLostAll` or `handleCorruption/Assignment`, etc
----------------------------------------------------------------
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] abbccdda commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
abbccdda commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-629937689
test this please
----------------------------------------------------------------
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] guozhangwang commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-630515587
I agree with @ableegoldman here, after the `while (taskIdIterator.hasNext()` loop we can see if there are still remaining tasks, and then log an WARN similar to the end of `handleRevocation` before clearing them:
```
if (!remainingPartitions.isEmpty()) {
log.warn("The following partitions {} are missing from the task partitions. It could potentially " +
"due to race condition of consumer detecting the heartbeat failure, or the tasks " +
"have been cleaned up by the handleAssignment callback.", remainingPartitions);
}
```
----------------------------------------------------------------
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] ableegoldman commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-630344469
Instead of removing the tasks during `handleLostAll`, can we just clear the `lockedTaskDirectories` set at the end of `releaseLockedUnassignedTaskDirectories`? This set is only used to keep track of which task directories we only temporarily locked for the rebalance, so it makes sense that it should be empty outside of a rebalance.
----------------------------------------------------------------
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] ableegoldman commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-631068928
@guozhangwang Why would we warn? You mean if we have any tasks leftover? These are just tasks that are currently assigned
----------------------------------------------------------------
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] guozhangwang merged pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
guozhangwang merged pull request #8682:
URL: https://github.com/apache/kafka/pull/8682
----------------------------------------------------------------
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] vvcephei commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
vvcephei commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-630272579
Test this please
----------------------------------------------------------------
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] ableegoldman commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-630522346
I also think we should reset/clear the set at the beginning of `tryToLockAllNonEmptyTaskDirectories`
----------------------------------------------------------------
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] guozhangwang commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-631068405
I think it is still better to log a warn at the end of `releaseLockedUnassignedTaskDirectories` if the map is not empty at the end, will add it before merging.
----------------------------------------------------------------
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] abbccdda commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
abbccdda commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-630551559
Will attempt Sophie's suggestion here
----------------------------------------------------------------
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] ableegoldman commented on pull request #8682: KAFKA-10011: Remove task id from lockedTaskDirectories during handleLostAll
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #8682:
URL: https://github.com/apache/kafka/pull/8682#issuecomment-631072710
Well, `releaseLockedUnassignedTaskDirectories` does exactly that -- it only releases the unassigned task directories (which we just lock in order to report the offset sums in the subscription info). So I would expect there to almost always be leftover tasks at the end of that method
----------------------------------------------------------------
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