search

tannerryan / ecpush

A Go library for subscribing to real-time meteorological data feeds from Environment Canada.
BSD 2-Clause "Simplified" License
2 stars 1 forks source link

queue declared with no expiry. #1

Closed petersilva closed 5 years ago

petersilva commented 5 years ago

Hi there, my team operates the dd.weather.gc.ca web site that ecpush pulls data from. Once in a while, we get pager calls where there are large queues with the name ecpush in them, but no-one consuming from them. It gets to the point where the server gets bogged down, and we need to have an analyst delete them. I think this is unintentional.

We would prefer if you would implement a 5 minute expire setting by default, which is suitable for trying things out, and have users specify a longer expiry when they really need it. If you´ve got a real business need, and you need to recover your queue after being down for several hours, then expiry of 1 day is fine, but please only do it if you really need it.

Thanks.

tannerryan commented 5 years ago

Hi Peter. Right now the queues are being declared as non-durable, non-auto delete, and non-exclusive. Although enabling auto-delete would solve the issue, it will clear the queue immediately after the connection is loss, resulting in a loss of all messages between the disconnection and reconnection.

Since "expires" is a broker specific AMQP feature, I just want to confirm that you would like an "expire=300" AMQP argument sent when declaring the queue? If so, I'll update the library immediately.

I'm actively using this for one of my projects, but I do know of others using this. Do you or one of your team members have access to the logs? I'd like to know if it's a known IP that has been creating the problematic number of queues. I can be reached at hello@tannerryan.ca.

petersilva commented 5 years ago

yes, autodelete is not appropriate if you want to tolerate transient failures. expires fits the bill much better.

I´m not sure how your library exposes the expiry setting... the one our implementations use is called "x-expires", and it is in milliseconds, so 300 would be a third of a second... likely want 3001000, but check what your API does, it might not expose it in native* units.

tannerryan commented 5 years ago

Hi Peter. The update has been performed in bd2f3d1aabe58b0eefc937bd2f2f6395574c5c5e. I tested with a local instance of RabbitMQ and the expires parameter is working as intended. If your team is utilizing RabbitMQ as the AMQP broker, this update will solve the problem.

The library is set to a static x-expires value of 5 minutes (in milliseconds). If anyone using this library requests a changeable expires value, I'll update the library to do so.

I actively use this library and I'm aware of a few others who are using it. I have since notified them of the update in the library. Hopefully they update their applications sooner than later. Sorry again!

petersilva commented 5 years ago

Looks good! We'll watch on our broker and see if any ecpush queues pile up again. Thanks!