Add Account cache #1519
Add Account cache #1519
Conversation
Once the amp config split was moved to main config package, these tests are checking the same thing twice (that a fetcher can be created from a fetcher config).
Adds a new `size_bytes` parameter for in_memory_cache to control single caches such as Account, and corresponding validation.
```
accounts:
in_memory_cache:
type: lru
ttl_seconds: 10
size_bytes: 100000
```
| return f.fetcher.FetchAccount(ctx, accountID) | ||
| func (f *fetcherWithCache) FetchAccount(ctx context.Context, accountID string) (account json.RawMessage, errs []error) { | ||
| accountData := f.cache.Accounts.Get(ctx, []string{accountID}) | ||
| // TODO: add metrics |
There was a problem hiding this comment.
You mentioned the metrics will be a separate PR right? And you're also planning to add account HTTP and DB fetchers in their own PRs too?
There was a problem hiding this comment.
The metrics commit is ready, I can make a PR right after this, or add another commit here.
Account http is almost ready, will make a PR separately.
I will not have time this round for the Db fetcher, but I'll try to take a stab at it.
config/stored_requests.go
Outdated
| if cfg.Size <= 0 { | ||
| errs = append(errs, fmt.Errorf("%s: in_memory_cache.size_bytes must be >= 0 when in_memory_cache.type=lru. Got %d", section, cfg.Size)) | ||
| } | ||
| if cfg.RequestCacheSize > 0 || cfg.ImpCacheSize > 0 { // should this be a warning instead ? "field is ignored" |
There was a problem hiding this comment.
Yeah, I think this should be fine with being a warning but either way, we should keep this consistent with what we do when size is specified in case of stored request/imp cache which right now is a warning
There was a problem hiding this comment.
I should have changed both at the same time, sorry.
There was a problem hiding this comment.
No worries! Happens to all of us. Thank you for the PR :)
Summary
Optional cache for Accounts (issue #1395, follow-up from #1426). This cache is not necessary for the current static file fetcher, it is intended to support upcoming http API and db fetchers.
Implementation
Added cache support for account fetcher.
Add new
size_bytesparameter forin_memory_cacheto control single cache.Add necessary config validation so
accountsusessize_byteswhile the rest haverequest_cache_size_bytesand
imp_cache_size_bytes.Removed some redundant tests and refactored others a little.
Example config: