calling Load(config) from modules

[aep]

the website mentions dynamic configuration but i haven't found any plugin that dynamically loads configs, and no documentation.

Is it ok to call Load() from a module?

caddy/caddy.go

Line 102 in e7457b4

func Load(cfgJSON []byte, forceReload bool) error {

how do we avoid the loop when caddy instantiates the module after reloading the config?

[francislavoie]

You're right, we haven't really documented how to use dynamic config loading. There's an explanation in the PR where it was introduced though, see the 3rd part #3994

[aep]

thanks. the linked PR doesnt explain if Load can be called at any point tho, as it is going to happen with a dynamic config.
Instead it seems to be intended for static configs loaded from a remote location that only gets loaded once.

[francislavoie]

Right, the other part is #4246 which added periodically pulling the config from the loader. So you could implement your own loader (a module in the namespace caddy.config_loaders).

[aep]

which would only allow polling instead of loading it on change tho.
i could poll every 1ms i guess, but then again i might as well just call the http/admin endpoint from itself, which seems to support reloading on demand.

[francislavoie]

but then again i might as well just call the http/admin endpoint from itself

The issue with that is the risk of deadlocks, cause the thing making the request will try to get shut down by the config loader which is swapping out the old config with the new, but if it can't shut down the old config (because there's still a pending request), it'll never be able to load the new config.

But yeah, I'd suggest configuring some low polling rate (50ms or 100ms should be more than frequent enough) and do a quick check to see if you should attempt to load a new one or not.

Noticed an issue with that idea though, /cc @mholt, the ConfigLoader interface returns ([]byte, error), so either you give a config that it'll attempt to load again, or it'll print an error to the logs. I don't really like this. I think it should be something like ([]byte, bool, error) so we can return false to skip loading (as in, loading is unnecessary at this time) instead of returning an error which would flood the logs if the interval is short. That feature is still marked "experimental", so we can probably make that change now.

[mholt]

@francislavoie How about instead we allow returning nil, nil to indicate that there's no new config to load.

Would this be helpful / solve the problem?

[aep]

ah, hm this is awkward. I found some other issues with async loading but don't remember since we ended up not using Caddy.

I think we should just close this.