Add option to skip herd avoidance check, but still trigger avoidance for other tasks

[thenewguy]

I have a situation where I would like to use thundering herd avoidance conditionally.

It would be easy to support this with a conditional check at https://github.com/PolicyStat/jobtastic/blob/master/jobtastic/task.py#L242 and pop the key out of the options that get passed to the superclass.

Something like the following should do the trick:

        # Check for an in-progress equivalent task to avoid duplicating work
        if not options.pop('disable_thundering_herd_avoidance', False):
            task_id = cache.get('herd:%s' % cache_key)
            if task_id:
                logging.info('Found existing in-progress task: %s', task_id)
                return self.AsyncResult(task_id)

This would work nicely with #56

[thenewguy]

Let me know if this would be acceptable and I can work it into the implementation of #56

[winhamwr]

To make sure I understand, you're talking about adding the ability to "turn off" thundering herd avoidance at task "call time", rather than at task definition time, right?

I'm a bit wary of adding any special kwargs that jobtastic hijacks, since it introduces the potential of existing jobtastic usages colliding with those special kwargs. Even if we use a long namespace like jobtastic_herd_avoidance_timeout, it violates the "ideally, there should be exactly one way to do things" idea.

I think there's a way to achieve what you're looking for right now via subclassing:

class FooTask(JobtasticTask):
    significant_kwargs = [
        ('numerators', str),
        ('denominators', str),
    ]
    herd_avoidance_timeout = 60
    ...

class FooSansHerdAvoidanceTask(FooTask):
    herd_avoidance_timeout = 0

If that pattern doesn't actually work because of a Jobtastic implementation detail, then I'm super interested in making it work. It might also be a good use case to document.

What do you think of the subclassing option to solve this?

[thenewguy]

@winhamwr I didn't like that idea at first, but from the perspective of a reusable app I think you are definitely correct. That is the cleaner approach for extending celery tasks in a reusable app.

[thenewguy]

@winhamwr So I've been trying to figure out why the subclass approach isn't giving the results I was looking for written as suggested.

I think the boolean mentioned in the original code snippet is still required. It would just be a class attr instead of a method kwarg.

Here is why:
Depending on the condition, I need to be able to ensure that my task runs and is computed. In this case, I want to avoid skipping the task due to herd avoidance. However, I want other tasks to be able to latch on and use the forced run for herd avoidance. In order for this to happen I need to be able to disable herd avoidance without setting the herd_avoidance_timeout to 0. Otherwise, the forced run does not leave its footprint in the cache so the avoidable tasks cannot latch to it.

[thenewguy]

@winhamwr Do you have a suggestion for how to test this? I've submitted a PR that implements/documents the class attr bypass_herd_avoidance

[thenewguy]

err I didn't submit a PR I created a branch on my fork and pushed the changes

[winhamwr]

@thenewguy I meant to take a look at this today. Sorry for the delay. I plan on taking a close look, tomorrow.

[thenewguy]

@winhamwr Here is a direct link to the commit: thenewguy@410d735 I've been using it for awhile now and haven't noticed any issues

[thenewguy]

@winhamwr not trying to pester you... but just checking back in on this

[winhamwr]

@thenewguy thanks for your persistance! I appreciate it.

I just re-reviewed #58 and it's ready to go with a very minor documentation tweak.

As far as thenewguy/jobtastic@410d735, I think I now understand your use case based on rereading your March 16 comment. I'm trying to think of what name for the attr best-communicates that idea.

bypass_thundering_herd_avoidance makes me think that it would be avoided altogether, when in fact we're only ignoring checking herd avoidance, not setting herd avoidance. I'm struggling with an alternative name that makes that distinction clear, though.

• always_start_new_herd?
• herd_leader_only?

Since I can't think of anything that concisely explains the behavior, I lean towards something weird enough that motivates reading of the documentation. Thoughts?

[winhamwr]

Also, for this feature, we definitely need an example in the README of how it could be used.

So you're setting cache_prefix to the same thing for two different task classes and having one of them be the "always run, no matter what" and the other be the "only run if I'm the only one running"? Am I understanding things correctly?