kombu icon indicating copy to clipboard operation
kombu copied to clipboard

Solve Kombu filesystem transport not thread safe

Open karajan1001 opened this issue 3 years ago • 4 comments

partly fix: #398

  1. add an exclusive lock during the whole exchange file update.
  2. add some unit tests for the file lock

This time I tested it on my local machine, which works well with 20 workers.

I'm struggling with how to test the lock status during the progress, I tried to mock some of the functions kombu.utils.encoding.str_to_bytes want to inject a lock operation into it. But what I found is that the actual running function was not the same one I mocked. ( I print the function info in the _queue_bind, it is not modified. but if I import the function in test_filesystem.py and print it out, it will show a mocked function. Any ideas on this?

karajan1001 avatar Sep 12 '22 14:09 karajan1001

This pull request fixes 1 alert when merging c39c07e0a6291734e38677e579c7e9eabf729e95 into afcde0a0bd6fc7554146dc8f177702a723043516 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Sep 12 '22 14:09 lgtm-com[bot]

This pull request fixes 1 alert when merging d058796b8ac46b0605204eaf0f3bfcf53b2eba92 into afcde0a0bd6fc7554146dc8f177702a723043516 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

lgtm-com[bot] avatar Sep 13 '22 10:09 lgtm-com[bot]

how is the progress?

auvipy avatar Sep 18 '22 17:09 auvipy

how is the progress?

The current situation is that I can verify this PR on my local computer ( really easy). And the demo is below

asciicast

We can see that in the current master if I start 10 workers together, 5 of them will error out. After switching to the fix_398 branch, all of the 10 workers are started.

But the current difficulty is that I'm not sure how to write a test for this. The previous problem raises from that between the reading and writing progress there is no file lock between them and will probably got a content change when we want to write to it(the current status had already been different from when we read it). A rigorous test should be something like testing the status between this two progress. I tried to mock some of the functions inside this progress. But I found some weird behavior when using celery together with mock, I successfully mocked a function but when celery runs it, it still uses the old real one.

So in one word, the current status is that PR is complete, and can be verified in the demo, the test is also here but might lack rigor.

karajan1001 avatar Sep 19 '22 06:09 karajan1001