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

Update active stake cache to use control table #196

Merged
merged 2 commits into from May 22, 2023
Merged

Conversation

rdlrt
Copy link
Contributor

@rdlrt rdlrt commented May 22, 2023

Description

Currently, active stake cache uses max(epoch_no) from existing tables to run delta cache on. But this may not always be accurate for pool and epoch, as during setup - values for epoch prior may not be available. Stake Snapshot Cache would provides data an epoch earlier to these tables, thus - relying on max(epoch_no) is no longer an accurate outcome.

Account's active stake cache does not require maintaining historical information and can be left as-is.

Motivation and context

Found recently that some of our instances reported empty array for pool_history endpoint. On verifying, found out epoch as well as pool cache tables could have missing entries due to our current mode of execution. This only started showing up recently on fresh instances restored via snapshot, wherein snapshot cache would fill entries prior to historical information is populated

How has this been tested?

On rdlrt2 instance

@rdlrt rdlrt requested review from Scitz0 and hodlonaut May 22, 2023 02:03
…isting, as max can have values from stake snapshot cache
Copy link
Contributor

@hodlonaut hodlonaut left a comment

Choose a reason for hiding this comment

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

Looks okay, so single control table entry is now in charge of keeping track of how the two different cache tables get refreshed/populated (my understanding). Just a small side comment, I find variable names a little bit confusing sometimes (i.e. if control table entry has a good descriptive one why don't we name the variable we extract its value into the same)

@rdlrt rdlrt merged commit 5936e3c into main May 22, 2023
2 checks passed
@rdlrt rdlrt deleted the activestakecache-patch branch May 22, 2023 06:29
rdlrt added a commit that referenced this pull request May 22, 2023
* Update active stake cache to use control table instead of max from existing, as max can have values from stake snapshot cache

* Rename _last_active_stake_cache_epoch_no to _last_active_stake_validated_epoch
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.

None yet

2 participants