-
Notifications
You must be signed in to change notification settings - Fork 324
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.
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
[api] add cache for ReadContract/State() #2827
Conversation
465c0be
to
b59f347
Compare
Codecov Report
@@ Coverage Diff @@
## master #2827 +/- ##
==========================================
+ Coverage 58.31% 58.42% +0.10%
==========================================
Files 245 246 +1
Lines 22514 22571 +57
==========================================
+ Hits 13129 13187 +58
+ Misses 7714 7705 -9
- Partials 1671 1679 +8
Continue to review full report at Codecov.
|
api/api.go
Outdated
key := height + string(methodName) | ||
for _, v := range arguments { | ||
key += string(v) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the key may not be unique in this way. A better way is using json or protobuf.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in latest push
api/read_cache.go
Outdated
} | ||
|
||
// Put writes according to key | ||
func (rc *ReadCache) Put(ns, key string, value []byte) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's ns for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ns is contract address, key is calldata
next will invalidate per contract address (contracts that are called in new block) in the read cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- https:/iotexproject/iotex-core/pull/2827/files#diff-bf98d5fab5bcfabded9769069e7318683f549ec91cff84c5f87a9ae3a3ca3d8dR1036 it is not contract address
- how can you tell which contract is modified in a block? how complicated will that be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- for readState, that is protocol's name, for ReadContract, that is contract's address
- not too complicated, either check all executions in new block, or check all receipts
d, ok := bin.get(key) | ||
if ok { | ||
rc.hit++ | ||
log.L().Info("API cache hit", zap.Int("total", rc.total), zap.Int("hit", rc.hit)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Info() to monitor the hit-rate for a while once this is rolled out, can turn off later
f2d2dff
to
646496e
Compare
api/api.go
Outdated
if d, ok := api.readCache.Get(sc.Contract() + string(sc.Data())); ok { | ||
res := iotexapi.ReadContractResponse{} | ||
if err := proto.Unmarshal(d, &res); err != nil { | ||
return nil, status.Error(codes.Internal, err.Error()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not expected, should log.err here in case it happens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case, it may be better to fall back to original routine.
api/api.go
Outdated
Data: hex.EncodeToString(retval), | ||
Receipt: receipt.ConvertToReceiptPb(), | ||
}, nil | ||
} | ||
d, _ := proto.Marshal(&res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handle err
|
||
// Clear clears the cache | ||
func (rc *ReadCache) Clear() { | ||
rc.bins = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need lock?
api/api.go
Outdated
if d, ok := api.readCache.Get(sc.Contract() + string(sc.Data())); ok { | ||
res := iotexapi.ReadContractResponse{} | ||
if err := proto.Unmarshal(d, &res); err != nil { | ||
return nil, status.Error(codes.Internal, err.Error()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case, it may be better to fall back to original routine.
api/read_cache.go
Outdated
} | ||
|
||
// Get reads according to key | ||
func (rc *ReadCache) Get(key string) ([]byte, bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will key length cause efficiency problem? if so, we may hash it
* [reward] extend foundation bonus (#2785) * Add unit tests to cover the functions in action/signedaction.go (#2824) * limit the pagination size for all API calls to 5000 -- 2781 (#2800) Co-authored-by: dayuanc <[email protected]> Co-authored-by: CoderZhi <[email protected]> Co-authored-by: Raullen Chai <[email protected]> * remove unnecessary WithFeatureCtx() (#2823) * fix ioctl accountDelete test (#2825) * [ioctl] display chainID and encoding (#2820) * bump Go to 1.17 (#2784) * [api] add cache for ReadContract/State() (#2827) Co-authored-by: dustinxie <[email protected]> Co-authored-by: dayuanc <[email protected]> Co-authored-by: dayuanc <[email protected]> Co-authored-by: CoderZhi <[email protected]> Co-authored-by: Raullen Chai <[email protected]> Co-authored-by: Haaai <[email protected]> Co-authored-by: dustinxie <[email protected]> Co-authored-by: dayuanc <[email protected]> Co-authored-by: dayuanc <[email protected]> Co-authored-by: CoderZhi <[email protected]> Co-authored-by: Raullen Chai <[email protected]> Co-authored-by: Haaai <[email protected]>
* saving, switch to linux * tracing grpc api * saving, switch to linux * tracing grpc api * update tracer.go for test * small change * fix comments * Squashed commit of the following: commit 77a5a15 Author: dustinxie <[email protected]> Date: Tue Oct 5 23:34:52 2021 -0700 [api] add cache for ReadContract/State() (#2827) commit 7d9bf96 Author: Haaai <[email protected]> Date: Tue Oct 5 17:31:37 2021 -0700 bump Go to 1.17 (#2784) commit 09ef379 Author: dustinxie <[email protected]> Date: Tue Oct 5 16:46:49 2021 -0700 [ioctl] display chainID and encoding (#2820) commit c2eab8d Author: Haaai <[email protected]> Date: Tue Oct 5 15:45:45 2021 -0700 fix ioctl accountDelete test (#2825) commit 9489b57 Author: dustinxie <[email protected]> Date: Tue Oct 5 15:14:31 2021 -0700 remove unnecessary WithFeatureCtx() (#2823) commit 3933ae9 Author: dayuanc <[email protected]> Date: Tue Oct 5 07:40:32 2021 -0700 limit the pagination size for all API calls to 5000 -- 2781 (#2800) Co-authored-by: dayuanc <[email protected]> Co-authored-by: CoderZhi <[email protected]> Co-authored-by: Raullen Chai <[email protected]> commit 88d273d Author: dayuanc <[email protected]> Date: Mon Oct 4 10:51:54 2021 -0700 Add unit tests to cover the functions in action/signedaction.go (#2824) commit ebe895c Author: dustinxie <[email protected]> Date: Thu Sep 30 15:32:52 2021 -0700 [reward] extend foundation bonus (#2785) commit baa0a92 Author: CoderZhi <[email protected]> Date: Tue Sep 28 14:09:44 2021 -0700 Fix codecov (#2813) commit 84f06d3 Author: dayuanc <[email protected]> Date: Tue Sep 28 12:17:47 2021 -0700 reduce unnecessary logs for mainnet (#2808) * downgrade some unnecessary error and info logs * fix comments Co-authored-by: Raullen Chai <[email protected]> commit a245185 Author: dustinxie <[email protected]> Date: Mon Sep 27 15:27:59 2021 -0700 [evm] panic in AccessList API (#2816) commit 8eec1b7 Author: dustinxie <[email protected]> Date: Mon Sep 27 11:01:34 2021 -0700 set Jutland to activate at 10-11-2021 3pm PDT (#2812) Co-authored-by: Raullen Chai <[email protected]> commit e89be88 Author: CoderZhi <[email protected]> Date: Sat Sep 25 22:51:54 2021 -0700 set target height (#2807) Co-authored-by: dustinxie <[email protected]> commit dba4993 Author: CoderZhi <[email protected]> Date: Thu Sep 23 11:44:26 2021 -0700 change commit block failure log level to error (#2810) commit 0459a1f Author: mas walker <[email protected]> Date: Wed Sep 22 20:18:23 2021 +0800 unbound feature and version/height (#2768) * unbound feature and version/height Co-authored-by: Raullen Chai <[email protected]> commit c1cea44 Author: millken <[email protected]> Date: Tue Sep 21 06:31:57 2021 +0800 update circleci config, using golang 1.16.6 #2789 (#2791) Co-authored-by: dustinxie <[email protected]> commit f245109 Author: Dustin Xie <[email protected]> Date: Mon Sep 20 09:54:38 2021 -0700 [evm] enable opCall fix at Jutland height commit 3eea5b7 Author: Dustin Xie <[email protected]> Date: Mon Sep 20 12:11:36 2021 -0700 [evm] fix datacopy.json for TestIstanbul commit 0641db0 Author: CoderZhi <[email protected]> Date: Fri Sep 17 14:37:28 2021 -0700 [evm] fix snapshot bug (#2802) * fix snapshot bug * add unit test Co-authored-by: Raullen Chai <[email protected]> commit e0683e8 Author: Haaai <[email protected]> Date: Sat Sep 18 01:34:12 2021 +0800 add datacopy contract test (#2788) * add datacopy test * modify contract * remove debug log * update testdata * add attack bytecode * update datacopy.json * correct code format * remove printStore() in datacopy.sol * update contract test data Co-authored-by: dustinxie <[email protected]> commit 44e0a68 Author: Haaai <[email protected]> Date: Fri Sep 17 15:31:29 2021 +0800 Allow ioctl to show the list of actions for an account (#2750) * [ioctl] allow ioctl to show the list of actions for an account commit d10dabe Author: Haaai <[email protected]> Date: Fri Sep 17 12:08:33 2021 +0800 [api] fix gas estimation calc bug (#2786) * fixed gas estimation bug in api.go * Tracing (#2) * [reward] extend foundation bonus (#2785) * Add unit tests to cover the functions in action/signedaction.go (#2824) * limit the pagination size for all API calls to 5000 -- 2781 (#2800) Co-authored-by: dayuanc <[email protected]> Co-authored-by: CoderZhi <[email protected]> Co-authored-by: Raullen Chai <[email protected]> * remove unnecessary WithFeatureCtx() (#2823) * fix ioctl accountDelete test (#2825) * [ioctl] display chainID and encoding (#2820) * bump Go to 1.17 (#2784) * [api] add cache for ReadContract/State() (#2827) Co-authored-by: dustinxie <[email protected]> Co-authored-by: dayuanc <[email protected]> Co-authored-by: dayuanc <[email protected]> Co-authored-by: CoderZhi <[email protected]> Co-authored-by: Raullen Chai <[email protected]> Co-authored-by: Haaai <[email protected]> Co-authored-by: Raullen Chai <[email protected]> Co-authored-by: dustinxie <[email protected]> Co-authored-by: dayuanc <[email protected]> Co-authored-by: dayuanc <[email protected]> Co-authored-by: CoderZhi <[email protected]> Co-authored-by: Haaai <[email protected]>
No description provided.