tleyden / open-ocr

Run your own OCR-as-a-Service using Tesseract and Docker
Apache License 2.0
1.33k stars 223 forks source link

Rabbit abuse #3

Closed tleyden closed 10 years ago

tleyden commented 10 years ago

One picture worth a thousand words, most of which start w/ the letter F.

http://tleyden-misc.s3.amazonaws.com/blog_images/rabbit_mq.png

borjaburgos commented 10 years ago

Is open-ocr creating a new queue for each request instead of reusing queues? or something else going on here that I'm missing?

carlhoerberg commented 10 years ago

temporary queues are declared as durable and not with auto_delete, you probably want it to be the opposite.

carlhoerberg commented 10 years ago

The problem lies here: https://github.com/tleyden/open-ocr/blob/master/ocr_rpc_client.go#L129 you probably want it to be durable: false, exclusive: true and auto_delete: true

also you don't have to bind the temporary queue to anything, instead you should set the "replyTo" proeprty of the RPC request publish to the queue name, and let the RPC server publish directly to that queue.

tleyden commented 10 years ago

The problem lies here: https://github.com/tleyden/open-ocr/blob/master/ocr_rpc_client.go#L129 you probably want it to be durable: false, exclusive: true and auto_delete: true

OK I made that change. But I noticed in the docs

Non-Durable and Auto-Deleted queues will not be redeclared on server restart and will be deleted by the server after a short time when the last consumer is canceled or the last consumer's channel is closed. Queues with this lifetime can also be deleted normally with QueueDelete. These durable queues can only be bound to non-durable exchanges.

It's currently using a non-durable queue with a durable exchange. Should I change the exchange to non-durable?

tleyden commented 10 years ago

also you don't have to bind the temporary queue to anything, instead you should set the "replyTo" proeprty of the RPC request publish to the queue name, and let the RPC server publish directly to that queue.

So just to confirm:

ReplyTo:         c.rabbitConfig.CallbackRoutingKey,

should become:

ReplyTo:         callbackQueue.Name,

where callbackQueue is the result of calling QueueDeclare? (and there's no longer a need to call QueueBind with that callbackQueue)

carlhoerberg commented 10 years ago

correct! :)

On Tuesday 3 June 2014 at 01:18, Traun Leyden wrote:

also you don't have to bind the temporary queue to anything, instead you should set the "replyTo" proeprty of the RPC request publish to the queue name, and let the RPC server publish directly to that queue.

So just to confirm: ReplyTo: c.rabbitConfig.CallbackRoutingKey,
should become: ReplyTo: callbackQueue.Name,
where callbackQueue is the result of calling QueueDeclare? (and there's no longer a need to call QueueBind with that callbackQueue)

— Reply to this email directly or view it on GitHub (https://github.com/tleyden/open-ocr/issues/3#issuecomment-44865502).

tleyden commented 10 years ago

Further local testing (being nice to cloudamqp.com's servers) is showing a connection leak whenever the RPC client request times out because workers don't respond fast enough.

tleyden commented 10 years ago

@carlhoerberg thanks again for your feedback! I'm running a stress test now, and things are looking great. No connection or queue resource leakage.