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

Throttle calls to a custom configuration loader #142

Merged
merged 1 commit into from
Mar 7, 2019

Conversation

joshuaflanagan
Copy link
Contributor

Many custom configuration loaders will retrieve the
configuration from an external resource. Resque::Pool
asks the loader for the latest configuration roughly
once per second. You may want to reduce load on your
external resource by caching the value, and only
really fetching the latest configuration after a
specific amount of time has passed.

Instead of forcing each configuration loader author
to re-write (and test) this logic, we provide Resque::Pool::ConfigThrottle.

See the spec for full details of its behavior.

Many custom configuration loaders will retrieve the
configuration from an external resource. `Resque::Pool`
asks the loader for the latest configuration roughly
once per second. You may want to reduce load on your
external resource by caching the value, and only
really fetching the latest configuration after a
specific amount of time has passed.

Instead of forcing each configuration loader author
to re-write (and test) this logic, we provide `Resque::Pool::ConfigThrottle`.

See the spec for full details of its behavior.
@nevans
Copy link
Collaborator

nevans commented Oct 28, 2015

This is great! This composability is exactly direction I was hoping to go with the config loader bits.

I just pushed an (unfinished work in progress) branch where I broke the default config loader down into Static, File, Memoizable, EnvironmentMerged, and then I moved forward from there to make an Overloaded and Redis loader (wrapping the whole thing in Throttled).


def call(env)
# We do not need to cache per `env`, since the value of `env` will not
# change during the life of the process.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment (and my own playing around with decomposing the default loader) makes me feel that sending in env as an argument to call is the wrong API. I'm currently thinking that environment should be passed in to the config loader's initialize, so we can codify the assumption that env will not change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came to the same conclusion. Not every config loader will care about the environment - in fact most probably won't.
I agree that any config loader that has a dependency on other information (redis connection, db connection, current environment, etc) should take that info via the constructor.

@nevans nevans mentioned this pull request Oct 28, 2015
@joshuaflanagan
Copy link
Contributor Author

Totally agree about the composability. The FileOrHashLoader was obviously awkward - glad you're breaking that up.

@nevans nevans mentioned this pull request Oct 28, 2015
@nevans
Copy link
Collaborator

nevans commented Oct 28, 2015

I don't have any more time to play with this today, but please see #143 (I renamed some things and changed your API a little bit), #145 (basic ConfigLoaders::Redis implementation), and the 4d8c5ae (the decomposed_default_loader commit) and let me know what you think.

@nevans nevans merged commit 9a3be9e into resque:master Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants