Skip to content

Add load_account_with() API to allow a callback to control whether to insert the loaded account into read_cache.#1350

Merged
mergify[bot] merged 3 commits intoanza-xyz:masterfrom
HaoranYi:accounts-db/load_account_with
May 16, 2024
Merged

Add load_account_with() API to allow a callback to control whether to insert the loaded account into read_cache.#1350
mergify[bot] merged 3 commits intoanza-xyz:masterfrom
HaoranYi:accounts-db/load_account_with

Conversation

@HaoranYi
Copy link

@HaoranYi HaoranYi commented May 14, 2024

Problem

#1157

Split the accounts-db related changes from #1157. The accounts-db changes don't depend on #1157. The new API can be used for other cases than #1157. It also makes the code review easier.

Summary of Changes

  • Add a new API in accounts-db - load_with(), which takes a callback to indicate whether to insert the loaded account into read_cache.
  • Add a test for the new API with read_cache.

Fixes #

@HaoranYi HaoranYi changed the title add load_account_with() API Add load_account_with() API to allow a callback to control whether to insert the loaded account into read_cache. May 14, 2024
self.read_only_accounts_cache
.store(*pubkey, slot, account.clone());
}
Some((account, slot))
Copy link

Choose a reason for hiding this comment

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

Looks good. This should work (for the new index structure of the program cache).

Copy link

Choose a reason for hiding this comment

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

refactor/last_written_slot_in_program_loading is where it will be used. Once this PR merges I can rebase that branch and once that is merged @HaoranYi could rebase #1157. That would be one way to make the two change sets compatible.

@HaoranYi HaoranYi requested a review from brooksprumo May 15, 2024 14:36
brooksprumo
brooksprumo previously approved these changes May 15, 2024
Copy link

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

:shipit:

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 95.18072% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 82.1%. Comparing base (e621336) to head (1a6138d).
Report is 58 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #1350    +/-   ##
========================================
  Coverage    82.1%    82.1%            
========================================
  Files         899      899            
  Lines      237263   237346    +83     
========================================
+ Hits       194810   194915   +105     
+ Misses      42453    42431    -22     

@Lichtso
Copy link

Lichtso commented May 16, 2024

Anything else holding this PR back? Otherwise I would say fix the typo, we reapprove and merge.

@HaoranYi
Copy link
Author

Fix a typo.

@HaoranYi HaoranYi requested a review from brooksprumo May 16, 2024 14:06
@HaoranYi HaoranYi added the automerge automerge Merge this Pull Request automatically once CI passes label May 16, 2024
@mergify mergify bot merged commit ae8825e into anza-xyz:master May 16, 2024
@HaoranYi HaoranYi deleted the accounts-db/load_account_with branch May 16, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge automerge Merge this Pull Request automatically once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants