ConfigSource run in the async mode

[vsevel]

Description

Recently the vault config source has moved to vertx (instead of okhttp client).
The vault config source has the ability to refresh itself periodically.
We had issue #16021 where properties were attempted to be refreshed in a non blocking thread.
Although calls to vault using the vertx client could return Uni, we are limited by the ConfigSource interface that requires a plain Map<String, String>.
The question here is whether the config source mechanism could be reactive friendly.

An alternative idea is to get a global timer responsible for calling periodically config sources and get them a chance to refresh their cache (if they do implement one). that way we could be blocking on this thread, and if we use non blocking data structures for the different caches, the applicative threads would never be blocked, and never attempt to call vault as a result of getting a config source property injected.

/cc @radcortez @sberyozkin

[sberyozkin]

Timer with a configurable poll interval is in interesting option IMHO, as making ConfigSource reactive may be tricky

[sberyozkin]

@vsevel Hi Vincent, by the way, can VaultConfigSource have its own timer ?

[vsevel]

@sberyozkin hello Sergey, it could, but I would rather not burn a thread just for the vault config source cache refresh. it would be better if there was a central pool in quarkus for those kind of scheduled tasks.

[radcortez]

I'm not opposed to provide a way for Config to be more reactive friendly.

It is unlikely to see such feature in MP Config directly (at least for the foreseeable future) so we would need to provide our own interface of ConfigSource to extend the API. We already have https://github.com/smallrye/smallrye-config/blob/8f45017cea7210410f01cba4f5d8f7f7dc65dd1b/implementation/src/main/java/io/smallrye/config/ConfigValueConfigSource.java#L23 in SR Config, so maybe we can do some experimental work over there.

Or most likely add a different ConfigSource like ReactiveConfigSource as a separate module.

[sberyozkin]

@vsevel Sure, what can be difficult is for this centralized timer task to decide which sources to refresh. @loicmathieu mentioned an idea of a Watch in the forum.
The individual custom source can run the task on the Blocking executor provided by Quarkus - I guess it would be used in any case...

[vsevel]

@sberyozkin I was thinking more about a central timer facility, where you could register your periodic tasks. the timer facility would be general purpose, not dedicated to config sources. I am surprised actually that none of the existing extensions have already expressed the need. the use case does not exist, or everybody is doing its own ScheduledExecutorService ?

[geoand]

Is this still relevant?

[vsevel]

I still believe it would be nice if the configuration module provided an option to refresh config sources. what do you think @radcortez @kdubb ?

[kdubb]

Yes! There are a couple of things in our Vault that refresh and are wired via the Vault ConfigSource.

Maybe we could do it in the Vault extension currently, without starting a thread, if we could count on injecting a ManagedExecutor? Does that required the context-propagation extension to be added as well?

[vsevel]

it applies to vault, but is not specific to vault. if there is no consensus to make it a global feature, then I agree we should do something inside vault.

[radcortez]

I still believe it would be nice if the configuration module provided an option to refresh config sources. what do you think @radcortez @kdubb ?

This has been a recurrent topic (about configuration refresh), and we usually advise not to do it, simply because at the moment there is no way to tell which configuration can be safely reloaded or not, or if it has any effect at all. For instance you cannot expect Hibernate to change its configuration mid-flight.

An alternative could be to state which namespaces support reloading. We would set quarkus namespace to no reload, but users would be free to manage their own namespaces.

[geoand]

What's the status of this one?

[radcortez]

Supporting configuration reloading out of the box shouldn't be done blindly for numerous reasons. Once you open that door, it will create a set of expectations that we cannot fulfill.

In this case, it is not only about ConfigSource reloading. Mappings and Roots are also cached, so even if the ConfigSource is reloaded, values will not be visible in these instances. Even if we also add reloading support for config objects, it would create many challenges:

• What if the configuration is invalid?
• Atomic reloads
• Track instances and reload automatically?
• ...

Yes, these can be solved, but getting it right requires an incredible amount of work, and I have reservations about whether it is worth it.

Regardless, I'm happy to keep discussing it :)