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.

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

RabbitMQ priority support #2635

Closed
KimiNewt opened this issue May 31, 2015 · 16 comments
Closed

RabbitMQ priority support #2635

KimiNewt opened this issue May 31, 2015 · 16 comments

Comments

@KimiNewt
Copy link

As of RabbitMQ 3.5, RabbitMQ has priority support: https://www.rabbitmq.com/priority.html

As such, can it be added to Celery? Doesn't it just need to have the priority field (which is already in Task) passed to RabbitMQ?

@ask
Copy link
Contributor

ask commented Jun 1, 2015

The AMQP priority property is already sent with the message

@thedrow
Copy link
Member

thedrow commented Jun 1, 2015

So this is implemented right?
I think it's safe to close this issue.

@chripede
Copy link

chripede commented Jun 3, 2015

I manually added a priority queue to RabbitMQ. When trying to push tasks to it, Celery fails with

amqp.exceptions.PreconditionFailed: Queue.declare: (406) PRECONDITION_FAILED - inequivalent arg 'x-max-priority' for queue 'prio' in vhost 'api': received none but current is the value '1' of type 'long'

Seems like it fails to (re)declare the queue in Rabbit.

@thedrow
Copy link
Member

thedrow commented Jun 3, 2015

Could it be that RabbitMQ doesn't conform to the standard in that case?
@ask Do we have a test that covers RabbitMQ priority queues?

@ernestoalejo
Copy link

Documentation is clearly very sparse and outdated in this aspect, but this works correctly for me when the queue is not created previously:

CELERY_QUEUES = (
  Queue('default', Exchange('default'), routing_key='default', queue_arguments={'x-max-priority': 8}),
)

If you want to apply a more strict priority to items probably prefetching should also be disabled:

CELERY_ACKS_LATE = True
CELERYD_PREFETCH_MULTIPLIER = 1

Use priority when sending the task to the queue:

add_task.apply_async(args=(arg1, arg2, arg3), priority=6)

@meteozond
Copy link

@ernestoalejo It is not actually true, due to this issue RabbitMQ accepts and sets priority attribute but does not respect it during message processing.
More than that we've tested this solution with 3.6.1 and found that it doesn't affect real messages priority at all.

@ernestoalejo
Copy link

I received the tasks in the correct priority. I send ~100 of a lower priority and then two of a higher priority and they get processed in the desired order, also if I sent that tasks several times in a row (with different parameters). Retries were processed after the other ones because I subtract 1 to the priority. It is definitely working for me. I can see the priority on the management of RabbitMQ configured without any problem.

Maybe there's something in your test that didn't reproduced the correct settings? Did you tried with a queue that was already created?

For reference this is my full settings.py:

if DEBUG:
  TASKQUEUE_BACKOFF = 10
else:
  CELERYD_HIJACK_ROOT_LOGGER = False
  CELERY_DEFAULT_RATE_LIMIT = '60/m'
  TASKQUEUE_BACKOFF = 30

CELERYD_TASK_TIME_LIMIT = 60
CELERY_ACCEPT_CONTENT = ['json']
CELERY_TASK_SERIALIZER = 'json'
CELERY_ACKS_LATE = True
CELERYD_PREFETCH_MULTIPLIER = 1
CELERY_DEFAULT_QUEUE = 'default'
CELERY_QUEUES = (
  Queue('default', Exchange('default'), routing_key='default', queue_arguments={'x-max-priority': 8}),
)
CELERYD_MAX_TASKS_PER_CHILD = 500
CELERY_ENABLE_REMOTE_CONTROL = False

celery.py

app = Celery('tasks', broker='amqp://guest@XXX-taskqueue//')
app.config_from_object('XXX.settings')
app.autodiscover_tasks(lambda: settings.INSTALLED_APPS)

The file setting the task:

foo_task.apply_async(args=(arg1, arg2), priority=7)

And my tasks:

@shared_task(max_retries=10)
def foo_task(arg1, arg2):
  try:
    _safe_foo_task(arg1, arg2)
  except Exception, err:
    logger.error('Task Exception: %s', str(err))
    traceback.print_exc()
    foo_task.retry(exc=err, countdown=settings.TASKQUEUE_BACKOFF, priority=6)

@meteozond
Copy link

We've tested two configurations:

1 Two different priority queues

CELERY_ACKS_LATE = True
CELERYD_PREFETCH_MULTIPLIER = 1
CELERY_DEFAULT_QUEUE = 'default'
CELERY_QUEUES = (
    Queue('important', Exchange('important'), routing_key='important', queue_arguments={'x-max-priority': 100}),
    Queue('default', Exchange('default'), routing_key='default', queue_arguments={'x-max-priority': 1}),
)

With

[(wait.apply_async(args=['default_task'], retry=False),wait.apply_async(args=['important_task'], retry=False, queue='important'))  for i in range(10)]

2 Different priority tasks inside one queue

[(wait.apply_async(args=['default'], retry=False, priority=1),wait.apply_async(args=['important'], retry=False,  priority=100))  for i in range(10)]

And consumer

celery worker -Q default,important

In first case it takes tasks nearly one by one from both queues.
In second it takes all tasks due their insertion order. Manual task fetching shows correct priority header, but as I've mentioned above it wasn't taken into account by Rabbit.

@ernestoalejo Don't you sending different priority task into the different queues? if so you will get 2th "high-priority" task sooner than 100th "low-priority".
What version do you use? Don't you use ant additional plugins?

Did you checked link, I provided? It's quite strange that you managed to get working feature, that developer doesn't.

@ernestoalejo
Copy link

The link is from 2014, maybe priority queues support was added later; it's documented now anyway: https://www.rabbitmq.com/priority.html. I'm using the standard "rabbitmq:3.6-management" image in Docker.

I considered priority between queues undefined behaviour so I went directly for a single queue (your second example). Note however that you have to declare the maximum priority in the correct queue. In your example:

CELERY_QUEUES = (
    Queue('important', Exchange('important'), routing_key='important', queue_arguments={'x-max-priority': 100}),
    Queue('default', Exchange('default'), routing_key='default', queue_arguments={'x-max-priority': 1}),
)

has to be changed to:

CELERY_QUEUES = (
    Queue('default', Exchange('default'), routing_key='default', queue_arguments={'x-max-priority': 100}),
)

if you want to send tasks with a priority of 100. The command will be celery worker -Q default instead.

Also note the warning in the documentation about hungry consumers. If the queue doesn't have a chance to see the full list it can't order the messages. If it's a test task without work add a sleep of one second or something to slow it artificially to see the effect of the priority queue.

@meteozond
Copy link

You are my personal hero, @ernestoalejo - you are right. Looks like per task priority in with queue x-max-priority really makes this magic work, thank you.

@thedrow
Copy link
Member

thedrow commented Apr 14, 2016

Should we change this into a documentation issue then?

@ask
Copy link
Contributor

ask commented Jun 24, 2016

Yes, we should make sure the documentation does not say this is unsupported anymore.

@vijeth-aradhya
Copy link

vijeth-aradhya commented May 16, 2017

Hey! I have just started using Celery and I'm trying to implement a sample application to understand prioritized tasking (with one queue) so that it will help a lot of other people as well 😄

I have currently added this code block provided here in the celery docs for prioritized tasking.

It is a very simple config for celery, and I'm thinking I might be missing something very trivial as the tasks are just executed in the received order.

Kindly have a quick look and let me know what the problem might be!
Thanks! @ask @ernestoalejo

celery==4.0.2
RabbitMQ ,"3.5.7"

@ernestoalejo
Copy link

@vijeth-aradhya It's better to open a Stack Overflow question to avoid hitting this bug now that is closed.

Seeing your code maybe the queue has no chance to prioritize the messages. Try with these two settings (adapt to your project as needed):

CELERY_ACKS_LATE = True
CELERYD_PREFETCH_MULTIPLIER = 1

Prefetch multiplier is 4 by default, maybe your tasks are downloaded before the queue can sort them.

@vijeth-aradhya
Copy link

@ernestoalejo Special thanks for clearing this out so quickly!

Yes, I will put it up in Stack Overflow henceforth. I commented here as the thread was very related to my query.

My apologies for commenting on a closed issue! 😃

@WangGeng77
Copy link

@ernestoalejo Many thanks for the answer! It is really helpful!
But I can not understand why we should set CELERY_ACKS_LATE to True. I have read the document of Celery and it says that this param is used when man needs a retry process

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants