feat: add short-lived cache handlers #106
Conversation
proxyd/config.go
Outdated
| TTL TOMLDuration `toml:"ttl"` | ||
| Enabled bool `toml:"enabled"` | ||
| TTL TOMLDuration `toml:"ttl"` | ||
| BlockTTL TOMLDuration `toml:"block_ttl"` |
There was a problem hiding this comment.
block_ttl could probably be changed to something live ShortLivedTTL to better reflect it can be used for multiple RPCs
| "eth_getBlockByNumber": shortLivedHandler, | ||
| "eth_blockNumber": shortLivedHandler, | ||
| "eth_getBalance": shortLivedHandler, | ||
| "eth_getStorageAt": shortLivedHandler, | ||
| "eth_getTransactionCount": shortLivedHandler, | ||
| "eth_getBlockTransactionCountByNumber": shortLivedHandler, | ||
| "eth_getUncleCountByBlockNumber": shortLivedHandler, | ||
| "eth_getCode": shortLivedHandler, | ||
| "eth_getTransactionByBlockNumberAndIndex": shortLivedHandler, | ||
| "eth_getUncleByBlockNumberAndIndex": shortLivedHandler, |
There was a problem hiding this comment.
Should all of these be low TTL? ex. eth_getCode, eth_getBlockByNumber or eth_getBlockTransactionCountByNumber seem like they would return consistent results ?
Just curious how these RPCs were chosen, any related data? I get that tip relevant RPCs and rapidly changing data fit the low TTL behavior
There was a problem hiding this comment.
Follow up could be making this configurable but I think that maybe out of scope for the inital PR
proxyd/proxyd.go
Outdated
|
|
||
| blockTtl := defaultBlockTtl | ||
| if config.Cache.BlockTTL != 0 { | ||
| blockTtl = time.Duration(config.Cache.BlockTTL) | ||
| } | ||
| cache = newRedisCache(redisClient, redisReadClient, config.Redis.Namespace, ttl, blockTtl) | ||
|
|
There was a problem hiding this comment.
Would it be possible to write the feature so the shortLivedTTL can be toggled on and off? With the default being off? It looks like in its current state shortLivedTTL will always be enabled
There was a problem hiding this comment.
Overall looks good, definitely can get this merged! Left a couple comments. PTAL, and in the final form please remove the prints, and add a couple more UTs if the feature can be toggled on and off.
Thanks for the contribution @meyer9!
|
Currently the PR is still in draft, please update once its ready. |
meyer9
force-pushed
the
meyer9/short-lived-cache
branch
from
9202ab3 to
bd91511
Compare
Description
We currently send a ton of traffic to backend nodes on barely uncacheable endpoints (endpoints based on block number). This PR adds a cache with a very short TTL (
block_ttl- defaults to2 seconds) for these endpoints.Short TTLs seem fine for Redis. Other caching backends will currently ignore short lived cache keys.
Open question: should we implement this for in-memory cache?
Tests
TODO
Additional context
Add any other context about the problem you're solving.