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

[Merged by Bors] - cache poet's /v1/pow_params with TTL #6199

Closed
wants to merge 3 commits into from

Conversation

poszu
Copy link
Contributor

@poszu poszu commented Aug 1, 2024

Motivation

Similarly to /v1/info, we query /v1/pow_params very often (once per submit per node ID). As the contents returned from this endpoint change rarely (once per epoch), it makes sense to cache the result.

Description

  • factored out a small struct to encapsulate the caching + expiration logic into cachedData struct
  • implemented caching of /v1/pow_params same way as /v1/info is already done

Test Plan

added a test, will also test on testnet-13 where we observe lots of requests to /v1/pow_params on 1:n nodes.

TODO

  • Explain motivation or link existing issue(s)
  • Test changes and document test plan
  • Update documentation as needed
  • Update changelog as needed

Comment on lines 57 to +58
CertifierInfoCacheTTL: 5 * time.Minute,
PowParamsCacheTTL: 5 * time.Minute,
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to make this just one parameter instead of 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe 🤷. Giving extra flexibility doesn't hurt and it is useful in systests where certifier doesn't change at all (and can be cached for long time) but the pow_params change every epoch (1 minute)

Copy link

codecov bot commented Aug 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (13dfb49) to head (c70e4df).
Report is 4 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #6199     +/-   ##
=========================================
- Coverage     81.9%   81.8%   -0.1%     
=========================================
  Files          301     301             
  Lines        32406   32426     +20     
=========================================
+ Hits         26548   26555      +7     
- Misses        4135    4146     +11     
- Partials      1723    1725      +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@poszu poszu changed the title cache poet's /poet_params with TTL cache poet's /v1/pow_params with TTL Aug 1, 2024
@poszu
Copy link
Contributor Author

poszu commented Aug 1, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Aug 1, 2024
## Motivation

Similarly to /v1/info, we query /v1/pow_params very often (once per submit per node ID). As the contents returned from this endpoint change rarely (once per epoch), it makes sense to cache the result.
@spacemesh-bors
Copy link

spacemesh-bors bot commented Aug 1, 2024

Pull request successfully merged into develop.

Build succeeded:

@spacemesh-bors spacemesh-bors bot changed the title cache poet's /v1/pow_params with TTL [Merged by Bors] - cache poet's /v1/pow_params with TTL Aug 1, 2024
@spacemesh-bors spacemesh-bors bot closed this Aug 1, 2024
@spacemesh-bors spacemesh-bors bot deleted the fix/cache-poet-pow-params branch August 1, 2024 13:24
poszu added a commit that referenced this pull request Aug 1, 2024
## Motivation

Similarly to /v1/info, we query /v1/pow_params very often (once per submit per node ID). As the contents returned from this endpoint change rarely (once per epoch), it makes sense to cache the result.
fasmat pushed a commit that referenced this pull request Aug 1, 2024
* configure poet /v1/info cache ttl in presets (#6198)

## Motivation

For the cache to work, it needs to have non-zero TTL set

* cache poet's /v1/pow_params with TTL (#6199)

## Motivation

Similarly to /v1/info, we query /v1/pow_params very often (once per submit per node ID). As the contents returned from this endpoint change rarely (once per epoch), it makes sense to cache the result.

* update changelog
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