Skip to content

perf: cache getConfig - #9377

Merged
mertalev merged 2 commits into
mainfrom
perf/cache-get-config
May 10, 2024
Merged

mertalev merged 2 commits into
mainfrom
perf/cache-get-config

Conversation

@mertalev

@mertalev mertalev commented May 10, 2024 •

Copy link
Copy Markdown
Member

Description

When not using a config file, we currently fetch the config on each call. This adds up to quite a few unnecessary queries since this method is called so frequently. Even when using a config file, we apply the same overrides and re-validate the same resulting object each time. This PR caches the config object, only updating it when a config update event is triggered.

How Has This Been Tested

Tested that jobs process as normal with and without a config file, that updating the config through the web UI updates the config object, and that multiple concurrent calls to getConfig result in only one query.

@jrasm91 jrasm91 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you test changing config at runtime and verifying the microservices uses the new config? Specifically for stuff like storage template migration and concurrency?

@mertalev

Copy link
Copy Markdown
Member Author

Yep. I ran thumbnail generation and changed the preview format midway. The success logs immediately started using the new format.

@danieldietzler danieldietzler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM :)

Comment thread server/src/interfaces/database.interface.ts Outdated
Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 10, 2024 •

Copy link
Copy Markdown

Deploying immich with  Cloudflare Pages  Cloudflare Pages

Latest commit: f68d5bc
Status: ✅  Deploy successful!
Preview URL: https://0671f7ec.immich.pages.dev
Branch Preview URL: https://perf-cache-get-config.immich.pages.dev

View logs

@mertalev
mertalev enabled auto-merge (squash) May 10, 2024 18:11
@mertalev
mertalev merged commit f3fbb9b into main May 10, 2024
@mertalev
mertalev deleted the perf/cache-get-config branch May 10, 2024 18:15
@jrasm91

jrasm91 commented May 10, 2024

Copy link
Copy Markdown
Member

Yep. I ran thumbnail generation and changed the preview format midway. The success logs immediately started using the new format.

Those don't use config$ though

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