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.

Sign up for GitHub

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

Jump to bottom

Massive threading and concurrency issues in async #105

Closed
arcivanov opened this issue Dec 5, 2017 · 0 comments
Closed

Massive threading and concurrency issues in async #105

arcivanov opened this issue Dec 5, 2017 · 0 comments

Comments

@arcivanov
Copy link
Member

arcivanov commented Dec 5, 2017 •
edited
Loading

While investigating closure issues in async I realized it was suboptimally implemented with several concurrency issues. Firstly, there was about 3 times the amount of code than was necessary. Secondly, the flush, drain and queue_timeout were all added to cover over the fact that the async thread couldn't be stopped reliably.

This became a massive rewrite of the entire async handler and sender, which shrunk the code dramatically, removed queue timeout, moved flush to default (observing the expected behavior of the Handler) and removed all possible race conditions on closing and sending.

Finally virtually all tests have been rewritten and cleaned up as they were reporting failures as successes. Tests now only take a few seconds to complete due to removal of sleep that pretended to synchronize several threads.

Original issue description:

Currently async handler will flush but also will discard on close. 
These parameters need to be controlled during handler instantiation to make sure that no logs are lost.

https://github.com/fluent/fluent-logger-python/blob/master/fluent/asyncsender.py#L76
@arcivanov arcivanov changed the title Async should support different closure modes Massive threading and concurrency issues in async Dec 7, 2017
@arcivanov arcivanov mentioned this issue Dec 7, 2017
Closed
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 7, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
b8216fc 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 7, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
8012768 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 7, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
c42c480 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 7, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
aeb84fd 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 7, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
6f162c4 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 8, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
a72b49f 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 8, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
c6ae575 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 8, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
5f3b462 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 8, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
2b1dab4 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 8, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
65e05b9 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
arcivanov added a commit to arcivanov/fluent-logger-python that referenced this issue Dec 8, 2017
@arcivanov
Almost complete rewrite of async sender and async handler
472ab00 
Queue timeout removed as it served no purpose other than hide multiple threading issues
asctime format was non-functional
Many tests would silently fail and appear successful
Tests now are near-instantaneous due to removal of sleep

fixes fluent#105, fixes fluent#106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@arcivanov