[R4R]validator rpc api consume mutch time #61
Conversation
unclezoro
force-pushed
the
rpcvaldiators
branch
from
9a19cb5
to
d6cf119
Compare
unclezoro
changed the title
unclezoro
requested review from
ackratos,
darren-liu,
rickyyangz and
yutianwu
and removed request for
ackratos
unclezoro
changed the title
unclezoro
force-pushed
the
rpcvaldiators
branch
from
d6cf119
to
c9b69fe
Compare
unclezoro
changed the title
unclezoro
changed the title
state/store.go
Outdated
| @@ -202,7 +201,14 @@ func (valInfo *ValidatorsInfo) Bytes() []byte { | |||
|
|
|||
| // LoadValidators loads the ValidatorSet for a given height. | |||
| // Returns ErrNoValSetForHeight if the validator set can't be found for this height. | |||
| func LoadValidators(db dbm.DB, height int64) (*types.ValidatorSet, error) { | |||
| // Notice: for recent 1048576 block, it will get exactly result. But the block before that height, | |||
There was a problem hiding this comment.
can we change comment magic number into latestStateToKeep?
state/store.go
Outdated
| proposalAddr := meta.Header.ProposerAddress | ||
| _, proposer := valInfo.ValidatorSet.GetByAddress(proposalAddr.Bytes()) | ||
| if proposer == nil { | ||
| panic( |
There was a problem hiding this comment.
doesn't look like it need panic here, can we just log an error? Or the panic would be captured on entrance of query?
There was a problem hiding this comment.
I think panic is ok(the previous code before I modified is also panic). When can't find proposer from ValidatorSet or can't load valInfo2 from store, our data store is broken, which is a disaster, just fail fast. This service weight accurate more than availability.
unclezoro
force-pushed
the
rpcvaldiators
branch
from
c9b69fe
to
6998a0d
Compare
|
tendermint/tendermint#3044 (comment) According to the above comment, I think there is no accuracy issue about ProposerPriority calculation. Do you agree with me? if |
| ) | ||
| } | ||
| valInfo2.ValidatorSet.IncrementProposerPriority(int(height - valInfo.LastHeightChanged)) // mutate |
There was a problem hiding this comment.
Without IncrementProposerPriority, the priority information of getted valInfo may be inaccurate. Why not to save validator info at every height.
There was a problem hiding this comment.
Without
IncrementProposerPriority, the priority information of getted valInfo may be inaccurate. Why not to save validator info at every height.
It takes too much storage, almost reduntant. @ackratos have save it in 100000000 recent blocks, but before that just rewrite the storage
There was a problem hiding this comment.
How about save validator info at every 1000 or 10000 height no matter validator change or not? Thus IncrementProposerPriority would not cost too much time and disk storage will be much less.
unclezoro
changed the title
Yes, |
also - unexpose it - add a comment - refactor code - add a benchmark, which shows that 100000 results in ~ 100ms to get 100 validators
unclezoro
force-pushed
the
rpcvaldiators
branch
from
6998a0d
to
d0e8d30
Compare
unclezoro
changed the title
Description
fix /validators rpc response time grow with time.
Rationale
This pr do not fully resolve the issue.
There still one problem:
** for recent 1048576 block, it will get exactly result. But the block before that height,
the ProposerPriority is not accurate. **
My concern is that it takes too much cost to get
ProposerPriority(we have to get each block round and replay it ).put the pr in tendermint, community want to rewrite
IncrementProposerPriority, but since it is a hard fork, it will not be that quick, so this pr is a hot fix, I will keep trace ittendermint/tendermint#3438
Now community submit this pr:
tendermint/tendermint#3537
Example
After fix this, no matter what hight we use, it takes 0.01s.
Changes
Preflight checks
make build)make test)make integration_test)Already reviewed by
...
Related issues