feat(config): Prepare configuration to be hot reloadable - #6346
Conversation
loewenheim
left a comment
There was a problem hiding this comment.
LGTM in the sense that I'm sure it doesn't change any current behavior, but I confess I haven't got a very good mental model of when/where a config snapshot should be taken and where it should be passed down to a subsequent function. In other words, where's the boundary from which a config should be immutable?
Basically the moment all overrides are applied right now. Later once we have hot reloads, the config stays mutable, it may be reloaded at any point.
Rule of thumb: Store
|
Most of the changes here are because of the introduction of the
ConfigSnapshotwhich is obtained from theConfig.A lot of the code now requires a snapshot instead of the config. Most actual changes are contained to
relay-config/src/config.rs.This also includes a change to actually start tracking all the dependencies of the config, which is still unused yet, but is a preparation for making the config hot reloadable. I started with this before I realized how many other changes this will require.
Also happened to see some leftovers in the config, which I cleaned up (e.g. cardinality limiting config).
There are still a few rough corners I want to tackle:
ServiceStatewhich I think should be something likeCurrentServiceStatewith a snapshot of the config.ProjectCacheServicegrabs a bunch of values on init and never refreshes them. This could be all made reload aware.UpstreamProjectSourceServicealso caches a few values that could be made reload-able.UploadServicealso caches some values on startup.HttpServerprobably should own a snapshot to make it explicit that nothing can be reloaded here.relay-configand also be validated on reload.And of course the actual reloading still needs to be implemented.
There should be absolutely no behavior change.
Closes: INGEST-1159