Exec: commit root caching #518
Conversation
winder
requested review from
asoliman92,
makramkd,
0xAustinWang,
0xnogo and
b-gopalswami
execute/plugin.go
Outdated
| // Filter out reports that cannot be executed (snoozed). | ||
| // TODO: filter func instead of 'canExecute'? | ||
| var filtered []exectypes.CommitData | ||
| for _, commitReport := range reports { |
There was a problem hiding this comment.
The original commit_roots.go also has a nice trick with a sliding window. Therefore we were limiting a number of database logs fetched from the db to memory by narrowing the lookup window based on the latest finalized and executed Commit Report. Not sure if applicable here, but I wanted to raise that as a follow-up perf improvement.
There was a problem hiding this comment.
Yes, thats another great optimization that I've been thinking about. We can take it a step further by adding a "Pending" cache, that way stuck transactions wouldn't prevent us from narrowing the lookup window.
execute/plugin.go
Outdated
| commitRootsCache cache.CommitsRootsCache | ||
|
|
||
| // TODO: remove this, it can be transient. Also its logger is never initialized. | ||
| observationOptimizer optimizers.ObservationOptimizer |
There was a problem hiding this comment.
Can't we remove it as a part of this PR? If not, could please create a ticket? I think we easily forget about these TODOs in the code
There was a problem hiding this comment.
Updated PR to address this TODO.
execute/plugin.go
Outdated
| // TODO: filter func instead of 'canExecute'? | ||
| var filtered []exectypes.CommitData | ||
| for _, commitReport := range reports { | ||
| if !canExecute(commitReport.SourceChain, commitReport.MerkleRoot) { |
There was a problem hiding this comment.
Might be worth logging as debug these skipped reports
There was a problem hiding this comment.
Good idea, added a log.
|
Add a
CommitsRootCachewhich allows the execute plugin to remember what has already been executed. This enables it to skip RPC calls associated with known-executed commit roots.In addition, a snooze button was added which allows us to snooze commit roots associated with cursed chains.