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

Hijack self.backend instead of trying to find a cache #10

Closed
winhamwr opened this issue Aug 31, 2012 · 3 comments
Closed

Hijack self.backend instead of trying to find a cache #10

winhamwr opened this issue Aug 31, 2012 · 3 comments

Comments

@winhamwr
Copy link
Contributor

The only requirement for a cache backend that will work with Jobtastic is that it has an atomic increment operation. Redis (and probably others) have this, so there's no reason we should limit the options to Memcached only. Ideally, #8 will be completed first so that we don't have to do a bunch of implicit configuration parsing, but at worst, we can look for Redis the same way we look for memcached.

Maybe there's a way to make this a bit more flexible though. It seems that all cache backends for both Django and Werkzeug must provide an incr method, regardless of whether it's atomic. There seem to be two options then:

  • Make it a documentation thing and call out the fact that non-atomic incr backends will not always protect you from thundering herd issues. We can let people use non-atomic incr backends this way, and they'll probably still improve things a majority of the time.
  • Use some kind of backend whitelist (which is the way things are currently implemented) that knows that Redis and Memcached are atomic, while the others aren't.

Of these two options, it seems like providing the freedom to use non-atomic backends is preferable.

@winhamwr
Copy link
Contributor Author

New plan for getting the persistant backend outline here.

Unless Ask is really against it, we're going to directly hijack the task storage backend. That means that no configuration at all will be required for Jobtastic, and we can just piggyback off of their stuff. If we do need extra config in the future, we can probably figure out how to access self.app.conf and completely piggyback on Celery. Nothing will be framework specific, Hooray!

As far as the incr method, some backends support it and some don't. On task init (which only happens once), if a task is configured to using thundering herd avoidance and they're using a backend without incr support, we can do a warnings.warn.

It would probably also be good to look for the backends that don't support retrieving the result multiple times, since that would invalidate lots of things.

@rhunwicks
Copy link
Contributor

In the light of #37 I think that this issue should be Closed. I don't think the Celery Result Backend is suitable as-is because we can't control the timeout, and so the only sensible way to use it would be to wrap the backend client in a caching library, e.g. Werkzeug, which means us choosing one caching library over another. I think pluggable caches as proposed in #37 is a better approach.

@winhamwr
Copy link
Contributor Author

Agreed. #37 is a superior approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants