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

feat(ARCO-283): remove freecache, combine methods with cache implementation #643

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 2 additions & 4 deletions cmd/arc/services/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/bitcoin-sv/arc/internal/cache"

"github.com/bitcoin-sv/arc/config"
"github.com/coocood/freecache"
"github.com/go-redis/redis/v8"
)

Expand All @@ -15,9 +14,8 @@ var ErrCacheUnknownType = errors.New("unknown cache type")
// NewCacheStore creates a new CacheStore based on the provided configuration.
func NewCacheStore(cacheConfig *config.CacheConfig) (cache.Store, error) {
switch cacheConfig.Engine {
case config.FreeCache:
cacheSize := freecache.NewCache(cacheConfig.Freecache.Size)
return cache.NewFreecacheStore(cacheSize), nil
case config.InMemory:
return cache.NewMemoryStore(), nil
case config.Redis:
c := redis.NewClient(&redis.Options{
Addr: cacheConfig.Redis.Addr,
Expand Down
9 changes: 4 additions & 5 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
)

const (
FreeCache = "freecache"
Redis = "redis"
InMemory = "in-memory"
Redis = "redis"
)

type ArcConfig struct {
Expand Down Expand Up @@ -137,9 +137,8 @@ type PostgresConfig struct {
}

type CacheConfig struct {
Engine string `mapstructure:"engine"`
Freecache *FreeCacheConfig `mapstructure:"freecache"`
Redis *RedisConfig `mapstructure:"redis"`
Engine string `mapstructure:"engine"`
Redis *RedisConfig `mapstructure:"redis"`
}

type FreeCacheConfig struct {
Expand Down
7 changes: 2 additions & 5 deletions config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,8 @@ func getCallbackerConfig() *CallbackerConfig {

func getCacheConfig() *CacheConfig {
return &CacheConfig{
Engine: FreeCache,
Freecache: &FreeCacheConfig{
Size: 10 * 1024 * 1024, // Default size 10MB.
},
Redis: &RedisConfig{
Engine: InMemory, // use in memory cache
Redis: &RedisConfig{ // example of Redis config
Addr: "localhost:6379",
Password: "",
DB: 0,
Expand Down
6 changes: 2 additions & 4 deletions config/example_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,8 @@ broadcasting: # settings for connection to nodes
p2p: 18335

cache:
engine: freecache # cache engine - freecache/redis
freecache: # freecache configuration
size: 10000000 # size of cache
redis:
engine: redis # cache engine - in-memory/redis
redis: # redis cache configuration in case that engine: redis
addr: "localhost:6379"
password: ""
db: 1
Expand Down
18 changes: 11 additions & 7 deletions internal/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ import (
)

var (
ErrCacheNotFound = errors.New("key not found in cache")
ErrCacheFailedToSet = errors.New("failed to set value in cache")
ErrCacheFailedToDel = errors.New("failed to delete value from cache")
ErrCacheFailedToGet = errors.New("failed to get value from cache")
ErrCacheNotFound = errors.New("key not found in cache")
ErrCacheFailedToSet = errors.New("failed to set value in cache")
ErrCacheFailedToDel = errors.New("failed to delete value from cache")
ErrCacheFailedToGet = errors.New("failed to get value from cache")
ErrCacheFailedToScan = errors.New("failed to scan cache")
ErrCacheFailedToGetCount = errors.New("failed to get count from cache")
)

type Store interface {
Get(key string) ([]byte, error)
Set(key string, value []byte, ttl time.Duration) error
Del(key string) error
Get(hash *string, key string) ([]byte, error)
GetAllForHash(hash string) (map[string][]byte, error)
Set(hash *string, key string, value []byte, ttl time.Duration) error
Del(hash *string, keys ...string) error
CountElementsForHash(hash string) (int64, error)
}
50 changes: 0 additions & 50 deletions internal/cache/freecache.go

This file was deleted.

121 changes: 121 additions & 0 deletions internal/cache/in_memory.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package cache

import (
"errors"
"sync"
"time"
)

type MemoryStore struct {
data sync.Map
}

func NewMemoryStore() *MemoryStore {
return &MemoryStore{}
}

// Get retrieves a value by key and hash (if given)
func (s *MemoryStore) Get(hash *string, key string) ([]byte, error) {
if hash != nil {
Comment on lines +17 to +19
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hash is optional, as I understand, but because of that, in every method we have to check if hash != nil. Also, currently, we're always using a hash and never access the general store.

What do you think about defining a list of allowedHashSets (e.g. in cache.go) with hashes of sets that we can use and only allow to use those HSets, instead of allowing using the general store? Or even better - just define those allowable sets in an Enum, which we can use everywhere in code.

Then, you could also check if given hash exists in allowedHashSets (or enum) and if not - return an ErrNotAllowedSet or something like that. You wouldn't have to retrieve values in a different way depending on whether the hash exists or not and it would not have to be a pointer.

hashValue, found := s.data.Load(hash)
if !found {
return nil, ErrCacheNotFound
}

hashMap, ok := hashValue.(map[string][]byte)
if !ok {
return nil, ErrCacheFailedToGet
}

fieldValue, exists := hashMap[key]
if !exists {
return nil, ErrCacheNotFound
}

return fieldValue, nil
}

value, found := s.data.Load(key)
if !found {
return nil, ErrCacheNotFound
}

bytes, ok := value.([]byte)
if !ok {
return nil, ErrCacheFailedToGet
}

return bytes, nil
}

// Set stores a key-value pair, ignoring the ttl parameter.
func (s *MemoryStore) Set(hash *string, key string, value []byte, _ time.Duration) error {
if hash != nil {
raw, _ := s.data.LoadOrStore(*hash, make(map[string][]byte))

hashMap, ok := raw.(map[string][]byte)
if !ok {
return ErrCacheFailedToSet
}

hashMap[key] = value

s.data.Store(*hash, hashMap)
return nil
}

s.data.Store(key, value)
return nil
}

// Del removes a key from the store.
func (s *MemoryStore) Del(hash *string, keys ...string) error {
if hash != nil {
hashValue, found := s.data.Load(*hash)
if !found {
return ErrCacheNotFound
}

hashMap, ok := hashValue.(map[string][]byte)
if !ok {
return errors.Join(ErrCacheFailedToDel, ErrCacheFailedToGet)
}

for _, k := range keys {
delete(hashMap, k)
}

s.data.Store(*hash, hashMap)
return nil
}

for _, k := range keys {
s.data.Delete(k)
}
return nil
}

// GetAllForHash retrieves all key-value pairs for a specific hash.
func (s *MemoryStore) GetAllForHash(hash string) (map[string][]byte, error) {
hashValue, found := s.data.Load(hash)
if !found {
return nil, ErrCacheNotFound
}

hashMap, ok := hashValue.(map[string][]byte)
if !ok {
return nil, ErrCacheFailedToGet
}

return hashMap, nil
}

// CountElementsForHash returns the number of elements in a hash in memory.
func (s *MemoryStore) CountElementsForHash(hash string) (int64, error) {
hashMap, err := s.GetAllForHash(hash)
if err != nil {
return 0, err
}

return int64(len(hashMap)), nil
}
64 changes: 56 additions & 8 deletions internal/cache/redis.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,29 +22,54 @@ func NewRedisStore(ctx context.Context, c redis.UniversalClient) *RedisStore {
}
}

// Get retrieves a value by key.
func (r *RedisStore) Get(key string) ([]byte, error) {
result, err := r.client.Get(r.ctx, key).Result()
// Get retrieves a value by key and hash (if given).
func (r *RedisStore) Get(hash *string, key string) ([]byte, error) {
var result string
var err error

if hash == nil {
result, err = r.client.Get(r.ctx, key).Result()
} else {
result, err = r.client.HGet(r.ctx, *hash, key).Result()
}
Comment on lines +25 to +34
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we're only saving and returning a []byte type from the store. One idea for improvement would be to make this store generic and use jsons. Example:

type RedisStore struct {
	// ...
}

func (r *RedisStore) Get[T any](key string) (T, error) {
	val, err := r.client.Get(r.ctx, key)
	if err != nil {
		return result, err
	}

	// Assume Redis values are stored as JSON strings
	var result T
	err = json.Unmarshal([]byte(val), &result)
	if err != nil {
		return result, ErrFailedToParseValue
	}

	return result, nil
}

func (r *RedisStore) Set[T any](key string, value T) error {
	// Marshal the value into JSON
	data, err := json.Marshal(value)
	if err != nil {
		return ErrFailedToMarshall
	}

	err = r.client.Set(r.ctx, key, string(data))
	if err != nil {
		return ErrCacheFailedToSet
	}

	return nil
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm going too far and this is not required, or maybe it's for another task, but I'm throwing the idea here.

WDYT? @boecklim @arkadiuszos4chain @pawellewandowski98


if errors.Is(err, redis.Nil) {
return nil, ErrCacheNotFound
} else if err != nil {
return nil, errors.Join(ErrCacheFailedToGet, err)
}

return []byte(result), nil
}

// Set stores a value with a TTL.
func (r *RedisStore) Set(key string, value []byte, ttl time.Duration) error {
err := r.client.Set(r.ctx, key, value, ttl).Err()
// Set stores a value with a TTL for a specific hash (if given).
func (r *RedisStore) Set(hash *string, key string, value []byte, ttl time.Duration) error {
var err error

if hash == nil {
err = r.client.Set(r.ctx, key, value, ttl).Err()
} else {
err = r.client.HSet(r.ctx, *hash, key, value).Err()
}
Comment on lines +45 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're only passing in the ttl when we're using the regular Set, otherwise, with HSet there's not TTL


if err != nil {
return errors.Join(ErrCacheFailedToSet, err)
}

return nil
}

// Del removes a value by key.
func (r *RedisStore) Del(key string) error {
result, err := r.client.Del(r.ctx, key).Result()
func (r *RedisStore) Del(hash *string, keys ...string) error {
var result int64
var err error

if hash == nil {
result, err = r.client.Del(r.ctx, keys...).Result()
} else {
result, err = r.client.HDel(r.ctx, *hash, keys...).Result()
}

if err != nil {
return errors.Join(ErrCacheFailedToDel, err)
}
Expand All @@ -53,3 +78,26 @@ func (r *RedisStore) Del(key string) error {
}
return nil
}

// GetAllForHash retrieves all key-value pairs for a specific hash.
func (r *RedisStore) GetAllForHash(hash string) (map[string][]byte, error) {
values, err := r.client.HGetAll(r.ctx, hash).Result()
if err != nil {
return nil, errors.Join(ErrCacheFailedToGet, err)
}

result := make(map[string][]byte)
for field, value := range values {
result[field] = []byte(value)
}
return result, nil
}

// CountElementsForHash returns the number of elements in a hash.
func (r *RedisStore) CountElementsForHash(hash string) (int64, error) {
count, err := r.client.HLen(r.ctx, hash).Result()
if err != nil {
return 0, errors.Join(ErrCacheFailedToGetCount, err)
}
return count, nil
}
2 changes: 0 additions & 2 deletions internal/metamorph/health_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
storeMocks "github.com/bitcoin-sv/arc/internal/metamorph/store/mocks"
)

const baseCacheSize = 100 * 1024 * 1024

func TestCheck(t *testing.T) {
tt := []struct {
name string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"testing"
"time"

"github.com/coocood/freecache"
_ "github.com/golang-migrate/migrate/v4/source/file"
_ "github.com/lib/pq"
"github.com/libsv/go-p2p/chaincfg/chainhash"
Expand Down Expand Up @@ -124,9 +123,8 @@ func TestDoubleSpendDetection(t *testing.T) {
require.NoError(t, err)
defer metamorphStore.Close(context.Background())

cStore := cache.NewFreecacheStore(freecache.NewCache(baseCacheSize))

pm := &mocks.PeerManagerMock{ShutdownFunc: func() {}}
cStore := cache.NewMemoryStore()

processor, err := metamorph.NewProcessor(metamorphStore, cStore, pm, statusMessageChannel,
metamorph.WithMinedTxsChan(minedTxChannel),
Expand Down
Loading
Loading