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

feat: cache all metrics data for 10 seconds for performances reasons #690

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Psycojoker
Copy link
Collaborator

The metrics building ends up taking a lot of performance and we wanted to cache it to reducing the CCN load.

This cache all heavy operations for 10 seconds (arbitrary value to discuss).

Those decisions are based on this line_profiling of the function:

2025-01-24_03-42

(gosh it tooks so much time to get docker-compose is a PITA)

Related Clickup or Jira tickets : ALEPH-362

Self proofreading checklist

  • Is my code clear enough and well documented
  • Are my files well typed
  • New translations have been added or updated if new strings have been introduced in the frontend
  • Database migrations file are included
  • Are there enough tests
  • Documentation has been included (for new feature)

How to test

Just launch pyaleph and go on "http://localhost:4024/metrics.json"

Copy link
Member

@nesitor nesitor left a comment

Choose a reason for hiding this comment

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

I see so good the changes, but either we defined if before, I think that we need to test it on staging to ensure if 10 seconds is enough cache time, or we need to increment it before merging it.
Also, if you can, please add this cache setting on the API configuration to allow customizing from the config.yml file without need a deployment, something like a cache_ttl for example.

@Psycojoker
Copy link
Collaborator Author

Done.

I've completely changed the code because it didn't made that much sens to make it that complicated in the end and I've also made all cache TTL of this file configurable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants