-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
eth/filters: add global block logs cache #25459
Conversation
lgtm! Thanks! |
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.
LGTM
eth/filters/filter.go
Outdated
@@ -266,6 +267,9 @@ func (f *Filter) checkMatches(ctx context.Context, header *types.Header) (logs [ | |||
if err != nil { | |||
return nil, err | |||
} | |||
if logsList == 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.
It's weird to add this "duplicated" check here, usually a nil error means the returned result is not empty.
I would vote to add the check here https://github.com/ethereum/go-ethereum/pull/25459/files#diff-9886da3412b43831145f62cec6e895eb3613a175b945e5b026543b7463454603R204
func (b *EthAPIBackend) GetLogs(ctx context.Context, hash common.Hash) ([][]*types.Log, error) {
result := b.eth.blockchain.GetLogsByHash(hash)
if result == nil {
return nil, errors.New("xxx")
}
return b.eth.blockchain.GetLogsByHash(hash), nil
}
Also in les_apiBackend.GetLogs
, we should also return the error if logs is not found.
func (b *LesApiBackend) GetLogs(ctx context.Context, hash common.Hash) ([][]*types.Log, error) {
if number := rawdb.ReadHeaderNumber(b.eth.chainDb, hash); number != nil {
return light.GetBlockLogs(ctx, b.eth.odr, hash, *number)
}
return nil, fmt.Errorf("failed to get logs for block %x", hash)
}
This is ready for testing now. The cache size is configurable via @holiman Please re-check this, the PR has changed significantly after your initial review. |
This adds a cache for block logs which is shared by all filters. In order to share the cache reference, filters now need to be created through a 'filter system' object.
This adds a cache for block logs which is shared by all filters. In order to share the cache reference, filters now need to be created through a 'filter system' object. Cache size is user-configurable via
--cache.blocklogs
.This PR is an alternative to #25425.
cc @ryanschneider
Fixes #25421
Includes #25468, which should be merged first.