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.

Sign up for GitHub

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

Jump to bottom

feat(indexer): Moar cache #244

Merged
merged 2 commits into from
Mar 31, 2022
Merged

feat(indexer): Moar cache #244

merged 2 commits into from
Mar 31, 2022

Conversation

Yawning
Copy link
Contributor

@Yawning Yawning commented Mar 31, 2022

No description provided.

@ptrus
Copy link
Member

ptrus commented Mar 31, 2022

Would adding one more cache for GetLogs (per round) make sense as well?

@codecov
Copy link

codecov bot commented Mar 31, 2022 •
edited
Loading

Codecov Report

Merging #244 (f7fc08e) into main (c5d8b86) will decrease coverage by 0.24%.
The diff coverage is 60.60%.

@@            Coverage Diff             @@
##             main     #244      +/-   ##
==========================================
- Coverage   58.12%   57.87%   -0.25%     
==========================================
  Files          25       26       +1     
  Lines        2985     3174     +189     
==========================================
+ Hits         1735     1837     +102     
- Misses       1069     1148      +79     
- Partials      181      189       +8
Impacted Files Coverage Δ
conf/config.go 41.07% <ø> (ø)
main.go 9.46% <0.00%> (ø)
indexer/backend_cache.go 35.23% <44.28%> (+8.41%) ⬆️
db/model/model.go 60.93% <60.93%> (ø)
indexer/backend.go 58.26% <100.00%> (-10.92%) ⬇️
indexer/indexer.go 46.19% <100.00%> (ø)
indexer/utils.go 75.65% <100.00%> (+3.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5d8b86...f7fc08e. Read the comment docs.

@Yawning
Copy link
Contributor Author

Yawning commented Mar 31, 2022

Would adding one more cache for GetLogs (per round) make sense as well?

In theory, yes. In practice, it depends on the access patterns, and how much work we want to put into it. The API doesn't help here, because GetLogs takes a range. If the state of the cache is pathologically sparse, then having a cache can end up using more database interactions than not having one.

My current inclination here is to wait till we have metrics on how often this is used, and what the requests look like. If requests like GetLogs(10, 10) is common, then I would not be opposed to special casing that.

@Yawning Yawning changed the title feat(indexer): Add a tx and tx receipt cache feat(indexer): Moar cache Mar 31, 2022
@Yawning
feat(indexer): Add a log cache
f7fc08e 
This auguments the block cache update/eviction routines to maintain a
map of block number to logs vector, and services requests from the cache
iff the entire response is present in the cache.
@Yawning Yawning merged commit a9f4e48 into main Mar 31, 2022
@Yawning Yawning deleted the yawning/feature/231-redux branch March 31, 2022 10:02
@Yawning Yawning mentioned this pull request Mar 31, 2022
Closed
@ptrus ptrus mentioned this pull request Apr 1, 2022
Merged
@Yawning Yawning mentioned this pull request Apr 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ptrus ptrus ptrus approved these changes

@kostko kostko Awaiting requested review from kostko kostko is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Yawning @ptrus