From f27c3b43db79d73c3dfb9de33fb8d0524337236b Mon Sep 17 00:00:00 2001 From: Sorin Stanculeanu Date: Thu, 5 Oct 2023 17:30:45 +0300 Subject: [PATCH 1/7] improved eviction on inner tx pool --- testscommon/evictionNotifierStub.go | 11 ++++++++++- txcache/baseTxCache.go | 4 ++++ types/interface.go | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/testscommon/evictionNotifierStub.go b/testscommon/evictionNotifierStub.go index 1a9c47fa..edaba8df 100644 --- a/testscommon/evictionNotifierStub.go +++ b/testscommon/evictionNotifierStub.go @@ -2,7 +2,8 @@ package testscommon // EvictionNotifierStub - type EvictionNotifierStub struct { - NotifyEvictionCalled func(txHash []byte) + NotifyEvictionCalled func(txHash []byte) + ShouldNotifyEvictionCalled func(txHash []byte) bool } // NotifyEviction - @@ -12,6 +13,14 @@ func (stub *EvictionNotifierStub) NotifyEviction(txHash []byte) { } } +// ShouldNotifyEviction - +func (stub *EvictionNotifierStub) ShouldNotifyEviction(txHash []byte) bool { + if stub.ShouldNotifyEvictionCalled != nil { + return stub.ShouldNotifyEvictionCalled(txHash) + } + return false +} + // IsInterfaceNil - func (stub *EvictionNotifierStub) IsInterfaceNil() bool { return stub == nil diff --git a/txcache/baseTxCache.go b/txcache/baseTxCache.go index bdcbf0e9..37c35a5d 100644 --- a/txcache/baseTxCache.go +++ b/txcache/baseTxCache.go @@ -43,6 +43,10 @@ func (cache *baseTxCache) enqueueEvictedHashesForNotification(txHashes [][]byte) for _, handler := range handlers { for _, txHash := range txHashes { + if !handler.ShouldNotifyEviction(txHash) { + continue + } + cache.evictionWorkerPool.Submit(func() { handler.NotifyEviction(txHash) }) diff --git a/types/interface.go b/types/interface.go index 4a483f6a..ad841209 100644 --- a/types/interface.go +++ b/types/interface.go @@ -234,5 +234,6 @@ type PersisterCreator interface { // EvictionNotifier defines the behaviour of a component which is able to handle an evicted transaction type EvictionNotifier interface { NotifyEviction(txHash []byte) + ShouldNotifyEviction(txHash []byte) bool IsInterfaceNil() bool } From c78a414ad9e93e06c75f930dd1cb52b344cb049a Mon Sep 17 00:00:00 2001 From: Sorin Stanculeanu Date: Thu, 5 Oct 2023 17:45:49 +0300 Subject: [PATCH 2/7] fixed tests --- txcache/crossTxCache_test.go | 3 +++ txcache/txCache_test.go | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/txcache/crossTxCache_test.go b/txcache/crossTxCache_test.go index 61aa9423..a7556dc2 100644 --- a/txcache/crossTxCache_test.go +++ b/txcache/crossTxCache_test.go @@ -80,6 +80,9 @@ func TestCrossTxCache_RegisterEvictionHandler(t *testing.T) { require.True(t, bytes.Equal([]byte("hash-1"), hash)) ch <- struct{}{} }, + ShouldNotifyEvictionCalled: func(txHash []byte) bool { + return true + }, }) require.NoError(t, err) diff --git a/txcache/txCache_test.go b/txcache/txCache_test.go index 4af75c83..e22a5025 100644 --- a/txcache/txCache_test.go +++ b/txcache/txCache_test.go @@ -559,6 +559,9 @@ func TestTxCache_NoCriticalInconsistency_WhenConcurrentAdditionsAndRemovals(t *t atomic.AddUint32(&handlerCalls, 1) evictionHandlerWG.Done() }, + ShouldNotifyEvictionCalled: func(txHash []byte) bool { + return true + }, }) // A lot of routines concur to add & remove THE FIRST transaction of a sender @@ -668,6 +671,9 @@ func TestTxCache_RegisterEvictionHandler(t *testing.T) { require.True(t, bytes.Equal([]byte("hash-1"), hash) || bytes.Equal([]byte("hash-2"), hash)) ch <- atomic.LoadUint32(&cnt) }, + ShouldNotifyEvictionCalled: func(txHash []byte) bool { + return true + }, }) require.NoError(t, err) From 0c486188b4ba31f46b464ad1924d9afa7800c713 Mon Sep 17 00:00:00 2001 From: Sorin Stanculeanu Date: Fri, 6 Oct 2023 11:48:12 +0300 Subject: [PATCH 3/7] added cache id on NotifyEviction --- testscommon/evictionNotifierStub.go | 6 +++--- txcache/baseTxCache.go | 3 ++- txcache/crossTxCache.go | 1 + txcache/crossTxCache_test.go | 2 +- txcache/txCache.go | 3 +-- txcache/txCache_test.go | 4 ++-- types/interface.go | 2 +- 7 files changed, 11 insertions(+), 10 deletions(-) diff --git a/testscommon/evictionNotifierStub.go b/testscommon/evictionNotifierStub.go index edaba8df..ee938950 100644 --- a/testscommon/evictionNotifierStub.go +++ b/testscommon/evictionNotifierStub.go @@ -2,14 +2,14 @@ package testscommon // EvictionNotifierStub - type EvictionNotifierStub struct { - NotifyEvictionCalled func(txHash []byte) + NotifyEvictionCalled func(txHash []byte, cacheId string) ShouldNotifyEvictionCalled func(txHash []byte) bool } // NotifyEviction - -func (stub *EvictionNotifierStub) NotifyEviction(txHash []byte) { +func (stub *EvictionNotifierStub) NotifyEviction(txHash []byte, cacheId string) { if stub.NotifyEvictionCalled != nil { - stub.NotifyEvictionCalled(txHash) + stub.NotifyEvictionCalled(txHash, cacheId) } } diff --git a/txcache/baseTxCache.go b/txcache/baseTxCache.go index 37c35a5d..309b0cfc 100644 --- a/txcache/baseTxCache.go +++ b/txcache/baseTxCache.go @@ -19,6 +19,7 @@ type baseTxCache struct { mutEvictionHandlers sync.RWMutex evictionHandlers []types.EvictionNotifier evictionWorkerPool evictionWorkerPool + name string } // RegisterEvictionHandler registers a handler which will be called when a tx is evicted from cache @@ -48,7 +49,7 @@ func (cache *baseTxCache) enqueueEvictedHashesForNotification(txHashes [][]byte) } cache.evictionWorkerPool.Submit(func() { - handler.NotifyEviction(txHash) + handler.NotifyEviction(txHash, cache.name) }) } } diff --git a/txcache/crossTxCache.go b/txcache/crossTxCache.go index 7baf4e8e..8210b783 100644 --- a/txcache/crossTxCache.go +++ b/txcache/crossTxCache.go @@ -42,6 +42,7 @@ func NewCrossTxCache(config ConfigDestinationMe) (*CrossTxCache, error) { baseTxCache: &baseTxCache{ evictionHandlers: make([]types.EvictionNotifier, 0), evictionWorkerPool: workerpool.New(maxNumOfEvictionWorkers), + name: config.Name, }, config: config, } diff --git a/txcache/crossTxCache_test.go b/txcache/crossTxCache_test.go index a7556dc2..c43b8734 100644 --- a/txcache/crossTxCache_test.go +++ b/txcache/crossTxCache_test.go @@ -76,7 +76,7 @@ func TestCrossTxCache_RegisterEvictionHandler(t *testing.T) { ch := make(chan struct{}) err = cache.RegisterEvictionHandler(&testscommon.EvictionNotifierStub{ - NotifyEvictionCalled: func(hash []byte) { + NotifyEvictionCalled: func(hash []byte, cacheId string) { require.True(t, bytes.Equal([]byte("hash-1"), hash)) ch <- struct{}{} }, diff --git a/txcache/txCache.go b/txcache/txCache.go index 6378297f..4249e792 100644 --- a/txcache/txCache.go +++ b/txcache/txCache.go @@ -16,7 +16,6 @@ var _ types.Cacher = (*TxCache)(nil) // TxCache represents a cache-like structure (it has a fixed capacity and implements an eviction mechanism) for holding transactions type TxCache struct { *baseTxCache - name string txListBySender *txListBySenderMap txByHash *txByHashMap config ConfigSourceMe @@ -55,8 +54,8 @@ func NewTxCache(config ConfigSourceMe, txGasHandler TxGasHandler) (*TxCache, err baseTxCache: &baseTxCache{ evictionHandlers: make([]types.EvictionNotifier, 0), evictionWorkerPool: workerpool.New(maxNumOfEvictionWorkers), + name: config.Name, }, - name: config.Name, txListBySender: newTxListBySenderMap(numChunks, senderConstraintsObj, scoreComputerObj, txGasHandler, txFeeHelper), txByHash: newTxByHashMap(numChunks), config: config, diff --git a/txcache/txCache_test.go b/txcache/txCache_test.go index e22a5025..ce277a96 100644 --- a/txcache/txCache_test.go +++ b/txcache/txCache_test.go @@ -555,7 +555,7 @@ func TestTxCache_NoCriticalInconsistency_WhenConcurrentAdditionsAndRemovals(t *t handlerCalls := uint32(0) evictionHandlerWG := sync.WaitGroup{} _ = cache.RegisterEvictionHandler(&testscommon.EvictionNotifierStub{ - NotifyEvictionCalled: func(hash []byte) { + NotifyEvictionCalled: func(hash []byte, cacheId string) { atomic.AddUint32(&handlerCalls, 1) evictionHandlerWG.Done() }, @@ -666,7 +666,7 @@ func TestTxCache_RegisterEvictionHandler(t *testing.T) { ch := make(chan uint32) cnt := uint32(0) err = cache.RegisterEvictionHandler(&testscommon.EvictionNotifierStub{ - NotifyEvictionCalled: func(hash []byte) { + NotifyEvictionCalled: func(hash []byte, cacheId string) { atomic.AddUint32(&cnt, 1) require.True(t, bytes.Equal([]byte("hash-1"), hash) || bytes.Equal([]byte("hash-2"), hash)) ch <- atomic.LoadUint32(&cnt) diff --git a/types/interface.go b/types/interface.go index ad841209..a001227e 100644 --- a/types/interface.go +++ b/types/interface.go @@ -233,7 +233,7 @@ type PersisterCreator interface { // EvictionNotifier defines the behaviour of a component which is able to handle an evicted transaction type EvictionNotifier interface { - NotifyEviction(txHash []byte) + NotifyEviction(txHash []byte, cacheId string) ShouldNotifyEviction(txHash []byte) bool IsInterfaceNil() bool } From b476f34fa7af958596f02be76a3492b9d52ff1f3 Mon Sep 17 00:00:00 2001 From: Sorin Stanculeanu Date: Fri, 6 Oct 2023 11:53:23 +0300 Subject: [PATCH 4/7] added cache id on ShouldNotifyEviction --- testscommon/evictionNotifierStub.go | 6 +++--- txcache/baseTxCache.go | 2 +- txcache/crossTxCache_test.go | 2 +- txcache/txCache_test.go | 4 ++-- types/interface.go | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/testscommon/evictionNotifierStub.go b/testscommon/evictionNotifierStub.go index ee938950..b354e393 100644 --- a/testscommon/evictionNotifierStub.go +++ b/testscommon/evictionNotifierStub.go @@ -3,7 +3,7 @@ package testscommon // EvictionNotifierStub - type EvictionNotifierStub struct { NotifyEvictionCalled func(txHash []byte, cacheId string) - ShouldNotifyEvictionCalled func(txHash []byte) bool + ShouldNotifyEvictionCalled func(txHash []byte, cacheId string) bool } // NotifyEviction - @@ -14,9 +14,9 @@ func (stub *EvictionNotifierStub) NotifyEviction(txHash []byte, cacheId string) } // ShouldNotifyEviction - -func (stub *EvictionNotifierStub) ShouldNotifyEviction(txHash []byte) bool { +func (stub *EvictionNotifierStub) ShouldNotifyEviction(txHash []byte, cacheId string) bool { if stub.ShouldNotifyEvictionCalled != nil { - return stub.ShouldNotifyEvictionCalled(txHash) + return stub.ShouldNotifyEvictionCalled(txHash, cacheId) } return false } diff --git a/txcache/baseTxCache.go b/txcache/baseTxCache.go index 309b0cfc..733ed19b 100644 --- a/txcache/baseTxCache.go +++ b/txcache/baseTxCache.go @@ -44,7 +44,7 @@ func (cache *baseTxCache) enqueueEvictedHashesForNotification(txHashes [][]byte) for _, handler := range handlers { for _, txHash := range txHashes { - if !handler.ShouldNotifyEviction(txHash) { + if !handler.ShouldNotifyEviction(txHash, cache.name) { continue } diff --git a/txcache/crossTxCache_test.go b/txcache/crossTxCache_test.go index c43b8734..e5d99f85 100644 --- a/txcache/crossTxCache_test.go +++ b/txcache/crossTxCache_test.go @@ -80,7 +80,7 @@ func TestCrossTxCache_RegisterEvictionHandler(t *testing.T) { require.True(t, bytes.Equal([]byte("hash-1"), hash)) ch <- struct{}{} }, - ShouldNotifyEvictionCalled: func(txHash []byte) bool { + ShouldNotifyEvictionCalled: func(txHash []byte, cacheId string) bool { return true }, }) diff --git a/txcache/txCache_test.go b/txcache/txCache_test.go index ce277a96..4e92b1bc 100644 --- a/txcache/txCache_test.go +++ b/txcache/txCache_test.go @@ -559,7 +559,7 @@ func TestTxCache_NoCriticalInconsistency_WhenConcurrentAdditionsAndRemovals(t *t atomic.AddUint32(&handlerCalls, 1) evictionHandlerWG.Done() }, - ShouldNotifyEvictionCalled: func(txHash []byte) bool { + ShouldNotifyEvictionCalled: func(txHash []byte, cacheId string) bool { return true }, }) @@ -671,7 +671,7 @@ func TestTxCache_RegisterEvictionHandler(t *testing.T) { require.True(t, bytes.Equal([]byte("hash-1"), hash) || bytes.Equal([]byte("hash-2"), hash)) ch <- atomic.LoadUint32(&cnt) }, - ShouldNotifyEvictionCalled: func(txHash []byte) bool { + ShouldNotifyEvictionCalled: func(txHash []byte, cacheId string) bool { return true }, }) diff --git a/types/interface.go b/types/interface.go index a001227e..67a913ef 100644 --- a/types/interface.go +++ b/types/interface.go @@ -234,6 +234,6 @@ type PersisterCreator interface { // EvictionNotifier defines the behaviour of a component which is able to handle an evicted transaction type EvictionNotifier interface { NotifyEviction(txHash []byte, cacheId string) - ShouldNotifyEviction(txHash []byte) bool + ShouldNotifyEviction(txHash []byte, cacheId string) bool IsInterfaceNil() bool } From 74be5e834cc7a4db56a5bb27cdee1e23782c8af3 Mon Sep 17 00:00:00 2001 From: Sorin Stanculeanu Date: Mon, 9 Oct 2023 15:43:59 +0300 Subject: [PATCH 5/7] reverted last changes as it is faster without cache id --- testscommon/evictionNotifierStub.go | 12 ++++++------ txcache/baseTxCache.go | 5 ++--- txcache/crossTxCache_test.go | 4 ++-- txcache/txCache_test.go | 8 ++++---- types/interface.go | 4 ++-- 5 files changed, 16 insertions(+), 17 deletions(-) diff --git a/testscommon/evictionNotifierStub.go b/testscommon/evictionNotifierStub.go index b354e393..edaba8df 100644 --- a/testscommon/evictionNotifierStub.go +++ b/testscommon/evictionNotifierStub.go @@ -2,21 +2,21 @@ package testscommon // EvictionNotifierStub - type EvictionNotifierStub struct { - NotifyEvictionCalled func(txHash []byte, cacheId string) - ShouldNotifyEvictionCalled func(txHash []byte, cacheId string) bool + NotifyEvictionCalled func(txHash []byte) + ShouldNotifyEvictionCalled func(txHash []byte) bool } // NotifyEviction - -func (stub *EvictionNotifierStub) NotifyEviction(txHash []byte, cacheId string) { +func (stub *EvictionNotifierStub) NotifyEviction(txHash []byte) { if stub.NotifyEvictionCalled != nil { - stub.NotifyEvictionCalled(txHash, cacheId) + stub.NotifyEvictionCalled(txHash) } } // ShouldNotifyEviction - -func (stub *EvictionNotifierStub) ShouldNotifyEviction(txHash []byte, cacheId string) bool { +func (stub *EvictionNotifierStub) ShouldNotifyEviction(txHash []byte) bool { if stub.ShouldNotifyEvictionCalled != nil { - return stub.ShouldNotifyEvictionCalled(txHash, cacheId) + return stub.ShouldNotifyEvictionCalled(txHash) } return false } diff --git a/txcache/baseTxCache.go b/txcache/baseTxCache.go index 733ed19b..37c35a5d 100644 --- a/txcache/baseTxCache.go +++ b/txcache/baseTxCache.go @@ -19,7 +19,6 @@ type baseTxCache struct { mutEvictionHandlers sync.RWMutex evictionHandlers []types.EvictionNotifier evictionWorkerPool evictionWorkerPool - name string } // RegisterEvictionHandler registers a handler which will be called when a tx is evicted from cache @@ -44,12 +43,12 @@ func (cache *baseTxCache) enqueueEvictedHashesForNotification(txHashes [][]byte) for _, handler := range handlers { for _, txHash := range txHashes { - if !handler.ShouldNotifyEviction(txHash, cache.name) { + if !handler.ShouldNotifyEviction(txHash) { continue } cache.evictionWorkerPool.Submit(func() { - handler.NotifyEviction(txHash, cache.name) + handler.NotifyEviction(txHash) }) } } diff --git a/txcache/crossTxCache_test.go b/txcache/crossTxCache_test.go index e5d99f85..a7556dc2 100644 --- a/txcache/crossTxCache_test.go +++ b/txcache/crossTxCache_test.go @@ -76,11 +76,11 @@ func TestCrossTxCache_RegisterEvictionHandler(t *testing.T) { ch := make(chan struct{}) err = cache.RegisterEvictionHandler(&testscommon.EvictionNotifierStub{ - NotifyEvictionCalled: func(hash []byte, cacheId string) { + NotifyEvictionCalled: func(hash []byte) { require.True(t, bytes.Equal([]byte("hash-1"), hash)) ch <- struct{}{} }, - ShouldNotifyEvictionCalled: func(txHash []byte, cacheId string) bool { + ShouldNotifyEvictionCalled: func(txHash []byte) bool { return true }, }) diff --git a/txcache/txCache_test.go b/txcache/txCache_test.go index 4e92b1bc..e22a5025 100644 --- a/txcache/txCache_test.go +++ b/txcache/txCache_test.go @@ -555,11 +555,11 @@ func TestTxCache_NoCriticalInconsistency_WhenConcurrentAdditionsAndRemovals(t *t handlerCalls := uint32(0) evictionHandlerWG := sync.WaitGroup{} _ = cache.RegisterEvictionHandler(&testscommon.EvictionNotifierStub{ - NotifyEvictionCalled: func(hash []byte, cacheId string) { + NotifyEvictionCalled: func(hash []byte) { atomic.AddUint32(&handlerCalls, 1) evictionHandlerWG.Done() }, - ShouldNotifyEvictionCalled: func(txHash []byte, cacheId string) bool { + ShouldNotifyEvictionCalled: func(txHash []byte) bool { return true }, }) @@ -666,12 +666,12 @@ func TestTxCache_RegisterEvictionHandler(t *testing.T) { ch := make(chan uint32) cnt := uint32(0) err = cache.RegisterEvictionHandler(&testscommon.EvictionNotifierStub{ - NotifyEvictionCalled: func(hash []byte, cacheId string) { + NotifyEvictionCalled: func(hash []byte) { atomic.AddUint32(&cnt, 1) require.True(t, bytes.Equal([]byte("hash-1"), hash) || bytes.Equal([]byte("hash-2"), hash)) ch <- atomic.LoadUint32(&cnt) }, - ShouldNotifyEvictionCalled: func(txHash []byte, cacheId string) bool { + ShouldNotifyEvictionCalled: func(txHash []byte) bool { return true }, }) diff --git a/types/interface.go b/types/interface.go index 67a913ef..ad841209 100644 --- a/types/interface.go +++ b/types/interface.go @@ -233,7 +233,7 @@ type PersisterCreator interface { // EvictionNotifier defines the behaviour of a component which is able to handle an evicted transaction type EvictionNotifier interface { - NotifyEviction(txHash []byte, cacheId string) - ShouldNotifyEviction(txHash []byte, cacheId string) bool + NotifyEviction(txHash []byte) + ShouldNotifyEviction(txHash []byte) bool IsInterfaceNil() bool } From 7385c2a1c32f82b01b11f80eed9c0196350604f7 Mon Sep 17 00:00:00 2001 From: Sorin Stanculeanu Date: Mon, 9 Oct 2023 15:48:32 +0300 Subject: [PATCH 6/7] fixes --- txcache/crossTxCache.go | 1 - txcache/txCache.go | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/txcache/crossTxCache.go b/txcache/crossTxCache.go index 8210b783..7baf4e8e 100644 --- a/txcache/crossTxCache.go +++ b/txcache/crossTxCache.go @@ -42,7 +42,6 @@ func NewCrossTxCache(config ConfigDestinationMe) (*CrossTxCache, error) { baseTxCache: &baseTxCache{ evictionHandlers: make([]types.EvictionNotifier, 0), evictionWorkerPool: workerpool.New(maxNumOfEvictionWorkers), - name: config.Name, }, config: config, } diff --git a/txcache/txCache.go b/txcache/txCache.go index 4249e792..6378297f 100644 --- a/txcache/txCache.go +++ b/txcache/txCache.go @@ -16,6 +16,7 @@ var _ types.Cacher = (*TxCache)(nil) // TxCache represents a cache-like structure (it has a fixed capacity and implements an eviction mechanism) for holding transactions type TxCache struct { *baseTxCache + name string txListBySender *txListBySenderMap txByHash *txByHashMap config ConfigSourceMe @@ -54,8 +55,8 @@ func NewTxCache(config ConfigSourceMe, txGasHandler TxGasHandler) (*TxCache, err baseTxCache: &baseTxCache{ evictionHandlers: make([]types.EvictionNotifier, 0), evictionWorkerPool: workerpool.New(maxNumOfEvictionWorkers), - name: config.Name, }, + name: config.Name, txListBySender: newTxListBySenderMap(numChunks, senderConstraintsObj, scoreComputerObj, txGasHandler, txFeeHelper), txByHash: newTxByHashMap(numChunks), config: config, From 3e8c549bd0c78a0350afe8d44f2a3182e4868a23 Mon Sep 17 00:00:00 2001 From: Sorin Stanculeanu Date: Mon, 9 Oct 2023 18:08:59 +0300 Subject: [PATCH 7/7] added early return in case there is no eviction handler --- txcache/baseTxCache.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/txcache/baseTxCache.go b/txcache/baseTxCache.go index 37c35a5d..51199657 100644 --- a/txcache/baseTxCache.go +++ b/txcache/baseTxCache.go @@ -37,6 +37,10 @@ func (cache *baseTxCache) RegisterEvictionHandler(handler types.EvictionNotifier // enqueueEvictedHashesForNotification will enqueue the provided hashes on the workers pool func (cache *baseTxCache) enqueueEvictedHashesForNotification(txHashes [][]byte) { cache.mutEvictionHandlers.RLock() + if len(cache.evictionHandlers) == 0 { + cache.mutEvictionHandlers.RUnlock() + return + } handlers := make([]types.EvictionNotifier, len(cache.evictionHandlers)) copy(handlers, cache.evictionHandlers) cache.mutEvictionHandlers.RUnlock()