Skip to content

add: ETCM-9734 caching for governed map data #732

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jankun4
Copy link
Contributor

@jankun4 jankun4 commented Apr 29, 2025

Description

link
Adds Cached GovernedMap DataSource implementation, and uses it in demo-node

Checklist

  • Commit sequence broadly makes sense and commits have useful messages.
  • The size limit of 400 LOC isn't needlessly exceeded
  • The PR refers to a JIRA ticket (if one exists)
  • New tests are added if needed and existing tests are updated.
  • New code is documented and existing documentation is updated.
  • Relevant logging and metrics added
  • Any changes are noted in the changelog.md for affected crate
  • Self-reviewed the diff

@jankun4 jankun4 force-pushed the ETCM-9734-caching-for-governed-map-data branch 2 times, most recently from 7f4d345 to 3574a17 Compare April 29, 2025 14:46
@jankun4 jankun4 force-pushed the ETCM-9734-caching-for-governed-map-data branch 2 times, most recently from d9982ea to a2c1d27 Compare April 29, 2025 20:22
Copy link
Contributor

@AmbientTea AmbientTea left a comment

Choose a reason for hiding this comment

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

See my comment on the cache safety. I think we're missing some checks. My suggestion is to avoid the issue completely and just replace the cache on every miss, this is what we're doing with native tokens and I don't see a scenario where having more complex cache extension logic makes has any substantial benefits.

@AmbientTea AmbientTea changed the title Etcm 9734 caching for governed map data add: ETCM-9734 caching for governed map data Apr 30, 2025
@jankun4 jankun4 force-pushed the ETCM-9734-caching-for-governed-map-data branch from a2c1d27 to 47cf50f Compare May 2, 2025 13:06
@jankun4 jankun4 requested review from AmbientTea and LGLO May 2, 2025 13:08
@jankun4 jankun4 force-pushed the ETCM-9734-caching-for-governed-map-data branch 3 times, most recently from 7748214 to 76d08af Compare May 5, 2025 11:27
@jankun4 jankun4 requested a review from kpinter-iohk May 5, 2025 11:28
@jankun4 jankun4 dismissed AmbientTea’s stale review May 5, 2025 12:06

The cache is completly replaced every time now.

@jankun4 jankun4 force-pushed the ETCM-9734-caching-for-governed-map-data branch from 76d08af to faeba66 Compare May 5, 2025 12:06
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.

4 participants