Interfacing stateDB cache #646
Conversation
aidan-kwon
requested review from
ehnuje,
jeongkyun-oh,
KimKyungup,
kjhman21,
joowon-byun and
yoomee1313
as code owners
aidan-kwon
force-pushed
the
0827-trieCache
branch
from
f291ad4
to
5e21ec4
Compare
|
I still see a |
@winnie-byun I was intended at the first time to minimize change. Anyway, it is changed now. |
| @@ -0,0 +1,32 @@ | |||
| package statedb | |||
There was a problem hiding this comment.
How about changing this file to localcache.go?
|
|
||
| import "github.com/VictoriaMetrics/fastcache" | ||
|
|
||
| type LocalCache struct { |
There was a problem hiding this comment.
The name LocalCache suggests that there is a RemoteCache struct, but there isn't.
Also, LocalCache sounds like an interface, not a struct.
How about just wrapping fastcache.cache as statedb.FastCache?
There was a problem hiding this comment.
- RemoteCache will be introduced later soon.
- When
fastcache.cacheis wrapped tostatedb.FastCache, type assertion is needed in every method ofstatedb.FastCacheto use methods offastcache.cache. In that case, newly introduced interfaces should returns an error also.
There was a problem hiding this comment.
I am gonna try to find a new name replacing LocalCache later. If you have any idea, please let me know.
There was a problem hiding this comment.
What I meant for "wrapping fastcache.cache as statedb.FastCache" is to change the name of current LocalCache to FastCache, not changing the current interfaces. Let's proceed as what you intended and check if additional change is needed later. I think it is good for now.
There was a problem hiding this comment.
OK. I will consider your comment in the next PR.
Proposed changes
Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
$ make test)