Skip to content

feat(config): Prepare configuration to be hot reloadable - #6346

Merged
Dav1dde merged 11 commits into
masterfrom
dav1d/reload-config-1
Sep 3, 2026
Merged

feat(config): Prepare configuration to be hot reloadable #6346
Dav1dde merged 11 commits into
masterfrom
dav1d/reload-config-1

Conversation

@Dav1dde

@Dav1dde Dav1dde commented Sep 2, 2026

Copy link
Copy Markdown
Member

Most of the changes here are because of the introduction of the ConfigSnapshot which is obtained from the Config.

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:

  • Axum routes (size limits) are initialized from a snapshot and don't adjust dynamically. Other limits would.
  • Axum routes inject ServiceState which I think should be something like CurrentServiceState with a snapshot of the config.
  • ProjectCacheService grabs a bunch of values on init and never refreshes them. This could be all made reload aware.
  • UpstreamProjectSourceService also caches a few values that could be made reload-able.
  • UploadService also caches some values on startup.
  • HttpServer probably should own a snapshot to make it explicit that nothing can be reloaded here.
  • Config validations happening on startup probably should be moved into relay-config and 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

@Dav1dde Dav1dde self-assigned this Sep 2, 2026
@Dav1dde
Dav1dde requested a review from a team as a code owner September 2, 2026 11:45
cursor[bot]

This comment was marked as resolved.

Comment thread relay-config/src/config.rs
@Dav1dde
Dav1dde marked this pull request as draft September 2, 2026 11:47
@Dav1dde
Dav1dde marked this pull request as ready for review September 2, 2026 12:33
Comment thread relay-server/src/endpoints/minidump.rs

@loewenheim loewenheim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread relay-config/src/config.rs Outdated
Comment thread relay-config/src/config.rs
Comment thread relay-config/src/source.rs
@Dav1dde

Dav1dde commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

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.

[...] when/where a config snapshot should be taken and where it should be passed down to a subsequent function.

Rule of thumb: Store Arc<Config> and acquire ConfigSnapshot for every unit of work, examples:

  • For each message/batch a service processes
  • For each request being handled

Comment thread relay-server/src/endpoints/minidump.rs
@linear-code

linear-code Bot commented Sep 3, 2026

Copy link
Copy Markdown

INGEST-1159

@Dav1dde
Dav1dde added this pull request to the merge queue Sep 3, 2026
Merged via the queue into master with commit cff629e Sep 3, 2026
53 of 54 checks passed
@Dav1dde
Dav1dde deleted the dav1d/reload-config-1 branch September 3, 2026 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants