Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move cache initialisation under supervision trees #291

Merged
merged 15 commits into from
Jul 27, 2024

Conversation

geekingfrog
Copy link
Collaborator

@geekingfrog geekingfrog commented May 22, 2024

This PR moves the caching logic into logically independent modules, and define supervision tree to start the caches and do initialisation at the same time.

The main benefit is defining a cache and warming it up is now in one place instead of spread across the application.ex and startup.ex files.

One thing to note: some caches are used together. For example config_site_type_store and :config_site_cache are both used by add_site_config_type. In this case, the restart strategy should be :one_for_all so that if a cache dies, the other is also terminated, and then both are restarted before initialisation.

This should be reviewed commit by commit where I tried to only move small bits at a time.

Ultimately I want to straighten up the startup logic which has a bunch of init code outside supervision trees, but that'll come later.

How to test that?

There shouldn't be any functional changes, so:

  • starting up
  • login as admin and doing a quick permission check (can you access protected parts?)
  • start up a lobby and issue a command like $help or $whois

@geekingfrog geekingfrog changed the title Init caches Move cache initialisation under supervision trees May 22, 2024
@geekingfrog geekingfrog marked this pull request as ready for review May 22, 2024 05:45
@StanczakDominik StanczakDominik added the needs testing Needs unit tests or testing on integration server label May 28, 2024
@Teifion
Copy link
Contributor

Teifion commented Jun 26, 2024

This is a really sensible idea; I'll be implementing it in my other projects. Thank you :)

@geekingfrog geekingfrog mentioned this pull request Jul 5, 2024
6 tasks
@geekingfrog geekingfrog force-pushed the init-caches branch 4 times, most recently from d217f45 to eb8717e Compare July 10, 2024 16:19
@L-e-x-o-n
Copy link
Collaborator

Works on integration server.

@L-e-x-o-n L-e-x-o-n merged commit 7f521fe into beyond-all-reason:master Jul 27, 2024
3 checks passed
@geekingfrog geekingfrog deleted the init-caches branch July 27, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs testing Needs unit tests or testing on integration server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants