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

Thread safety on producer.metrics() #1681

Closed
emeric254 opened this issue Dec 20, 2018 · 3 comments
Closed

Thread safety on producer.metrics() #1681

emeric254 opened this issue Dec 20, 2018 · 3 comments

Comments

@emeric254
Copy link
Contributor

It's seems there is a thread safety issue when retrieving producer metrics from an independent thread.

We did not encounter this behavior on consumers but we checked the library code it should be affected too.

This is the error we got when reading metrics for a producer:

File "...\lib\site-packages\kafka\producer\kafka.py", line 722, in metrics
    for k, v in six.iteritems(self._metrics.metrics):
RuntimeError: dictionary changed size during iteration

Here is the current faulty code in the producer (note the direct use of self._metrics.metrics):

def metrics(self, raw=False):
    [...]
    metrics = {}
    for k, v in six.iteritems(self._metrics.metrics):
        if k.group not in metrics:
            metrics[k.group] = {}
        if k.name not in metrics[k.group]:
            metrics[k.group][k.name] = {}
        metrics[k.group][k.name] = v.value()
    return metrics

One solution is to return a copy of actual metrics :

def metrics(self, raw=False):
    [...]
    metrics = {}
    for k, v in six.iteritems(self._metrics.metrics.copy()):
        if k.group not in metrics:
            metrics[k.group] = {}
        if k.name not in metrics[k.group]:
            metrics[k.group][k.name] = {}
        metrics[k.group][k.name] = v.value()
    return metrics

I will create a PR for this

@emeric254
Copy link
Contributor Author

see PR #1682

@tvoinarovskyi
Copy link
Collaborator

Well, it makes sense for Producer, as we declare it as Thread safe for sending.

@tvoinarovskyi
Copy link
Collaborator

Closing this, as the fix was merged. Thanks for the contribution!

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

No branches or pull requests

2 participants