From 82106e5caa57b848b892484ae934b2038ebdcacc Mon Sep 17 00:00:00 2001 From: Ofir Elias Date: Thu, 27 Apr 2023 15:58:51 +0300 Subject: [PATCH 1/2] use HMSET for batch Builder status store on initial sync from DB to Redis use HMSET instead of multiple HSET calls --- datastore/redis.go | 16 ++++++- datastore/redis_test.go | 72 +++++++++++++++++++++++++++++ services/housekeeper/housekeeper.go | 11 +++-- 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/datastore/redis.go b/datastore/redis.go index 9f565825..f466b925 100644 --- a/datastore/redis.go +++ b/datastore/redis.go @@ -40,6 +40,11 @@ var ( RedisBlockBuilderStatusBlacklisted BlockBuilderStatus = "blacklisted" ) +type PubKeyStatusPair struct { + PubKey string + Status BlockBuilderStatus +} + func PubkeyHexToLowerStr(pk boostTypes.PubkeyHex) string { return strings.ToLower(string(pk)) } @@ -385,8 +390,17 @@ func (r *RedisCache) GetBidTrace(slot uint64, proposerPubkey, blockHash string) return resp, err } +func (r *RedisCache) SetMultiBlockBuilderStatus(pairs []PubKeyStatusPair) (err error) { + values := []string{} + for _, v := range pairs { + values = append(values, v.PubKey, string(v.Status)) + } + + return r.client.HMSet(context.Background(), r.keyBlockBuilderStatus, values).Err() +} + func (r *RedisCache) SetBlockBuilderStatus(builderPubkey string, status BlockBuilderStatus) (err error) { - return r.client.HSet(context.Background(), r.keyBlockBuilderStatus, builderPubkey, string(status)).Err() + return r.SetMultiBlockBuilderStatus([]PubKeyStatusPair{{Status: status, PubKey: builderPubkey}}) } func (r *RedisCache) GetBlockBuilderStatus(builderPubkey string) (isHighPrio, isBlacklisted bool, err error) { diff --git a/datastore/redis_test.go b/datastore/redis_test.go index 98ca4df6..ec356e33 100644 --- a/datastore/redis_test.go +++ b/datastore/redis_test.go @@ -176,6 +176,78 @@ func TestActiveValidators(t *testing.T) { require.True(t, vals[pk1]) } +func TestBlockBuilderStatus(t *testing.T) { + cache := setupTestRedis(t) + + pk1 := "0x1a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249" + pk2 := "0x2a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249" + pk3 := "0x8016d3229030424cfeff6c5b813970ea193f8d012cfa767270ca9057d58eddc556e96c14544bf4c038dbed5f24aa8da0" + + t.Run("Set and get single block builder status", func(t *testing.T) { + // Set status test + require.NoError(t, cache.SetBlockBuilderStatus(pk1, RedisBlockBuilderStatusHighPrio)) + + isHighPrio, isBlacklisted, err := cache.GetBlockBuilderStatus(pk1) + require.NoError(t, err) + require.True(t, isHighPrio) + require.False(t, isBlacklisted) + + // Update status test + require.NoError(t, cache.SetBlockBuilderStatus(pk1, RedisBlockBuilderStatusBlacklisted)) + + isHighPrio, isBlacklisted, err = cache.GetBlockBuilderStatus(pk1) + require.NoError(t, err) + require.False(t, isHighPrio) + require.True(t, isBlacklisted) + }) + + t.Run("Set and get multi block builder statuses", func(t *testing.T) { + pairs := []PubKeyStatusPair{ + {PubKey: pk1, Status: RedisBlockBuilderStatusLowPrio}, + {PubKey: pk2, Status: RedisBlockBuilderStatusHighPrio}, + {PubKey: pk3, Status: RedisBlockBuilderStatusBlacklisted}, + } + + // Set multi status test + require.NoError(t, cache.SetMultiBlockBuilderStatus(pairs)) + + isHighPrio, isBlacklisted, err := cache.GetBlockBuilderStatus(pk1) + require.NoError(t, err) + require.False(t, isHighPrio) + require.False(t, isBlacklisted) + + isHighPrio, isBlacklisted, err = cache.GetBlockBuilderStatus(pk2) + require.NoError(t, err) + require.True(t, isHighPrio) + require.False(t, isBlacklisted) + + isHighPrio, isBlacklisted, err = cache.GetBlockBuilderStatus(pk3) + require.NoError(t, err) + require.False(t, isHighPrio) + require.True(t, isBlacklisted) + + // Update multi status test + pairs[0].Status = RedisBlockBuilderStatusBlacklisted + + require.NoError(t, cache.SetMultiBlockBuilderStatus(pairs)) + + isHighPrio, isBlacklisted, err = cache.GetBlockBuilderStatus(pk1) + require.NoError(t, err) + require.False(t, isHighPrio) + require.True(t, isBlacklisted) + + isHighPrio, isBlacklisted, err = cache.GetBlockBuilderStatus(pk2) + require.NoError(t, err) + require.True(t, isHighPrio) + require.False(t, isBlacklisted) + + isHighPrio, isBlacklisted, err = cache.GetBlockBuilderStatus(pk3) + require.NoError(t, err) + require.False(t, isHighPrio) + require.True(t, isBlacklisted) + }) +} + func TestBuilderBids(t *testing.T) { slot := uint64(2) parentHash := "0x13e606c7b3d1faad7e83503ce3dedce4c6bb89b0c28ffb240d713c7b110b9747" diff --git a/services/housekeeper/housekeeper.go b/services/housekeeper/housekeeper.go index c33a200e..137b133d 100644 --- a/services/housekeeper/housekeeper.go +++ b/services/housekeeper/housekeeper.go @@ -340,12 +340,15 @@ func (hk *Housekeeper) updateBlockBuildersInRedis() { } hk.log.Infof("updating %d block builders in Redis...", len(builders)) + pairs := []datastore.PubKeyStatusPair{} + for _, builder := range builders { status := datastore.MakeBlockBuilderStatus(builder.IsHighPrio, builder.IsBlacklisted) hk.log.Infof("updating block builder in Redis: %s - %s", builder.BuilderPubkey, status) - err = hk.redis.SetBlockBuilderStatus(builder.BuilderPubkey, status) - if err != nil { - hk.log.WithError(err).Error("failed to set block builder status in redis") - } + pairs = append(pairs, datastore.PubKeyStatusPair{PubKey: builder.BuilderPubkey, Status: status}) + } + err = hk.redis.SetMultiBlockBuilderStatus(pairs) + if err != nil { + hk.log.WithError(err).Error("failed to set block builders status in redis") } } From 14a020ccb4f6eb42f763063a5c04996f1a134ca5 Mon Sep 17 00:00:00 2001 From: Ofir Elias Date: Thu, 27 Apr 2023 17:14:18 +0300 Subject: [PATCH 2/2] use map instead of struct --- datastore/redis.go | 13 ++++--------- datastore/redis_test.go | 10 +++++----- services/housekeeper/housekeeper.go | 6 +++--- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/datastore/redis.go b/datastore/redis.go index f466b925..bbc88ea6 100644 --- a/datastore/redis.go +++ b/datastore/redis.go @@ -40,11 +40,6 @@ var ( RedisBlockBuilderStatusBlacklisted BlockBuilderStatus = "blacklisted" ) -type PubKeyStatusPair struct { - PubKey string - Status BlockBuilderStatus -} - func PubkeyHexToLowerStr(pk boostTypes.PubkeyHex) string { return strings.ToLower(string(pk)) } @@ -390,17 +385,17 @@ func (r *RedisCache) GetBidTrace(slot uint64, proposerPubkey, blockHash string) return resp, err } -func (r *RedisCache) SetMultiBlockBuilderStatus(pairs []PubKeyStatusPair) (err error) { +func (r *RedisCache) SetMultiBlockBuilderStatus(pkStatusMap map[string]BlockBuilderStatus) (err error) { values := []string{} - for _, v := range pairs { - values = append(values, v.PubKey, string(v.Status)) + for publicKey, status := range pkStatusMap { + values = append(values, publicKey, string(status)) } return r.client.HMSet(context.Background(), r.keyBlockBuilderStatus, values).Err() } func (r *RedisCache) SetBlockBuilderStatus(builderPubkey string, status BlockBuilderStatus) (err error) { - return r.SetMultiBlockBuilderStatus([]PubKeyStatusPair{{Status: status, PubKey: builderPubkey}}) + return r.SetMultiBlockBuilderStatus(map[string]BlockBuilderStatus{builderPubkey: status}) } func (r *RedisCache) GetBlockBuilderStatus(builderPubkey string) (isHighPrio, isBlacklisted bool, err error) { diff --git a/datastore/redis_test.go b/datastore/redis_test.go index ec356e33..3d01b4e7 100644 --- a/datastore/redis_test.go +++ b/datastore/redis_test.go @@ -202,10 +202,10 @@ func TestBlockBuilderStatus(t *testing.T) { }) t.Run("Set and get multi block builder statuses", func(t *testing.T) { - pairs := []PubKeyStatusPair{ - {PubKey: pk1, Status: RedisBlockBuilderStatusLowPrio}, - {PubKey: pk2, Status: RedisBlockBuilderStatusHighPrio}, - {PubKey: pk3, Status: RedisBlockBuilderStatusBlacklisted}, + pairs := map[string]BlockBuilderStatus{ + pk1: RedisBlockBuilderStatusLowPrio, + pk2: RedisBlockBuilderStatusHighPrio, + pk3: RedisBlockBuilderStatusBlacklisted, } // Set multi status test @@ -227,7 +227,7 @@ func TestBlockBuilderStatus(t *testing.T) { require.True(t, isBlacklisted) // Update multi status test - pairs[0].Status = RedisBlockBuilderStatusBlacklisted + pairs[pk1] = RedisBlockBuilderStatusBlacklisted require.NoError(t, cache.SetMultiBlockBuilderStatus(pairs)) diff --git a/services/housekeeper/housekeeper.go b/services/housekeeper/housekeeper.go index 137b133d..8fe2219d 100644 --- a/services/housekeeper/housekeeper.go +++ b/services/housekeeper/housekeeper.go @@ -340,14 +340,14 @@ func (hk *Housekeeper) updateBlockBuildersInRedis() { } hk.log.Infof("updating %d block builders in Redis...", len(builders)) - pairs := []datastore.PubKeyStatusPair{} + pkStatusMap := make(map[string]datastore.BlockBuilderStatus) for _, builder := range builders { status := datastore.MakeBlockBuilderStatus(builder.IsHighPrio, builder.IsBlacklisted) hk.log.Infof("updating block builder in Redis: %s - %s", builder.BuilderPubkey, status) - pairs = append(pairs, datastore.PubKeyStatusPair{PubKey: builder.BuilderPubkey, Status: status}) + pkStatusMap[builder.BuilderPubkey] = status } - err = hk.redis.SetMultiBlockBuilderStatus(pairs) + err = hk.redis.SetMultiBlockBuilderStatus(pkStatusMap) if err != nil { hk.log.WithError(err).Error("failed to set block builders status in redis") }