Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the metrics deregistration in AbstractKafkaConnector when multiple stop are called #865

Merged
merged 5 commits into from
Nov 5, 2021

Conversation

vmaheshw
Copy link
Collaborator

@vmaheshw vmaheshw commented Nov 5, 2021

Metric registration and deregistration are ref counted. For aggregate metrics, all the tasks share the same metric object. If the ref-count gets to zero, the metric Register class removes the metrics and on next registration, creates a new metric object that will be emitted outside the container. So, if there are more de-registration than the registration, some of the live running tasks will point to a metric object, that no longer exists in Metric Registry, resulting in incorrect Aggregate metrics.

This PR addresses and fixes that deregistration of metrics should happen only once per task thread.

@vmaheshw vmaheshw merged commit 4522c52 into linkedin:master Nov 5, 2021
@vmaheshw vmaheshw deleted the fixAggMetrics branch November 5, 2021 18:50
vmaheshw added a commit to vmaheshw/brooklin that referenced this pull request Mar 1, 2022
…e stop are called (linkedin#865)

Metric registration and deregistration are ref counted. For aggregate metrics, all the tasks share the same metric object. If the ref-count gets to zero, the metric Register class removes the metrics and on next registration, creates a new metric object that will be emitted outside the container. So, if there are more de-registration than the registration, some of the live running tasks will point to a metric object, that no longer exists in Metric Registry, resulting in incorrect Aggregate metrics.

This PR addresses and fixes that deregistration of metrics should happen only once per task thread.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants