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

automatically load config source and mark it dynamic or not #41

Closed
Emily-Jiang opened this issue Jan 27, 2017 · 7 comments
Closed

automatically load config source and mark it dynamic or not #41

Emily-Jiang opened this issue Jan 27, 2017 · 7 comments

Comments

@Emily-Jiang
Copy link
Member

Automatically load config sources and annotate it whether dynamic or not

@struberg
Copy link
Contributor

Can you explain what you mean with that?

@Emily-Jiang
Copy link
Member Author

I would like to add one more method to ConfigSource /**
*
* @return the refresh interval in milliseconds
*/
long getRefreshInterval();
Any number greater than 0 will mark the config source as dynamic.

@struberg
Copy link
Contributor

I made the experience that refreshing the whole config is not that optimal in practice.
You often need to specifcy different reload times depending on each key separately.
Imo the Config itself should not do any auto-refresh but I'd defer this to the ConfigValue

@OndroMih
Copy link
Contributor

I agree with @struberg - config sources should be rather dumb and provide enough metadata for higher levels to use them effectively. Moreover, a hint that a config source is dynamic is not useful if there is not method to reload values. I don't see how a method like getRefreshInterval() could even be used with the current API.

I'd rather explore ways of caching/prefetching config values effectively, adding enough hints from the config source to make it effective. Among useful hints would be whether config values might change during runtime (static vs. dynamic), or how expensive it is to read/reload values, e.g. in-memory, local, remote (local network), slow-remote (internet).

Maybe @Emily-Jiang meant the method getRefreshInterval() to be used for this purpose, in that case I would change its name, e.g. getRefreshLatency(). A higher-level mechanism could then assume cache-retention periods from this number. But it wouldn't be easy to get this relative value always correct (hard to prevent people from giving latency 1000 to fast sources, while others giving latency 100 to really slow sources).

@Emily-Jiang
Copy link
Member Author

We need to differentiate between dynamic sources and static sources. For static sources, we don't need to go back to reload the config properties. For dynamic sources, we need to go back to pull the config changes after a period of interval.

The problem with baking this time delay on the ConfigValue is that you will have no clue where the config property is from. It might be from a slow database or might from a fast connection. How much time delay would you want to specify per config basis? Secondly, how do you know a particular property being resolved is from a static or dynamic config source? If the config value is from a static source, it is pointless to void cache unnecessarily.

My approach is from the source itself as we know for sure whether it is dynamic or static. The config object is always up to date.

@struberg
Copy link
Contributor

struberg commented Mar 13, 2017

I don't yet understand why there is a difference from the user pov. Whether a ConfigSource does reload it's values each time, caches it for a Minute or even only loads it at startup is a question of each ConfigSource itself.

The problem with baking this time delay on the ConfigValue is that you
will have no clue where the config property is from.

And the 'user' part also should not need to care! It only knows it's own requirements.

Secondly, how do you know a particular property being resolved is
from a static or dynamic config source?

That imo doesn't matter at all. A properties file inside a jar obviously cannot change without re-packaging the jar file. That will not likely happen during runtime. Otoh a DataBaseConfigSource might bulk-reload all it's values from the DB every 5 seconds (most probably even in the background in a parallel thread).

If the config value is from a static source, it is pointless to void cache unnecessarily.

No. Think about config.getValue("myproject.archive.url"). We assume it gets picked up from some property file in a jar of your application. Of course this property file wont ever change. BUT: Some from the Ops team might just add the key to the DataBaseConfigSource (with higher ordinal) because he wants to change the url during runtime. You see?
Just because a value gets currently served from a static ConfigSource doesn't mean that it cannot get changed from a user point of view.

Emily-Jiang added a commit that referenced this issue Mar 15, 2017
Emily-Jiang added a commit that referenced this issue Mar 16, 2017
#41 Add default value in the ConfigProperty
@Emily-Jiang
Copy link
Member Author

close this one as we have another issue to readdress this issue, just use 'push' style to push the changes into the config.

@dmlloyd dmlloyd added this to the MP Config future milestone Mar 4, 2020
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

No branches or pull requests

4 participants