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

producer/kafka: Disable logging during object destruction #2043

Merged
merged 1 commit into from
Sep 7, 2020

Conversation

gioele
Copy link
Contributor

@gioele gioele commented May 1, 2020

Logging in an object destructor leads to crashes like the following

Exception ignored in: <bound method KafkaProducer.__del__ of 
    <kafka.producer.kafka.KafkaProducer object at 0x7f0c47309b38>>
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/kafka/producer/kafka.py", line 447, in __del__
  File "/usr/local/lib/python3.6/site-packages/kafka/producer/kafka.py", line 460, in close
  File "/usr/local/lib/python3.6/logging/__init__.py", line 1305, in info
  File "/usr/local/lib/python3.6/logging/__init__.py", line 1546, in isEnabledFor
TypeError: '>=' not supported between instances of 'int' and 'NoneType'

In closing PR #1445, @dpkp made it clear that producers should be closed while they are being destructed.

This commit does not modify the existing architecture and code structure, and preserves all the logging calls in close. At the same time, by "nullifying" the logger, it fixes the crash that is inherent in the use of logging methods during the destruction of an object.

Fixes #1218.


This change is Reviewable

Logging in an object destructor leads to crashes like the following

```
Exception ignored in: <bound method KafkaProducer.__del__ of <kafka.producer.kafka.KafkaProducer object at 0x7f0c47309b38>>
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/kafka/producer/kafka.py", line 447, in __del__
  File "/usr/local/lib/python3.6/site-packages/kafka/producer/kafka.py", line 460, in close
  File "/usr/local/lib/python3.6/logging/__init__.py", line 1305, in info
  File "/usr/local/lib/python3.6/logging/__init__.py", line 1546, in isEnabledFor
TypeError: '>=' not supported between instances of 'int' and 'NoneType'
```

In issue dpkp#1218 and dpkp#1445, @dpkp made it clear that producers should be
closed while they are being destructed.

This commit does not modify the existing architecture and code structure,
and preserves all the logging calls in `close`. At the same time, by
"nullifying" the logger, it fixes the crash that is inherent in the use
of logging methods during the destruction of an object.
@gioele gioele force-pushed the fix-log-crash-during-destr branch from 38f2d74 to 20f3cca Compare May 1, 2020 19:07
@rjduffner
Copy link

@dpkp, this has been open for awhile, is there any chance this makes it in?

@dpkp dpkp merged commit e4913db into dpkp:master Sep 7, 2020
@mauhcs
Copy link

mauhcs commented Sep 8, 2020

I see that the PR explicitly says that it is preserving all the log calls int he close methods, but why is that?
Could the line log.info("Closing the Kafka producer with %s secs timeout.", timeout) in the close method be commented out?

@dpkp
Copy link
Owner

dpkp commented Sep 8, 2020

I see that the PR explicitly says that it is preserving all the log calls int he close methods, but why is that?
Could the line log.info("Closing the Kafka producer with %s secs timeout.", timeout) in the close method be commented out?

This is still a useful log message at INFO level, which generally should say when a producer connects, disconnects, etc.

@gioele gioele deleted the fix-log-crash-during-destr branch September 8, 2020 05:54
gabriel-tincu pushed a commit to aiven/kafka-python that referenced this pull request Sep 22, 2020
chandlernine pushed a commit to layer9ai/kafka-python that referenced this pull request Jun 9, 2021
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.

Should Kafka stop logging during shutdown?
4 participants