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/05 00:09:12 UTC
[GitHub] [kafka] splett2 opened a new pull request #10267: KAFKA-12427: Don't update connection idle time for muted connections
splett2 opened a new pull request #10267:
URL: https://github.com/apache/kafka/pull/10267
`Selector.poll()` will always call `pollSelectionKeys()` for channels with buffered data. `pollSelectionKeys()` will always update connection last idle time, even if the channel is muted and we don't actually read from the channel.
There is an existing unit test `idleExpiryWithBufferedReceives` that fails to catch this behavior because the `MockTime` object used in the test is updated in a large enough increment to expire a connection between calls to `poll()`. After updating the test to advance time in smaller increments, the test fails without the Selector change.
### 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] splett2 commented on pull request #10267: KAFKA-12427: Don't update connection idle time for muted connections
Posted by GitBox <gi...@apache.org>.
splett2 commented on pull request #10267:
URL: https://github.com/apache/kafka/pull/10267#issuecomment-791045318
cc @rajinisivaram for reviews/thoughts
This approach is fairly simple, and we still end up calling `pollSelectionKeys()` with muted channels. An alternative that I was thinking of would be to remove a kafka channel from `keysWithBufferedReads` when explicitly muting, and then checking whether the channel has buffered data when unmuting.
----------------------------------------------------------------
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 #10267: KAFKA-12427: Don't update connection idle time for muted connections
Posted by GitBox <gi...@apache.org>.
rajinisivaram merged pull request #10267:
URL: https://github.com/apache/kafka/pull/10267
----------------------------------------------------------------
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 #10267: KAFKA-12427: Don't update connection idle time for muted connections
Posted by GitBox <gi...@apache.org>.
rajinisivaram commented on pull request #10267:
URL: https://github.com/apache/kafka/pull/10267#issuecomment-791420001
@splett2 Thanks for the PR. I think it would be better to store channels in `keysWithBufferedReads` only when the channel is not explicitly muted since we care about this state only when we are ready to read from the channel. So we could check if channel is explicitly muted when adding to `keysWithBufferedReads` and add the key to `keysWithBufferedReads` when unmuting. This would avoid processing the channel unnecessarily.
----------------------------------------------------------------
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] splett2 commented on pull request #10267: KAFKA-12427: Don't update connection idle time for muted connections
Posted by GitBox <gi...@apache.org>.
splett2 commented on pull request #10267:
URL: https://github.com/apache/kafka/pull/10267#issuecomment-793092807
@rajinisivaram
thanks for the review, that makes sense to me.
----------------------------------------------------------------
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 a change in pull request #10267: KAFKA-12427: Don't update connection idle time for muted connections
Posted by GitBox <gi...@apache.org>.
rajinisivaram commented on a change in pull request #10267:
URL: https://github.com/apache/kafka/pull/10267#discussion_r588308473
##########
File path: clients/src/main/java/org/apache/kafka/common/network/Selector.java
##########
@@ -517,7 +517,7 @@ void pollSelectionKeys(Set<SelectionKey> selectionKeys,
// register all per-connection metrics at once
sensors.maybeRegisterConnectionMetrics(nodeId);
- if (idleExpiryManager != null)
+ if (idleExpiryManager != null && !explicitlyMutedChannels.contains(channel))
Review comment:
This is limiting the times when we perform the update. For example when sending response, the channel is active, but explicitly muted. Don't think we want to skip update in this case.
----------------------------------------------------------------
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