-
Notifications
You must be signed in to change notification settings - Fork 80
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
Use semaphore for queue lock #115
Conversation
jkemp101
commented
Sep 27, 2018
•
edited
Loading
edited
- Add tests for max workers > 1
- Test Redis impact with large number of locked queues, large number of workers, and new tasks being queued to locked queues
|
||
self.redis.zrem(self.name, self.id) | ||
|
||
def acquire(self): |
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.
We could add an optional def acquire(self, time=time.time):
parameter, pass time explicitly in tests and avoid freezefrog dependency.
tasktiger/__init__.py
Outdated
# The maximum number of workers that will be allowed to actively | ||
# process tasks for any queue. Similar to Single Worker Queues | ||
# but allows specifying any number 1 or greater. | ||
'MAX_WORKERS_PER_QUEUE': None, |
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.
Should there be an option to configure this per queue, and then deprecate SINGLE_WORKER_QUEUES
? E.g.:
MAX_WORKERS_PER_QUEUE = {
None: 4, # default
'some-queue': 1,
'some-queue.subqueue': 2,
'other-queue': 6,
}
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.
I'm a bit torn on how we should handle queue configuration as we add more and more queue specific options. Right now a queue's configuration is spread across multiple keys in the config. Do you think we should consider moving to something like this instead and stop using ONLY_QUEUES
, BATCH_QUEUES
, SINGLE_WORKER_QUEUES
/MAX_WORKERS_PER_QUEUE
?
QUEUES = {
'a': {
max_workers: 5,
batch: 50
},
'b': {}
}
We could also make it smart enough to detect if ONLY_QUEUES
is a list or a dictionary so we don't have to add QUEUES
to the config.
If we wanted to make that change I could remove MAX_WORKERS_PER_QUEUE
for now and just support it as a command line option until we refactor the config handling.
tasktiger/__init__.py
Outdated
@@ -323,6 +331,25 @@ def delay(self, func, args=None, kwargs=None, queue=None, | |||
|
|||
return task | |||
|
|||
def queue_system_lock(self, queue, timeout): |
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.
Is this used anywhere?