Skip to content
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

[Access] Upgrade lru cache #4605

Closed
wants to merge 88 commits into from
Closed
Show file tree
Hide file tree
Changes from 85 commits
Commits
Show all changes
88 commits
Select commit Hold shift + click to select a range
0f82f8c
4585 Add more testable backend constructor with unit tests
nozim Aug 3, 2023
3d23bcf
4585: Add mock file generation for NodeCommunicator
nozim Aug 3, 2023
f018aca
4585: Add simple lru caching for transaction results and refactor it'…
nozim Aug 3, 2023
8dd08db
4585: Ignore mock file
nozim Aug 3, 2023
b3865c4
4585: Fix goimport issues
nozim Aug 3, 2023
ffd8cf7
Merge branch 'master' into 4585-add-caching-tx-results
nozim Aug 3, 2023
ecdafd7
Merge branch 'master' into 4585-add-caching-tx-results
nozim Aug 5, 2023
d3a5145
Refactor backend constructor, replace old
nozim Aug 5, 2023
8c6fbb4
Upgrade lru cache in backend
nozim Aug 8, 2023
33410a2
Merge branch 'master' into upgrade-lru-cache
nozim Aug 8, 2023
cba02b4
Merge branch 'master' into 4585-add-caching-tx-results
nozim Aug 8, 2023
985b558
Fix goimports
nozim Aug 8, 2023
c19b3a9
Goimport fixes
nozim Aug 9, 2023
fe00dfd
Merge branch 'master' into upgrade-lru-cache
nozim Aug 9, 2023
9507585
Merge branch 'master' into upgrade-lru-cache
nozim Aug 10, 2023
eb0257c
Merge branch 'master' into 4585-add-caching-tx-results
nozim Aug 12, 2023
42897b2
Fix failing tests
nozim Aug 12, 2023
9935534
Update backend constructor annotation
nozim Aug 12, 2023
1e60d8d
Update mock call
nozim Aug 12, 2023
0d51ec3
Remove unused mock setup in backend transactions
nozim Aug 12, 2023
da2c6be
Refine backend transaction tests
nozim Aug 12, 2023
de50262
Simplify backend transactiont tests
nozim Aug 12, 2023
01d423e
Rename test case
nozim Aug 12, 2023
43193d6
Require mock to be called exactly once
nozim Aug 12, 2023
77e65b4
Refactor Access backend constructor
nozim Aug 13, 2023
ed1b3b2
Fix backend tests
nozim Aug 13, 2023
4cfeafc
Revert comment changes, rename colliding status variable name
nozim Aug 13, 2023
fef906c
Revert comment changes, rename colliding status variable
nozim Aug 13, 2023
5b31bfa
Rename Backend constructor params structure
nozim Aug 13, 2023
ae8ccaa
Introduce TxResultCacheParam for configuration
nozim Aug 13, 2023
37bb9a3
Initialise TxResultCacheSize from access node flags
nozim Aug 13, 2023
33232c2
Fix linter issues
nozim Aug 13, 2023
00768db
Remove comment changes
nozim Aug 13, 2023
507af3e
Fix syntax error
nozim Aug 14, 2023
d7c82aa
Rearrange imports according to convention
nozim Aug 14, 2023
3da4a39
Add caching for transactions not found anywhere with respective tests
nozim Aug 14, 2023
3af348b
Fix imports
nozim Aug 14, 2023
6aee5d7
Deduplicate caching related backend transactions unit tests
nozim Aug 14, 2023
33bb8b7
Deduplicate backend transaction tests
nozim Aug 14, 2023
800ec4c
Minor backend transactions history conditional refactoring
nozim Aug 15, 2023
5967df8
Fix backend invocation in observer builder
nozim Aug 15, 2023
47fe825
Fix backend invocation in access test
nozim Aug 15, 2023
77678a4
Fix backend instance invocation
nozim Aug 15, 2023
a1d9268
wip
nozim Aug 15, 2023
3e75d93
Merge branch 'master' into upgrade-lru-cache
nozim Aug 15, 2023
4ef02f3
Refactor magic number hash size in cache key
nozim Aug 15, 2023
e169555
Refactor newcache creation in connectiont tests
nozim Aug 15, 2023
72d1330
Refactor connection tests, add error and flag check
nozim Aug 15, 2023
3eed6df
Refactor simple cache to generic version
nozim Aug 16, 2023
958e7b8
Merge branch 'master' into upgrade-lru-cache
nozim Aug 16, 2023
659e22f
Group imports in connection test
nozim Aug 16, 2023
fe6e4b1
Merge branch 'master' into upgrade-lru-cache
nozim Aug 17, 2023
770d052
Merge branch 'master' into 4585-add-caching-tx-results
nozim Aug 17, 2023
ae57bcf
Merge branch 'master' into 4585-add-caching-tx-results
nozim Aug 18, 2023
73ea0e9
Merge branch 'master' into 4585-add-caching-tx-results
nozim Aug 21, 2023
a4ae343
Remove mock file from gitignore
nozim Aug 21, 2023
a6e33aa
Move node communicator interface out of backend file
nozim Aug 21, 2023
6b95d40
Remove irrelevant code
nozim Aug 21, 2023
5ff6b10
Fix imports order according to convention
nozim Aug 21, 2023
1e28982
Fix imports ordering
nozim Aug 22, 2023
7a2cbb4
Fix linter related issues for security libs
nozim Aug 22, 2023
fc26728
Fix imports in derived_chain_data.go
nozim Aug 22, 2023
86a5fa0
Merge branch 'master' into upgrade-lru-cache
nozim Aug 22, 2023
56e0307
Bring mock generation command to conventional way
nozim Aug 22, 2023
b69632b
Refactor communicator interface to reduce dependency between packages
nozim Aug 22, 2023
fff2cf2
Rename bnd variable in access_node_builder
nozim Aug 22, 2023
a59b763
Refine mock call expectations with type arguments in backend transact…
nozim Aug 22, 2023
730b46e
Add backend transaction tests annotations
nozim Aug 22, 2023
2af6f84
fix imports with locals at the bottom
nozim Aug 22, 2023
2660643
Merge branch 'master' into upgrade-lru-cache
kc1116 Aug 22, 2023
8e4b0fc
Merge branch 'master' into 4585-add-caching-tx-results
franklywatson Aug 22, 2023
689ac6d
Revert import change in mock folders
nozim Aug 23, 2023
c2938e4
Merge branch 'master' into upgrade-lru-cache
nozim Aug 23, 2023
eb2ba85
Add generated mock files
nozim Aug 23, 2023
8ff9fb8
Refactor test check for error
nozim Aug 23, 2023
d03e611
Merge branch 'master' into upgrade-lru-cache
jordanschalm Aug 24, 2023
e3a49d2
Remove unrelevant changes
nozim Aug 25, 2023
41c81a6
Fix imports in backend scripts
nozim Aug 25, 2023
f7805a0
Remove unrelated changes from historical access test
nozim Aug 25, 2023
4936f8b
Fix imports
nozim Aug 25, 2023
9efd131
Remove unused mock
nozim Aug 25, 2023
d6a71cd
Fix compilation errors
nozim Aug 25, 2023
b07879a
Merge branch 'master' into upgrade-lru-cache
franklywatson Aug 30, 2023
eb1808a
Revert mock import changes
nozim Aug 31, 2023
e29737b
Merge branch 'master' into upgrade-lru-cache
franklywatson Aug 31, 2023
9a69798
Add nosec annotation to import
nozim Sep 1, 2023
3ef9bbe
Merge branch 'master' into upgrade-lru-cache
nozim Sep 1, 2023
04e8222
Merge branch 'master' into upgrade-lru-cache
franklywatson Sep 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions engine/access/rpc/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package backend

import (
"context"
"crypto/md5"
nozim marked this conversation as resolved.
Show resolved Hide resolved
nozim marked this conversation as resolved.
Show resolved Hide resolved
"fmt"
"net"
"strconv"
"time"

lru "github.com/hashicorp/golang-lru"
lru2 "github.com/hashicorp/golang-lru/v2"
accessproto "github.com/onflow/flow/protobuf/go/flow/access"
"github.com/rs/zerolog"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand All @@ -22,8 +24,6 @@ import (
"github.com/onflow/flow-go/module"
"github.com/onflow/flow-go/state/protocol"
"github.com/onflow/flow-go/storage"

accessproto "github.com/onflow/flow/protobuf/go/flow/access"
)

// minExecutionNodesCnt is the minimum number of execution nodes expected to have sent the execution receipt for a block
Expand Down Expand Up @@ -119,7 +119,7 @@ func New(
retry.Activate()
}

loggedScripts, err := lru.New(DefaultLoggedScriptsCacheSize)
loggedScripts, err := lru2.New[[md5.Size]byte, time.Time](DefaultLoggedScriptsCacheSize)
if err != nil {
log.Fatal().Err(err).Msg("failed to initialize script logging cache")
}
Expand Down Expand Up @@ -228,14 +228,13 @@ func NewCache(
log zerolog.Logger,
accessMetrics module.AccessMetrics,
connectionPoolSize uint,
) (*lru.Cache, uint, error) {
) (*lru2.Cache[string, *connection.CachedClient], uint, error) {

var cache *lru.Cache
var cache *lru2.Cache[string, *connection.CachedClient]
cacheSize := connectionPoolSize
if cacheSize > 0 {
var err error
cache, err = lru.NewWithEvict(int(cacheSize), func(_, evictedValue interface{}) {
store := evictedValue.(*connection.CachedClient)
cache, err = lru2.NewWithEvict[string, *connection.CachedClient](int(cacheSize), func(_ string, store *connection.CachedClient) {
store.Close()
log.Debug().Str("grpc_conn_evicted", store.Address).Msg("closing grpc connection evicted from pool")
if accessMetrics != nil {
Expand Down
18 changes: 9 additions & 9 deletions engine/access/rpc/backend/backend_block_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ func (b *backendBlockDetails) GetLatestBlock(_ context.Context, isSealed bool) (
return nil, flow.BlockStatusUnknown, status.Errorf(codes.Internal, "could not get latest block: %v", err)
}

status, err := b.getBlockStatus(block)
stat, err := b.getBlockStatus(block)
if err != nil {
return nil, status, err
return nil, stat, err
}
return block, status, nil
return block, stat, nil
}

func (b *backendBlockDetails) GetBlockByID(_ context.Context, id flow.Identifier) (*flow.Block, flow.BlockStatus, error) {
Expand All @@ -62,11 +62,11 @@ func (b *backendBlockDetails) GetBlockByID(_ context.Context, id flow.Identifier
return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(err)
}

status, err := b.getBlockStatus(block)
stat, err := b.getBlockStatus(block)
if err != nil {
return nil, status, err
return nil, stat, err
}
return block, status, nil
return block, stat, nil
}

func (b *backendBlockDetails) GetBlockByHeight(_ context.Context, height uint64) (*flow.Block, flow.BlockStatus, error) {
Expand All @@ -75,11 +75,11 @@ func (b *backendBlockDetails) GetBlockByHeight(_ context.Context, height uint64)
return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(err)
}

status, err := b.getBlockStatus(block)
stat, err := b.getBlockStatus(block)
if err != nil {
return nil, status, err
return nil, stat, err
}
return block, status, nil
return block, stat, nil
}

func (b *backendBlockDetails) getBlockStatus(block *flow.Block) (flow.BlockStatus, error) {
Expand Down
20 changes: 10 additions & 10 deletions engine/access/rpc/backend/backend_block_headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ func (b *backendBlockHeaders) GetLatestBlockHeader(_ context.Context, isSealed b
return nil, flow.BlockStatusUnknown, status.Errorf(codes.Internal, "could not get latest block header: %v", err)
}

status, err := b.getBlockStatus(header)
stat, err := b.getBlockStatus(header)
if err != nil {
return nil, status, err
return nil, stat, err
}
return header, status, nil
return header, stat, nil
}

func (b *backendBlockHeaders) GetBlockHeaderByID(_ context.Context, id flow.Identifier) (*flow.Header, flow.BlockStatus, error) {
Expand All @@ -55,11 +55,11 @@ func (b *backendBlockHeaders) GetBlockHeaderByID(_ context.Context, id flow.Iden
return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(err)
}

status, err := b.getBlockStatus(header)
stat, err := b.getBlockStatus(header)
if err != nil {
return nil, status, err
return nil, stat, err
}
return header, status, nil
return header, stat, nil
}

func (b *backendBlockHeaders) GetBlockHeaderByHeight(_ context.Context, height uint64) (*flow.Header, flow.BlockStatus, error) {
Expand All @@ -68,19 +68,19 @@ func (b *backendBlockHeaders) GetBlockHeaderByHeight(_ context.Context, height u
return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(err)
}

status, err := b.getBlockStatus(header)
stat, err := b.getBlockStatus(header)
if err != nil {
return nil, status, err
return nil, stat, err
}
return header, status, nil
return header, stat, nil
}

func (b *backendBlockHeaders) getBlockStatus(header *flow.Header) (flow.BlockStatus, error) {
sealed, err := b.state.Sealed().Head()
if err != nil {
// In the RPC engine, if we encounter an error from the protocol state indicating state corruption,
// we should halt processing requests, but do throw an exception which might cause a crash:
// - It is unsafe to process requests if we have an internally bad state.
// - It is unsafe to process requests if we have an internally bad State.
// TODO: https://github.com/onflow/flow-go/issues/4028
// - We would like to avoid throwing an exception as a result of an Access API request by policy
// because this can cause DOS potential
Expand Down
3 changes: 1 addition & 2 deletions engine/access/rpc/backend/backend_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import (
"context"
"fmt"

"github.com/onflow/flow-go/cmd/build"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/onflow/flow-go/access"
"github.com/onflow/flow-go/cmd/build"
"github.com/onflow/flow-go/engine/common/rpc/convert"
"github.com/onflow/flow-go/model/flow"
"github.com/onflow/flow-go/state/protocol"
Expand Down
28 changes: 13 additions & 15 deletions engine/access/rpc/backend/backend_scripts.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"time"

"github.com/hashicorp/go-multierror"
lru "github.com/hashicorp/golang-lru"
lru "github.com/hashicorp/golang-lru/v2"
"github.com/onflow/flow/protobuf/go/flow/access"
execproto "github.com/onflow/flow/protobuf/go/flow/execution"
"github.com/rs/zerolog"
Expand All @@ -27,13 +27,14 @@ import (
const uniqueScriptLoggingTimeWindow = 10 * time.Minute

type backendScripts struct {
headers storage.Headers
executionReceipts storage.ExecutionReceipts
state protocol.State
connFactory connection.ConnectionFactory
log zerolog.Logger
metrics module.BackendScriptsMetrics
loggedScripts *lru.Cache
headers storage.Headers
executionReceipts storage.ExecutionReceipts
state protocol.State
connFactory connection.ConnectionFactory
log zerolog.Logger
metrics module.BackendScriptsMetrics
loggedScripts *lru.Cache[[md5.Size]byte, time.Time]

archiveAddressList []string
archivePorts []uint
scriptExecValidation bool
Expand Down Expand Up @@ -219,7 +220,7 @@ func (b *backendScripts) executeScriptOnAvailableArchiveNodes(
Msg("script failed to execute on the execution node")
return nil, err
case codes.NotFound:
// failures due to unavailable blocks are explicitly marked Not found
// failures due to unavailable blocks are explicitly marked Not found
b.metrics.ScriptExecutionErrorOnArchiveNode()
b.log.Error().Err(err).Msg("script execution failed for archive node")
return nil, err
Expand Down Expand Up @@ -310,14 +311,11 @@ func (b *backendScripts) executeScriptOnAvailableExecutionNodes(

// shouldLogScript checks if the script hash is unique in the time window
func (b *backendScripts) shouldLogScript(execTime time.Time, scriptHash [16]byte) bool {
rawTimestamp, seen := b.loggedScripts.Get(scriptHash)
if !seen || rawTimestamp == nil {
timestamp, seen := b.loggedScripts.Get(scriptHash)
if !seen {
return true
} else {
// safe cast
timestamp := rawTimestamp.(time.Time)
return execTime.Sub(timestamp) >= uniqueScriptLoggingTimeWindow
}
return execTime.Sub(timestamp) >= uniqueScriptLoggingTimeWindow
}

func (b *backendScripts) tryExecuteScriptOnExecutionNode(
Expand Down
10 changes: 5 additions & 5 deletions engine/access/rpc/connection/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"sync"
"time"

lru "github.com/hashicorp/golang-lru"
lru "github.com/hashicorp/golang-lru/v2"
"go.uber.org/atomic"
"google.golang.org/grpc"
)
Expand Down Expand Up @@ -38,12 +38,12 @@ func (s *CachedClient) Close() {

// Cache represents a cache of CachedClient instances with a given maximum size.
type Cache struct {
cache *lru.Cache
cache *lru.Cache[string, *CachedClient]
size int
}

// NewCache creates a new Cache with the specified maximum size and the underlying LRU cache.
func NewCache(cache *lru.Cache, size int) *Cache {
func NewCache(cache *lru.Cache[string, *CachedClient], size int) *Cache {
return &Cache{
cache: cache,
size: size,
Expand All @@ -57,7 +57,7 @@ func (c *Cache) Get(address string) (*CachedClient, bool) {
if !ok {
return nil, false
}
return val.(*CachedClient), true
return val, true
}

// GetOrAdd atomically gets the CachedClient for the given address from the cache, or adds a new one
Expand All @@ -71,7 +71,7 @@ func (c *Cache) GetOrAdd(address string, timeout time.Duration) (*CachedClient,

val, existed, _ := c.cache.PeekOrAdd(address, client)
if existed {
return val.(*CachedClient), true
return val, true
}

client.Address = address
Expand Down
Loading