fix threafing bug with send buffer and threaded workers #335
Conversation
When using threaded workers (e.g. Django runserver in Django 2.0+), writes to the send buffer need to be guarded by a threading lock. Otherwise we end up with garbled content in the buffer, which then later on leads to parsing errors in the APM Server. fixes elastic#334
| self._last_flush = timeit.default_timer() | ||
| self._flush_timer = None | ||
|
|
||
| def queue(self, event_type, data, flush=False): | ||
| self._queue(self.queued_data, {event_type: data}) | ||
| with self._queue_lock: | ||
| self.queued_data.write((self._json_serializer({event_type: data}) + "\n").encode("utf-8")) | ||
| since_last_flush = timeit.default_timer() - self._last_flush | ||
| queue_size = self.queued_data_size |
There was a problem hiding this comment.
self.queued_data_size calls self.queued_data so calls to self.queued_data_size must also be protected by the lock, or you lock inside self.queued_data
There was a problem hiding this comment.
Great catch. I'll remove the call to self.queued_data in queued_data_size and instead check for existence of self._queued_data directly and return 0 if it doesn't exist. Then we don't need a lock as it is read only. Makes sense?
There was a problem hiding this comment.
yeah that would probably work. It seems like queue_size here could end up being the size of a queue that was just flushed and we would then flush again, but that might not be an issue
There was a problem hiding this comment.
agree, this shouldn't really matter
this makes the use of a lock for this method unnecessary
|
Just an FYI on commit msg, Django runserver is threaded since 1.4 |
|
@ciarancourtney oh, you're right. Weirdly enough, I can't replicate the issue in Django 1.11 with runserver |
When using threaded workers (e.g. Django runserver), writes to the send buffer need to be guarded by a threading lock. Otherwise we end up with garbled content in the buffer, which then later on leads to parsing errors in the APM Server. fixes #334
|
merged in 4a02ee5 |
When using threaded workers (e.g. Django runserver in Django 2.0+),
writes to the send buffer need to be guarded by a threading lock.
Otherwise we end up with garbled content in the buffer, which then later
on leads to parsing errors in the APM Server.
fixes #334