Ghost callbacks spam the retry set?

[geeksam]

Ghost callbacks spam the retry set?

[mperham]

Is Redis set to use noeviction or another maxmemory policy?

[geeksam]

redis.info["maxmemory_policy"] # => "volatile-lru"

[mperham]

Oh dear, that’s half of your problem. Redis gets under memory pressure and starts evicting data to free up RAM. You lose data and errors like these are symptomatic of that loss. See the Using Redis wiki page.

[geeksam]

"Great! Now for the other half of the problem." -Zeno

[geeksam]

Seriously, though... we'll change the policy, which sounds like it should eliminate the specific behavior observed. I'll give the wiki page a more careful read-through.

Is there any other diagnostic info you need and/or any other action [not detailed in the wiki page, which I'll review] that we should take? (Guessing not, but you said "half of [our] problem", so I'd like to be thorough.) :) I'll take "closed without comment" as a "no." :)

[mperham]

Sorry, just a poor turn of phrase. Sidekiq expects Redis to use noeviction otherwise data can be randomly evicted and inconsistent. If that happens, I don't make any claims for how Sidekiq may behave. Push back on me if you think there is still room for improvement. Should the batch code silently swallow missing callback data?

[geeksam]

Disclaiming support for a dysfunctional eviction policy seems perfectly reasonable. The job raising an error is also perfectly reasonable. The point at which (IMO) it went pear-shaped, though, was when those errors went through the same retry process as anything else. Given the scale that Sidekiq can allow users to achieve, things can go very wrong very fast.

The problem I had with this error, once I noticed it, was that—unlike an error in one of my app's jobs—I didn't see any obvious way to fix it. All I could do was decide that the jobs weren't coming back, pop a rails console into prod, and manually delete those jobs from the retry set so they didn't keep doing a "Sorceror's Apprentice" number on our Sentry quota.

Silently swallowing a Sidekiq::Batch::NoSuchBatch probably isn't the right behavior, but I don't think the standard retry policy is the way to go either. Is it likely that a missing batch will come back? If not, raise once [see GIF below] and be done, or maybe put it into a dead letter queue for manual disposition? Or, maybe the first few get retried, but as [internal] error rates go up, a Circuit Breaker shunts them into a DLQ instead of back into the retry set?

[geeksam]

Oh, one other easy potential intervention: mention a potential cause of the exception in the error message? I did search for it at the time (and checked again just now) but didn't find anything that would've pointed me in the right direction.

[mperham]

I've added code comments to those two places in the batch code where data inconsistency rears its head, explaining about noeviction, which should make it much easier to fix once you've opened up the code to see the problem area.

I don't know how I can safely write another retry path for bad batch data. It seems like a bad time, testing would be difficult, that code path would be used rarely so a high likelihood for hiding bugs. I'm going to stop here; open a new issue if you have other ideas for improvement.

[geeksam]

Entirely reasonable, given the low probability and only moderate severity. Thanks!