[feature] Provide hook to force refresh of system properties about config HOT 7 CLOSED

yeah. the problem is that any such hook will be unthreadsafe, and/or force synchronization where we currently have none, so it's tricky to figure out how to go about it. I think there might already be a super hacky private hook used in the tests for the lib itself but it has problems.

from config.

So are you saying that "yes this is useful but hard to implement" or "is't not worth implementing"? :-)

from config.

It's definitely useful, but there might be a tradeoff where it either slows down production code with a global lock or is unthreadsafe/dangerous, so it isn't a no-brainer definitely-good thing to add. I'm not sure exactly how bad the tradeoff is without digging in a bit to try to see how it might work.

Hmm. Looking at the code, getting the singleton is already synchronized (see LoaderCache.getOrElseUpdate in ConfigImpl.java). We drop the cache if the class loader changes. We could also drop the cache if the system properties config has been changed.

The method to reload the system properties config already exists in impl/ConfigImpl.java (reloadSystemPropertiesConfig) but it isn't threadsafe.

If SystemPropertiesHolder.systemProperties were volatile then I guess the worst case is that you get the old system properties, which may be "threadsafe enough" for this case.

So maybe it's just 1) in LoaderCache.getOrElseUpdate check for SystemPropertiesHolder.systemProperties changed in addition to class loader changed 2) put volatile on SystemPropertiesHolder.systemProperties 3) make a public wrapper for reloadSystemPropertiesConfig()

But there may be complications once we get into it.

from config.

I also need this feature

from config.

As long as in the end the returned Config() is immutable (and not refreshable, whatever that would mean), I think we’re fine. Creating new Config()s should be a rare operation, and if it needs synchronization that will not kill it. It should be lockless, though, because you wouldn’t think that reading system properties could block. As @havocp said, having one reader (who did not explicitly request a refresh) read “old” data is fine; so I think it would just need to be a volatile field somewhere and “last refresher wins” semantics.

from config.

I agree with @rkuhn, production code should be reloading config very, very infrequently. My own use-case is specifically motivated by unit tests only.

As for thread safety, System properties are inherently thread unsafe. You could add internal synchronization to guard against multiple threads reloading the config concurrently but with appropriate caveats in documentation, I think most callers should be OK without it.

from config.

In 0.6.0 just published (may take a bit to sync to maven)

from config.