-
-
Notifications
You must be signed in to change notification settings - Fork 294
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
fix #173 #244
base: master
Are you sure you want to change the base?
fix #173 #244
Conversation
The idea itself sounds good to me. |
5cc180d
to
f0df034
Compare
} else { | ||
// job is failed but we want to return it back into | ||
// the queue | ||
$this->redis->zadd("$this->channel.reserved", time(), $id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That cold lead to fail-add indefinite cycle. Should be limited number of re-adding the job back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, you are right, but I rely on a class/global defined number of attempts. In this case if all attempts have been used up, job would be marked as a handled and this condition will never be invoked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha.
@zhuravljov what do you think? |
Also, a few things are confusing me a bit in this PR:
Should I fix it? |
Any updates? |
https://github.com/yiisoft/yii2-redis/blob/master/src/Mutex.php#L113-L119 If lock cannot be set, redis mutex will try again after 1 second. 1 second is like ages in this context - I'm able to handle tens of jobs in this time. |
Implementation of the mutex can be changed via config. It's only default behavior. |
Right now instance of |
Yeah, right now However, you are right about the unfairly low delay of other actions. I could suggest to try to use same mutex for those actions and also, start a discussion in the main repo about decreasing delay of What do you think? |
But there is no valid reason for requiring |
Oh, I get it. |
would be a good idea to make the timeout a float in redis mutex to allow retry much faster. |
What about the solution such this? It's not finished yet, just to pic the idea of using
yii/redis/Mutex
to solve the concurrency issue.