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

Retry Handler #73

Merged
merged 22 commits into from Nov 9, 2014

Conversation

@justinmills
Copy link
Contributor

commented Nov 7, 2014

This takes the work started by imothee and adds support for a queue specific retry (rather than routing the message back through the "main" exchange).

imothee and others added some commits Mar 13, 2014

Changes to queue to pass opts into handlers, worker to use hdr not hd…
…r.delivery_tag, max_retry_handler to create and bind required queues.
Justin Mills
Maxretry Handler
This was originally started here:
https://github.com/otherlevels/sneakers/tree/maxretry and discussed as
a sneakers issue here: #18. I
took this, updated the rest of sneakers to work with it and wrote tests
of both Handlers to ensure things are still working as expected. NOTE:
This is a breaking change in that any Handlers out there will no longer
work as the interface has changed. I expect there may be one more
interface-changing commit to follow on to this depending on how to
tackle the metrics issue.
Justin Mills
Make Maxretry Handler only retry per-queue.
This required a bit of a more complicated setup, but before this change,
the retry was retrying failures to the exchange which meant if you had a
fanout exchange, every queue would see the retry, not just the queue
that the failure occurred upon. To fix this, I created another exchange
and bound the queue you're retrying on to this one (in addition to the
exchange it was currently bound to). This allows us to put the retry
dead lettering to it and it will ONLY send the message to the queue
you're attempting retry on. Updated the example to demonstrate this more
clearly.

As part of this I made a couple of changes:
- Add another argument to handler constructor
- Use worker config when creating the handler.
- Fix bug in worker where it didn't respect incoming options. This
  didn't look to be used anywhere.
Justin Mills
Merge pull request #2 from Yesware/yw-make-maxretry-per-queue
Make Maxretry Handler only retry per-queue.
Justin Mills
Log backtraces if present when jobs fail.
This is already done on sneakers master, but by logging each line of the
backtrace on a separate line. While this looks nice, it makes it very
hard for log tools to get the entire backtrace by searching for lines
that contain "ERROR" for instance. I'll probably merge sneakers master
into our fork and undo that change so we are all sync'd up again.
Justin Mills
Merge branch 'master' into merge-upstream-master
Conflicts:
	lib/sneakers/queue.rb
Justin Mills
Merge upstream master and cleanup backtrace logging
Undo the backtrace logging how it was done and do it on a single line.
Also include some more information so it's easier to find all timeouts,
all unexpected errors, etc.
Justin Mills
Merge branch 'yw-master' into merge-upstream-master
Conflicts:
	lib/sneakers/worker.rb
Justin Mills
Improve logging in Maxretry handler and fix fallthrough metric in worker
We needed some more insight into when a worker was retrying a job. Would
be nice to have stats too, but that really requires that these handlers
also have access to the worker. I think eventually we'll replace the
exchange and queue with the worker as an argument. Until then, this
logging should give us enough information to know how often it's
happening and possibly why.

Also the fallthrough case is noop when Sneakers cannot interpret the
response. The metric being collected was retry which was wrong as the
handler's retry method was not called and instead the noop method was.
Better to keep these two in line.
Justin Mills
Merge pull request #5 from Yesware/improve-logging-in-maxretry-handler
Improve logging in Maxretry handler and fix fallthrough metric in worker
Justin Mills
Reject a JSON payload with error details
When the retry worker runs out of retries and sends a message on to the
error queue, capture as much as we can so you have some hope of
understanding what's going on. This will require a more complicated
replay than simply moving them off this queue onto another exchange, but
that seems reasonable.
Justin Mills
Merge pull request #6 from Yesware/reject-json-with-details
Reject a JSON payload with error details
Larry Kang
Merge pull request #7 from Yesware/option_for_nondurable_queue
Add option :queue_durable to allow creation of a non-durable queue
Justin Mills
Merge branch 'master' into merge-upstream
Conflicts:
	lib/sneakers/queue.rb
	lib/sneakers/worker.rb
	spec/sneakers/worker_spec.rb
@jondot

This comment has been minimized.

Copy link
Owner

commented Nov 9, 2014

This is a ton of awesome work from awesome people! excellent!! :)

jondot added a commit that referenced this pull request Nov 9, 2014

@jondot jondot merged commit 632e982 into jondot:master Nov 9, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.