Handling reliable_push buffer full

[stevenharman]

We are enabling reliable_push! and were digging into what exactly happens if we fill up the local, in-memory queue before a connection to Redis can be re-established. Right now, it looks like the job is dropped on the floor (expected) and an error level log line is emitted of the form [ReliablePush] Reached job backup limit, discarding job due to #{ex.class}: #{ex.message}.

We're wondering if you'd be up for adding instrumentation hooks around the various points of the Sidekiq::ReliableClient where we're unable to push a job to Redis. For example, maybe we'd like to log a little more information about the job so we can "re-create" it later, if needed? Or perhaps more likely, allow us to raise an error in the event the job could not be pushed onto the local queue?

[mperham]

I'm thinking it would be smarter to raise an error rather than dropping the job. This is the point of backpressure: once the local queue fills up, it should break rather than silently failing. Reliable push is designed for transient network hiccups, not a sustained outage. wdyt?

[mperham]

I'll be back toward the end of the month. It'll ship before Sept.

[stevenharman]

Thanks Mike! Enjoy the rest of your summer. 😄

[mperham]

Ok, it will now raise Sidekiq::ReliableClient::BufferFull if it tries to store too many client payloads. I'm not sure if this is a large enough change in behavior to warrant a version bump to 7.2.0 or if can be part of a patch release. It's an edge case of an edge case.

[stevenharman]

This is great Mike. Thank you.

A version bump or patch release… 🤷. Sidekiq is widely-used enough that this is going to break some expectation of someone, somewhere. I guess I'd say point release if this changes documented behavior, or a patch release otherwise.

[mperham]

Ok. I'm hoping to ship a patch release next week but I have a few other issues/PRs still pending.