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 2023/01/17 10:30:56 UTC
[GitHub] [kafka] yufeiyan1220 opened a new pull request, #13125: KAFKA-14626 Kafka Consumer Coordinator does not cleanup all metrics
yufeiyan1220 opened a new pull request, #13125:
URL: https://github.com/apache/kafka/pull/13125
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by "guozhangwang (via GitHub)" <gi...@apache.org>.
guozhangwang commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1421133959
@yufeiyan1220 just following up on this PR. Do you want to address the latest comment, I'm thinking if we can just move `throttleTimeSensor` to the Selector while keeping its group name a special case to avoid breaking compatibility?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] github-actions[bot] commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1585816382
This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has merge conflicts, please update it with the latest from trunk (or appropriate release branch) <p> If this PR is no longer valid or desired, please feel free to close it. If no activity occurrs in the next 30 days, it will be automatically closed.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] philipnee commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by GitBox <gi...@apache.org>.
philipnee commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1397952270
Seems like we aren't particularly consistent at removing these metrics and sensors, fetcher would be another example. Mind making the clean up more comprehensive?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] yufeiyan1220 commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by GitBox <gi...@apache.org>.
yufeiyan1220 commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1397837046
> Thanks for the PR and the issue @yufeiyan1220 - I wonder if the clean up is necessary, as the metrics will be closed upon the client closing. Willing to hear what others say.
I found that problem when I tried to create and close consumer frequently. Just like the test case I have made in `KafkaConsumerTest.java`, these consumer coordinator metrics are still present if I does not close these metrics explicitly. I think most of users might not try to create and close consumer frequently, but there is a potential for memory leak actually.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] yufeiyan1220 commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by "yufeiyan1220 (via GitHub)" <gi...@apache.org>.
yufeiyan1220 commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1399206848
> Seems like we aren't particularly consistent at removing these metrics and sensors, fetcher would be another example. Mind making the clean up more comprehensive?
I have made the change that is ensure all metrics about consumer are removed after shutting down. When it comes to producer, I found that all producer metrics are already closed, so I don't need to add more stuff for producer.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] philipnee commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by GitBox <gi...@apache.org>.
philipnee commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1396238469
Thanks for the PR and the issue @yufeiyan1220 - I wonder if the clean up is necessary, as the metrics will be closed upon the client closing. Willing to hear what others say.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] yufeiyan1220 commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by GitBox <gi...@apache.org>.
yufeiyan1220 commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1397979098
> Seems like we aren't particularly consistent at removing these metrics and sensors, fetcher would be another example. Mind making the clean up more comprehensive?
Never mind. I think it should be better to make the metric removal logic consistent. Let me take some time to look for these cases.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] guozhangwang commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by "guozhangwang (via GitHub)" <gi...@apache.org>.
guozhangwang commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1401212144
The concern I had is that, the same metrics registry object is used across multiple nested classes, while some of those classes may be closed and re-created along the lifetime of the client, and if we only try to close the metrics registry once upon closing the client, we may miss those events.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] philipnee commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by "philipnee (via GitHub)" <gi...@apache.org>.
philipnee commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1401162257
Thanks @guozhangwang - A comment I have here is, would it be more convenient to close/remove all the metrics and sensors in the Metrics class upon closing instead of relying on these sub metrics to remove the metrics/sensors? It seems fine to just remove the registered sensros and metrics upon closing the Metrics obj.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] philipnee commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by "philipnee (via GitHub)" <gi...@apache.org>.
philipnee commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1400882904
hey @cmccabe - would you have time to take a look at this?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] yufeiyan1220 commented on pull request #13125: KAFKA-14626: Kafka Consumer Coordinator does not cleanup all metrics after shutdown
Posted by "yufeiyan1220 (via GitHub)" <gi...@apache.org>.
yufeiyan1220 commented on PR #13125:
URL: https://github.com/apache/kafka/pull/13125#issuecomment-1427011395
> throttleTimeSensor
Sorry for my late reply because I am busy doing some other work.... Yeah, I am ready to do it now.
I found that `throttleTimeSensor` uses `FetcherMetricsRegistry` as creation parameter. Which one is better to move `throttleTimeSensor` into `NetworkClient`, with `FetcherMetricsRegistry` as the parameter to create `throttleTimeSensor` within the `NetworkClient`, or just a removal logic when closing the `NetworkClient`?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org