From 0a23bf705b890283b31178bec3a5224c8210932b Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Tue, 29 Oct 2019 19:15:01 -0700 Subject: [PATCH 1/9] add feature flag to enable shuffled index cache --- beacon-chain/cache/shuffled_indices.go | 7 +++++++ beacon-chain/cache/shuffled_indices_test.go | 8 ++++++++ beacon-chain/core/helpers/committee_test.go | 6 ++++++ shared/featureconfig/config.go | 13 +++++++++---- shared/featureconfig/flags.go | 5 +++++ 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/beacon-chain/cache/shuffled_indices.go b/beacon-chain/cache/shuffled_indices.go index c9ef6d443d6d..7fb531458ec1 100644 --- a/beacon-chain/cache/shuffled_indices.go +++ b/beacon-chain/cache/shuffled_indices.go @@ -7,6 +7,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "k8s.io/client-go/tools/cache" ) @@ -62,6 +63,9 @@ func NewShuffledIndicesCache() *ShuffledIndicesCache { // IndicesByIndexSeed fetches IndicesByIndexSeed by epoch and seed. Returns true with a // reference to the ShuffledIndicesInEpoch info, if exists. Otherwise returns false, nil. func (c *ShuffledIndicesCache) IndicesByIndexSeed(index uint64, seed []byte) ([]uint64, error) { + if !featureconfig.Get().EnableShuffledIndexCache { + return nil, nil + } c.lock.RLock() defer c.lock.RUnlock() key := string(seed) + strconv.Itoa(int(index)) @@ -88,6 +92,9 @@ func (c *ShuffledIndicesCache) IndicesByIndexSeed(index uint64, seed []byte) ([] // AddShuffledValidatorList adds IndicesByIndexSeed object to the cache. This method also trims the least // recently added IndicesByIndexSeed object if the cache size has ready the max cache size limit. func (c *ShuffledIndicesCache) AddShuffledValidatorList(shuffledIndices *IndicesByIndexSeed) error { + if !featureconfig.Get().EnableShuffledIndexCache { + return nil + } c.lock.Lock() defer c.lock.Unlock() if err := c.shuffledIndicesCache.AddIfNotPresent(shuffledIndices); err != nil { diff --git a/beacon-chain/cache/shuffled_indices_test.go b/beacon-chain/cache/shuffled_indices_test.go index 1d1f32273a90..da2b6fe72009 100644 --- a/beacon-chain/cache/shuffled_indices_test.go +++ b/beacon-chain/cache/shuffled_indices_test.go @@ -4,8 +4,16 @@ import ( "reflect" "strconv" "testing" + + "github.com/prysmaticlabs/prysm/shared/featureconfig" ) +func init() { + fc := featureconfig.Get() + fc.EnableShuffledIndexCache = true + featureconfig.Init(fc) +} + func TestShuffleKeyFn_OK(t *testing.T) { sInfo := &IndicesByIndexSeed{ Index: 999, diff --git a/beacon-chain/core/helpers/committee_test.go b/beacon-chain/core/helpers/committee_test.go index 0b6f90b21a03..c40e7b00407d 100644 --- a/beacon-chain/core/helpers/committee_test.go +++ b/beacon-chain/core/helpers/committee_test.go @@ -9,6 +9,7 @@ import ( "github.com/prysmaticlabs/go-bitfield" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/sliceutil" "google.golang.org/grpc/codes" @@ -176,6 +177,11 @@ func TestComputeCommittee_WithoutCache(t *testing.T) { } func TestComputeCommittee_WithCache(t *testing.T) { + fc := featureconfig.Get() + fc.EnableShuffledIndexCache = true + featureconfig.Init(fc) + defer featureconfig.Init(nil) + // Create 10 committees committeeCount := uint64(10) validatorCount := committeeCount * params.BeaconConfig().TargetCommitteeSize diff --git a/shared/featureconfig/config.go b/shared/featureconfig/config.go index 6b851577065c..0b6d0c3d04aa 100644 --- a/shared/featureconfig/config.go +++ b/shared/featureconfig/config.go @@ -36,10 +36,11 @@ type Flag struct { EnableFinalizedBlockRootIndex bool // EnableFinalizedBlockRootIndex in the database. This operation may be expensive at runtime. // Cache toggles. - EnableAttestationCache bool // EnableAttestationCache; see https://github.com/prysmaticlabs/prysm/issues/3106. - EnableEth1DataVoteCache bool // EnableEth1DataVoteCache; see https://github.com/prysmaticlabs/prysm/issues/3106. - EnableNewCache bool // EnableNewCache enables the node to use the new caching scheme. - EnableBLSPubkeyCache bool // EnableBLSPubkeyCache to improve wall time of PubkeyFromBytes. + EnableAttestationCache bool // EnableAttestationCache; see https://github.com/prysmaticlabs/prysm/issues/3106. + EnableEth1DataVoteCache bool // EnableEth1DataVoteCache; see https://github.com/prysmaticlabs/prysm/issues/3106. + EnableNewCache bool // EnableNewCache enables the node to use the new caching scheme. + EnableBLSPubkeyCache bool // EnableBLSPubkeyCache to improve wall time of PubkeyFromBytes. + EnableShuffledIndexCache bool // EnableShuffledIndexCache to cache expensive shuffled index computation. } var featureConfig *Flag @@ -114,6 +115,10 @@ func ConfigureBeaconChain(ctx *cli.Context) { log.Warn("Enabled finalized block root index") cfg.EnableFinalizedBlockRootIndex = true } + if ctx.GlobalBool(enableShuffledIndexCache.Name) { + log.Warn("Enabled shuffled index cache.") + cfg.EnableShuffledIndexCache = true + } Init(cfg) } diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index de51ed042bf8..6410529b0a19 100644 --- a/shared/featureconfig/flags.go +++ b/shared/featureconfig/flags.go @@ -29,6 +29,10 @@ var ( Name: "enable-eth1-data-vote-cache", Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", } + enableShuffledIndexCache = cli.BoolFlag{ + Name: "enable-shuffled-index-cache", + Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", + } // InitSyncNoVerifyFlag enables the initial sync no verify configuration. InitSyncNoVerifyFlag = cli.BoolFlag{ Name: "init-sync-no-verify", @@ -99,6 +103,7 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{ OptimizeProcessEpoch, enableBackupWebhookFlag, enableBLSPubkeyCacheFlag, + enableShuffledIndexCache, pruneFinalizedStatesFlag, enableFinalizedBlockRootIndexFlag, }...) From 0923b863c9697712ae202102bf115c309025a5dc Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Tue, 29 Oct 2019 19:16:06 -0700 Subject: [PATCH 2/9] gaz' --- beacon-chain/core/helpers/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon-chain/core/helpers/BUILD.bazel b/beacon-chain/core/helpers/BUILD.bazel index 5aa98df69e0f..f13fef7702b2 100644 --- a/beacon-chain/core/helpers/BUILD.bazel +++ b/beacon-chain/core/helpers/BUILD.bazel @@ -60,6 +60,7 @@ go_test( "//proto/eth/v1alpha1:go_default_library", "//shared/bls:go_default_library", "//shared/bytesutil:go_default_library", + "//shared/featureconfig:go_default_library", "//shared/params:go_default_library", "//shared/sliceutil:go_default_library", "//shared/testutil:go_default_library", From b3b69620a19539010f5bbbedda933e405f670e10 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Tue, 29 Oct 2019 20:38:52 -0700 Subject: [PATCH 3/9] more flags for cache --- beacon-chain/cache/active_count.go | 4 +++ beacon-chain/cache/active_indices.go | 4 +++ beacon-chain/cache/checkpoint_state.go | 9 +++++++ beacon-chain/cache/committee.go | 23 ++++++++++++++++ beacon-chain/cache/feature_flag_test.go | 10 +++++-- beacon-chain/cache/shuffled_indices_test.go | 8 ------ shared/featureconfig/config.go | 30 +++++++++++++++++---- shared/featureconfig/flags.go | 20 ++++++++++++++ 8 files changed, 93 insertions(+), 15 deletions(-) diff --git a/beacon-chain/cache/active_count.go b/beacon-chain/cache/active_count.go index d11e0761fc96..cda0e83e7113 100644 --- a/beacon-chain/cache/active_count.go +++ b/beacon-chain/cache/active_count.go @@ -7,6 +7,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/params" "k8s.io/client-go/tools/cache" ) @@ -62,6 +63,9 @@ func NewActiveCountCache() *ActiveCountCache { // ActiveCountInEpoch fetches ActiveCountByEpoch by epoch. Returns true with a // reference to the ActiveCountInEpoch info, if exists. Otherwise returns false, nil. func (c *ActiveCountCache) ActiveCountInEpoch(epoch uint64) (uint64, error) { + if !featureconfig.Get().EnableActiveCountCache { + return params.BeaconConfig().FarFutureEpoch, nil + } c.lock.RLock() defer c.lock.RUnlock() obj, exists, err := c.activeCountCache.GetByKey(strconv.Itoa(int(epoch))) diff --git a/beacon-chain/cache/active_indices.go b/beacon-chain/cache/active_indices.go index 4c62b97e0b62..6fe18be717b7 100644 --- a/beacon-chain/cache/active_indices.go +++ b/beacon-chain/cache/active_indices.go @@ -7,6 +7,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "k8s.io/client-go/tools/cache" ) @@ -61,6 +62,9 @@ func NewActiveIndicesCache() *ActiveIndicesCache { // ActiveIndicesInEpoch fetches ActiveIndicesByEpoch by epoch. Returns true with a // reference to the ActiveIndicesInEpoch info, if exists. Otherwise returns false, nil. func (c *ActiveIndicesCache) ActiveIndicesInEpoch(epoch uint64) ([]uint64, error) { + if !featureconfig.Get().EnableActiveIndicesCache { + return nil, nil + } c.lock.RLock() defer c.lock.RUnlock() obj, exists, err := c.activeIndicesCache.GetByKey(strconv.Itoa(int(epoch))) diff --git a/beacon-chain/cache/checkpoint_state.go b/beacon-chain/cache/checkpoint_state.go index c8dd0bf1dbe4..bda241bcd94b 100644 --- a/beacon-chain/cache/checkpoint_state.go +++ b/beacon-chain/cache/checkpoint_state.go @@ -9,6 +9,7 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/hashutil" "k8s.io/client-go/tools/cache" ) @@ -68,6 +69,10 @@ func NewCheckpointStateCache() *CheckpointStateCache { // StateByCheckpoint fetches state by checkpoint. Returns true with a // reference to the CheckpointState info, if exists. Otherwise returns false, nil. func (c *CheckpointStateCache) StateByCheckpoint(cp *ethpb.Checkpoint) (*pb.BeaconState, error) { + if !featureconfig.Get().EnableCheckpointStateCache { + return nil, nil + } + c.lock.RLock() defer c.lock.RUnlock() h, err := hashutil.HashProto(cp) @@ -98,6 +103,10 @@ func (c *CheckpointStateCache) StateByCheckpoint(cp *ethpb.Checkpoint) (*pb.Beac // AddCheckpointState adds CheckpointState object to the cache. This method also trims the least // recently added CheckpointState object if the cache size has ready the max cache size limit. func (c *CheckpointStateCache) AddCheckpointState(cp *CheckpointState) error { + if !featureconfig.Get().EnableCheckpointStateCache { + return nil + } + c.lock.Lock() defer c.lock.Unlock() if err := c.cache.AddIfNotPresent(cp); err != nil { diff --git a/beacon-chain/cache/committee.go b/beacon-chain/cache/committee.go index 0b1ae83a5e08..e0859faf31e7 100644 --- a/beacon-chain/cache/committee.go +++ b/beacon-chain/cache/committee.go @@ -7,6 +7,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/sliceutil" "k8s.io/client-go/tools/cache" @@ -67,6 +68,9 @@ func NewCommitteeCache() *CommitteeCache { // ShuffledIndices fetches the shuffled indices by epoch and shard. Every list of indices // represent one committee. Returns true if the list exists with epoch and shard. Otherwise returns false, nil. func (c *CommitteeCache) ShuffledIndices(epoch uint64, shard uint64) ([]uint64, error) { + if !featureconfig.Get().EnableShuffledIndexCache { + return nil, nil + } c.lock.RLock() defer c.lock.RUnlock() obj, exists, err := c.CommitteeCache.GetByKey(strconv.Itoa(int(epoch))) @@ -94,6 +98,9 @@ func (c *CommitteeCache) ShuffledIndices(epoch uint64, shard uint64) ([]uint64, // AddCommitteeShuffledList adds Committee shuffled list object to the cache. T // his method also trims the least recently list if the cache size has ready the max cache size limit. func (c *CommitteeCache) AddCommitteeShuffledList(committee *Committee) error { + if !featureconfig.Get().EnableShuffledIndexCache { + return nil + } c.lock.Lock() defer c.lock.Unlock() if err := c.CommitteeCache.AddIfNotPresent(committee); err != nil { @@ -105,6 +112,9 @@ func (c *CommitteeCache) AddCommitteeShuffledList(committee *Committee) error { // Epochs returns the epochs stored in the committee cache. These are the keys to the cache. func (c *CommitteeCache) Epochs() ([]uint64, error) { + if !featureconfig.Get().EnableShuffledIndexCache { + return nil, nil + } c.lock.RLock() defer c.lock.RUnlock() @@ -121,6 +131,9 @@ func (c *CommitteeCache) Epochs() ([]uint64, error) { // EpochInCache returns true if an input epoch is part of keys in cache. func (c *CommitteeCache) EpochInCache(wantedEpoch uint64) (bool, error) { + if !featureconfig.Get().EnableShuffledIndexCache { + return false, nil + } c.lock.RLock() defer c.lock.RUnlock() @@ -138,6 +151,9 @@ func (c *CommitteeCache) EpochInCache(wantedEpoch uint64) (bool, error) { // CommitteeCount returns the total number of committees in a given epoch as stored in cache. func (c *CommitteeCache) CommitteeCount(epoch uint64) (uint64, bool, error) { + if !featureconfig.Get().EnableShuffledIndexCache { + return 0, false, nil + } c.lock.RLock() defer c.lock.RUnlock() obj, exists, err := c.CommitteeCache.GetByKey(strconv.Itoa(int(epoch))) @@ -162,6 +178,9 @@ func (c *CommitteeCache) CommitteeCount(epoch uint64) (uint64, bool, error) { // StartShard returns the start shard number in a given epoch as stored in cache. func (c *CommitteeCache) StartShard(epoch uint64) (uint64, bool, error) { + if !featureconfig.Get().EnableShuffledIndexCache { + return 0, false, nil + } c.lock.RLock() defer c.lock.RUnlock() obj, exists, err := c.CommitteeCache.GetByKey(strconv.Itoa(int(epoch))) @@ -186,6 +205,10 @@ func (c *CommitteeCache) StartShard(epoch uint64) (uint64, bool, error) { // ActiveIndices returns the active indices of a given epoch stored in cache. func (c *CommitteeCache) ActiveIndices(epoch uint64) ([]uint64, error) { + if !featureconfig.Get().EnableShuffledIndexCache { + return nil, nil + } + c.lock.RLock() defer c.lock.RUnlock() obj, exists, err := c.CommitteeCache.GetByKey(strconv.Itoa(int(epoch))) diff --git a/beacon-chain/cache/feature_flag_test.go b/beacon-chain/cache/feature_flag_test.go index 36e9dcac996e..f8977502cfed 100644 --- a/beacon-chain/cache/feature_flag_test.go +++ b/beacon-chain/cache/feature_flag_test.go @@ -4,7 +4,13 @@ import "github.com/prysmaticlabs/prysm/shared/featureconfig" func init() { featureconfig.Init(&featureconfig.Flag{ - EnableAttestationCache: true, - EnableEth1DataVoteCache: true, + EnableAttestationCache: true, + EnableEth1DataVoteCache: true, + EnableShuffledIndexCache: true, + EnableCommitteeCache: true, + EnableCheckpointStateCache: true, + EnableActiveCountCache: true, + EnableFinalizedBlockRootIndex: true, + EnableActiveIndicesCache: true, }) } diff --git a/beacon-chain/cache/shuffled_indices_test.go b/beacon-chain/cache/shuffled_indices_test.go index da2b6fe72009..1d1f32273a90 100644 --- a/beacon-chain/cache/shuffled_indices_test.go +++ b/beacon-chain/cache/shuffled_indices_test.go @@ -4,16 +4,8 @@ import ( "reflect" "strconv" "testing" - - "github.com/prysmaticlabs/prysm/shared/featureconfig" ) -func init() { - fc := featureconfig.Get() - fc.EnableShuffledIndexCache = true - featureconfig.Init(fc) -} - func TestShuffleKeyFn_OK(t *testing.T) { sInfo := &IndicesByIndexSeed{ Index: 999, diff --git a/shared/featureconfig/config.go b/shared/featureconfig/config.go index 0b6d0c3d04aa..5daae2414322 100644 --- a/shared/featureconfig/config.go +++ b/shared/featureconfig/config.go @@ -36,11 +36,15 @@ type Flag struct { EnableFinalizedBlockRootIndex bool // EnableFinalizedBlockRootIndex in the database. This operation may be expensive at runtime. // Cache toggles. - EnableAttestationCache bool // EnableAttestationCache; see https://github.com/prysmaticlabs/prysm/issues/3106. - EnableEth1DataVoteCache bool // EnableEth1DataVoteCache; see https://github.com/prysmaticlabs/prysm/issues/3106. - EnableNewCache bool // EnableNewCache enables the node to use the new caching scheme. - EnableBLSPubkeyCache bool // EnableBLSPubkeyCache to improve wall time of PubkeyFromBytes. - EnableShuffledIndexCache bool // EnableShuffledIndexCache to cache expensive shuffled index computation. + EnableAttestationCache bool // EnableAttestationCache; see https://github.com/prysmaticlabs/prysm/issues/3106. + EnableEth1DataVoteCache bool // EnableEth1DataVoteCache; see https://github.com/prysmaticlabs/prysm/issues/3106. + EnableNewCache bool // EnableNewCache enables the node to use the new caching scheme. + EnableBLSPubkeyCache bool // EnableBLSPubkeyCache to improve wall time of PubkeyFromBytes. + EnableShuffledIndexCache bool // EnableShuffledIndexCache to cache expensive shuffled index computation. + EnableCommitteeCache bool // EnableCommitteeCache to cache committee computation. + EnableCheckpointStateCache bool // EnableCheckpointStateCache. + EnableActiveIndicesCache bool // EnableActiveIndicesCache. + EnableActiveCountCache bool // EnableActiveCountCache. } var featureConfig *Flag @@ -119,6 +123,22 @@ func ConfigureBeaconChain(ctx *cli.Context) { log.Warn("Enabled shuffled index cache.") cfg.EnableShuffledIndexCache = true } + if ctx.GlobalBool(enableCommitteeCacheFlag.Name) { + log.Warn("Enabled committee cache.") + cfg.EnableCommitteeCache = true + } + if ctx.GlobalBool(enableCheckpointStateCacheFlag.Name) { + log.Warn("Enabled checkpoint state cache.") + cfg.EnableCheckpointStateCache = true + } + if ctx.GlobalBool(enableActiveIndicesCacheFlag.Name) { + log.Warn("Enabled active indices cache.") + cfg.EnableActiveIndicesCache = true + } + if ctx.GlobalBool(enableActiveCountCacheFlag.Name) { + log.Warn("Enabled active count cache.") + cfg.EnableActiveCountCache = true + } Init(cfg) } diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index 6410529b0a19..af83c0c70c13 100644 --- a/shared/featureconfig/flags.go +++ b/shared/featureconfig/flags.go @@ -33,6 +33,22 @@ var ( Name: "enable-shuffled-index-cache", Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", } + enableCommitteeCacheFlag = cli.BoolFlag{ + Name: "enable-committee-cache", + Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", + } + enableCheckpointStateCacheFlag = cli.BoolFlag{ + Name: "enable-checkpoint-state-cache", + Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", + } + enableActiveIndicesCacheFlag = cli.BoolFlag{ + Name: "enable-active-indices-cache", + Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", + } + enableActiveCountCacheFlag = cli.BoolFlag{ + Name: "enable-active-count-cache", + Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", + } // InitSyncNoVerifyFlag enables the initial sync no verify configuration. InitSyncNoVerifyFlag = cli.BoolFlag{ Name: "init-sync-no-verify", @@ -104,6 +120,10 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{ enableBackupWebhookFlag, enableBLSPubkeyCacheFlag, enableShuffledIndexCache, + enableCommitteeCacheFlag, + enableCheckpointStateCacheFlag, + enableActiveIndicesCacheFlag, + enableActiveCountCacheFlag, pruneFinalizedStatesFlag, enableFinalizedBlockRootIndexFlag, }...) From 9a8e49b9ddfe850ac1a8c3d7f0941c358eb31ea8 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Wed, 30 Oct 2019 15:36:16 -0700 Subject: [PATCH 4/9] also allow new-cache flag --- beacon-chain/cache/committee.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/beacon-chain/cache/committee.go b/beacon-chain/cache/committee.go index e0859faf31e7..ed17b81c6167 100644 --- a/beacon-chain/cache/committee.go +++ b/beacon-chain/cache/committee.go @@ -68,7 +68,7 @@ func NewCommitteeCache() *CommitteeCache { // ShuffledIndices fetches the shuffled indices by epoch and shard. Every list of indices // represent one committee. Returns true if the list exists with epoch and shard. Otherwise returns false, nil. func (c *CommitteeCache) ShuffledIndices(epoch uint64, shard uint64) ([]uint64, error) { - if !featureconfig.Get().EnableShuffledIndexCache { + if !featureconfig.Get().EnableShuffledIndexCache && !featureconfig.Get().EnableNewCache { return nil, nil } c.lock.RLock() @@ -98,7 +98,7 @@ func (c *CommitteeCache) ShuffledIndices(epoch uint64, shard uint64) ([]uint64, // AddCommitteeShuffledList adds Committee shuffled list object to the cache. T // his method also trims the least recently list if the cache size has ready the max cache size limit. func (c *CommitteeCache) AddCommitteeShuffledList(committee *Committee) error { - if !featureconfig.Get().EnableShuffledIndexCache { + if !featureconfig.Get().EnableShuffledIndexCache && !featureconfig.Get().EnableNewCache { return nil } c.lock.Lock() @@ -131,7 +131,7 @@ func (c *CommitteeCache) Epochs() ([]uint64, error) { // EpochInCache returns true if an input epoch is part of keys in cache. func (c *CommitteeCache) EpochInCache(wantedEpoch uint64) (bool, error) { - if !featureconfig.Get().EnableShuffledIndexCache { + if !featureconfig.Get().EnableShuffledIndexCache && !featureconfig.Get().EnableNewCache { return false, nil } c.lock.RLock() @@ -151,7 +151,7 @@ func (c *CommitteeCache) EpochInCache(wantedEpoch uint64) (bool, error) { // CommitteeCount returns the total number of committees in a given epoch as stored in cache. func (c *CommitteeCache) CommitteeCount(epoch uint64) (uint64, bool, error) { - if !featureconfig.Get().EnableShuffledIndexCache { + if !featureconfig.Get().EnableShuffledIndexCache && !featureconfig.Get().EnableNewCache { return 0, false, nil } c.lock.RLock() @@ -178,7 +178,7 @@ func (c *CommitteeCache) CommitteeCount(epoch uint64) (uint64, bool, error) { // StartShard returns the start shard number in a given epoch as stored in cache. func (c *CommitteeCache) StartShard(epoch uint64) (uint64, bool, error) { - if !featureconfig.Get().EnableShuffledIndexCache { + if !featureconfig.Get().EnableShuffledIndexCache && !featureconfig.Get().EnableNewCache { return 0, false, nil } c.lock.RLock() @@ -205,7 +205,7 @@ func (c *CommitteeCache) StartShard(epoch uint64) (uint64, bool, error) { // ActiveIndices returns the active indices of a given epoch stored in cache. func (c *CommitteeCache) ActiveIndices(epoch uint64) ([]uint64, error) { - if !featureconfig.Get().EnableShuffledIndexCache { + if !featureconfig.Get().EnableShuffledIndexCache && !featureconfig.Get().EnableNewCache { return nil, nil } From 738e87b147b58f4df69ae50c188a08da46fd05eb Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Wed, 30 Oct 2019 17:20:05 -0700 Subject: [PATCH 5/9] gofmt, fix a test --- .../blockchain/forkchoice/process_attestation_test.go | 7 ++++++- beacon-chain/core/epoch/epoch_processing_test.go | 3 +++ beacon-chain/core/helpers/committee_test.go | 7 ++++++- shared/featureconfig/flags.go | 1 + tools/extractor/main.go | 1 - 5 files changed, 16 insertions(+), 3 deletions(-) diff --git a/beacon-chain/blockchain/forkchoice/process_attestation_test.go b/beacon-chain/blockchain/forkchoice/process_attestation_test.go index a4fc02c5f98e..e5395cae3934 100644 --- a/beacon-chain/blockchain/forkchoice/process_attestation_test.go +++ b/beacon-chain/blockchain/forkchoice/process_attestation_test.go @@ -13,6 +13,7 @@ import ( testDB "github.com/prysmaticlabs/prysm/beacon-chain/db/testing" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/testutil" ) @@ -110,7 +111,11 @@ func TestStore_OnAttestation(t *testing.T) { } } -func TestStore_SaveCheckpointState(t *testing.T) { +func TestStore_SaveCheckpointState_CacheEnabled(t *testing.T) { + c := featureconfig.Get() + c.EnableCheckpointStateCache = true + featureconfig.Init(c) + defer featureconfig.Init(nil) ctx := context.Background() db := testDB.SetupDB(t) defer testDB.TeardownDB(t, db) diff --git a/beacon-chain/core/epoch/epoch_processing_test.go b/beacon-chain/core/epoch/epoch_processing_test.go index c1280b5ab3fb..71c2ac1c9bde 100644 --- a/beacon-chain/core/epoch/epoch_processing_test.go +++ b/beacon-chain/core/epoch/epoch_processing_test.go @@ -1118,6 +1118,9 @@ func TestAttestationDelta_CantGetAttestationIndices(t *testing.T) { _, _, err := attestationDelta(state) wanted := "could not get attestation indices" + if err == nil { + t.Fatal("Wanted error, did not get an error") + } if !strings.Contains(err.Error(), wanted) { t.Fatalf("Got: %v, want: %v", err.Error(), wanted) } diff --git a/beacon-chain/core/helpers/committee_test.go b/beacon-chain/core/helpers/committee_test.go index c40e7b00407d..e4da4d18e491 100644 --- a/beacon-chain/core/helpers/committee_test.go +++ b/beacon-chain/core/helpers/committee_test.go @@ -799,6 +799,11 @@ func TestShuffledIndices_ShuffleRightLength(t *testing.T) { } func TestUpdateCommitteeCache_CanUpdate(t *testing.T) { + c := featureconfig.Get() + c.EnableShuffledIndexCache = true + featureconfig.Init(c) + defer featureconfig.Init(nil) + ClearAllCaches() validatorCount := int(params.BeaconConfig().MinGenesisActiveValidatorCount) @@ -833,7 +838,7 @@ func TestUpdateCommitteeCache_CanUpdate(t *testing.T) { t.Fatal(err) } if len(indices) != int(params.BeaconConfig().TargetCommitteeSize) { - t.Error("Did not save correct indices lengths") + t.Errorf("Did not save correct indices lengths, got %d wanted %d", len(indices), params.BeaconConfig().TargetCommitteeSize) } } diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index af83c0c70c13..f8c42c5bbed2 100644 --- a/shared/featureconfig/flags.go +++ b/shared/featureconfig/flags.go @@ -89,6 +89,7 @@ var ( // Deprecated flags list. const deprecatedUsage = "DEPRECATED. DO NOT USE." + var ( deprecatedNoGenesisDelayFlag = cli.BoolFlag{ Name: "no-genesis-delay", diff --git a/tools/extractor/main.go b/tools/extractor/main.go index 8dcec7a64c1b..df7c8144004a 100644 --- a/tools/extractor/main.go +++ b/tools/extractor/main.go @@ -16,7 +16,6 @@ var ( datadir = flag.String("datadir", "", "Path to data directory.") state = flag.Uint("state", 0, "Extract state at this slot.") - ) func init() { From 0df0502826b8d60f3c2f74b3dd74f9daf5730b3c Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Thu, 31 Oct 2019 16:11:19 -0700 Subject: [PATCH 6/9] remove flag for checkpoint state cache, add comment to move it --- .../blockchain/forkchoice/process_attestation_test.go | 7 +------ beacon-chain/cache/checkpoint_state.go | 11 ++--------- beacon-chain/cache/feature_flag_test.go | 1 - shared/featureconfig/config.go | 5 ----- shared/featureconfig/flags.go | 5 ----- 5 files changed, 3 insertions(+), 26 deletions(-) diff --git a/beacon-chain/blockchain/forkchoice/process_attestation_test.go b/beacon-chain/blockchain/forkchoice/process_attestation_test.go index e5395cae3934..a4fc02c5f98e 100644 --- a/beacon-chain/blockchain/forkchoice/process_attestation_test.go +++ b/beacon-chain/blockchain/forkchoice/process_attestation_test.go @@ -13,7 +13,6 @@ import ( testDB "github.com/prysmaticlabs/prysm/beacon-chain/db/testing" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" - "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/testutil" ) @@ -111,11 +110,7 @@ func TestStore_OnAttestation(t *testing.T) { } } -func TestStore_SaveCheckpointState_CacheEnabled(t *testing.T) { - c := featureconfig.Get() - c.EnableCheckpointStateCache = true - featureconfig.Init(c) - defer featureconfig.Init(nil) +func TestStore_SaveCheckpointState(t *testing.T) { ctx := context.Background() db := testDB.SetupDB(t) defer testDB.TeardownDB(t, db) diff --git a/beacon-chain/cache/checkpoint_state.go b/beacon-chain/cache/checkpoint_state.go index bda241bcd94b..4de925f0a928 100644 --- a/beacon-chain/cache/checkpoint_state.go +++ b/beacon-chain/cache/checkpoint_state.go @@ -9,11 +9,12 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" - "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/hashutil" "k8s.io/client-go/tools/cache" ) +// TODO: Move to proper place. + var ( // ErrNotCheckpointState will be returned when a cache object is not a pointer to // a CheckpointState struct. @@ -69,10 +70,6 @@ func NewCheckpointStateCache() *CheckpointStateCache { // StateByCheckpoint fetches state by checkpoint. Returns true with a // reference to the CheckpointState info, if exists. Otherwise returns false, nil. func (c *CheckpointStateCache) StateByCheckpoint(cp *ethpb.Checkpoint) (*pb.BeaconState, error) { - if !featureconfig.Get().EnableCheckpointStateCache { - return nil, nil - } - c.lock.RLock() defer c.lock.RUnlock() h, err := hashutil.HashProto(cp) @@ -103,10 +100,6 @@ func (c *CheckpointStateCache) StateByCheckpoint(cp *ethpb.Checkpoint) (*pb.Beac // AddCheckpointState adds CheckpointState object to the cache. This method also trims the least // recently added CheckpointState object if the cache size has ready the max cache size limit. func (c *CheckpointStateCache) AddCheckpointState(cp *CheckpointState) error { - if !featureconfig.Get().EnableCheckpointStateCache { - return nil - } - c.lock.Lock() defer c.lock.Unlock() if err := c.cache.AddIfNotPresent(cp); err != nil { diff --git a/beacon-chain/cache/feature_flag_test.go b/beacon-chain/cache/feature_flag_test.go index f8977502cfed..82b3f25a10b0 100644 --- a/beacon-chain/cache/feature_flag_test.go +++ b/beacon-chain/cache/feature_flag_test.go @@ -8,7 +8,6 @@ func init() { EnableEth1DataVoteCache: true, EnableShuffledIndexCache: true, EnableCommitteeCache: true, - EnableCheckpointStateCache: true, EnableActiveCountCache: true, EnableFinalizedBlockRootIndex: true, EnableActiveIndicesCache: true, diff --git a/shared/featureconfig/config.go b/shared/featureconfig/config.go index ab40fe2453c7..0dabc567b391 100644 --- a/shared/featureconfig/config.go +++ b/shared/featureconfig/config.go @@ -43,7 +43,6 @@ type Flag struct { EnableShuffledIndexCache bool // EnableShuffledIndexCache to cache expensive shuffled index computation. EnableSkipSlotsCache bool // EnableSkipSlotsCache caches the state in skipped slots. EnableCommitteeCache bool // EnableCommitteeCache to cache committee computation. - EnableCheckpointStateCache bool // EnableCheckpointStateCache. EnableActiveIndicesCache bool // EnableActiveIndicesCache. EnableActiveCountCache bool // EnableActiveCountCache. } @@ -132,10 +131,6 @@ func ConfigureBeaconChain(ctx *cli.Context) { log.Warn("Enabled committee cache.") cfg.EnableCommitteeCache = true } - if ctx.GlobalBool(enableCheckpointStateCacheFlag.Name) { - log.Warn("Enabled checkpoint state cache.") - cfg.EnableCheckpointStateCache = true - } if ctx.GlobalBool(enableActiveIndicesCacheFlag.Name) { log.Warn("Enabled active indices cache.") cfg.EnableActiveIndicesCache = true diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index bc86efcfea7d..c29214fe335f 100644 --- a/shared/featureconfig/flags.go +++ b/shared/featureconfig/flags.go @@ -37,10 +37,6 @@ var ( Name: "enable-committee-cache", Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", } - enableCheckpointStateCacheFlag = cli.BoolFlag{ - Name: "enable-checkpoint-state-cache", - Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", - } enableActiveIndicesCacheFlag = cli.BoolFlag{ Name: "enable-active-indices-cache", Usage: "Enable unsafe cache mechanism. See https://github.com/prysmaticlabs/prysm/issues/3106", @@ -127,7 +123,6 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{ enableBLSPubkeyCacheFlag, enableShuffledIndexCache, enableCommitteeCacheFlag, - enableCheckpointStateCacheFlag, enableActiveIndicesCacheFlag, enableActiveCountCacheFlag, pruneFinalizedStatesFlag, From 55264e1e60c4d459b187276931a67783e5629582 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Thu, 31 Oct 2019 16:16:13 -0700 Subject: [PATCH 7/9] delete test that is going to be deleted soon anyway --- .../core/epoch/epoch_processing_test.go | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/beacon-chain/core/epoch/epoch_processing_test.go b/beacon-chain/core/epoch/epoch_processing_test.go index 71c2ac1c9bde..517ee5f29e21 100644 --- a/beacon-chain/core/epoch/epoch_processing_test.go +++ b/beacon-chain/core/epoch/epoch_processing_test.go @@ -1096,36 +1096,6 @@ func TestAttestationDelta_CantGetAttestation(t *testing.T) { } } -func TestAttestationDelta_CantGetAttestationIndices(t *testing.T) { - e := params.BeaconConfig().SlotsPerEpoch - - state := buildState(e+2, 1) - atts := make([]*pb.PendingAttestation, 2) - for i := 0; i < len(atts); i++ { - atts[i] = &pb.PendingAttestation{ - Data: ðpb.AttestationData{ - Crosslink: ðpb.Crosslink{ - Shard: uint64(i), - }, - Target: ðpb.Checkpoint{}, - Source: ðpb.Checkpoint{}, - }, - InclusionDelay: uint64(i + 100), - AggregationBits: bitfield.Bitlist{0xFF, 0x01}, - } - } - state.PreviousEpochAttestations = atts - - _, _, err := attestationDelta(state) - wanted := "could not get attestation indices" - if err == nil { - t.Fatal("Wanted error, did not get an error") - } - if !strings.Contains(err.Error(), wanted) { - t.Fatalf("Got: %v, want: %v", err.Error(), wanted) - } -} - func TestAttestationDelta_NoOneAttested(t *testing.T) { e := params.BeaconConfig().SlotsPerEpoch validatorCount := params.BeaconConfig().MinGenesisActiveValidatorCount / 32 From 84c91df0ea90310117662885fd4b5459606f70f0 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Thu, 31 Oct 2019 16:18:13 -0700 Subject: [PATCH 8/9] lets move it later --- beacon-chain/cache/checkpoint_state.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/beacon-chain/cache/checkpoint_state.go b/beacon-chain/cache/checkpoint_state.go index 4de925f0a928..c8dd0bf1dbe4 100644 --- a/beacon-chain/cache/checkpoint_state.go +++ b/beacon-chain/cache/checkpoint_state.go @@ -13,8 +13,6 @@ import ( "k8s.io/client-go/tools/cache" ) -// TODO: Move to proper place. - var ( // ErrNotCheckpointState will be returned when a cache object is not a pointer to // a CheckpointState struct. From 4f801ac991c6866e516e4c79ad49c46efbde8585 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Sat, 2 Nov 2019 13:50:04 -0700 Subject: [PATCH 9/9] fix --- beacon-chain/cache/feature_flag_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/beacon-chain/cache/feature_flag_test.go b/beacon-chain/cache/feature_flag_test.go index 82b3f25a10b0..6710a4703216 100644 --- a/beacon-chain/cache/feature_flag_test.go +++ b/beacon-chain/cache/feature_flag_test.go @@ -9,7 +9,6 @@ func init() { EnableShuffledIndexCache: true, EnableCommitteeCache: true, EnableActiveCountCache: true, - EnableFinalizedBlockRootIndex: true, EnableActiveIndicesCache: true, }) }