From 07151d458f9c636d03d5658cf6f28d928ab339d4 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Wed, 27 May 2020 21:37:03 -0700 Subject: [PATCH 1/5] Test --- beacon-chain/cache/attestation_data.go | 2 +- beacon-chain/rpc/validator/attester.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/beacon-chain/cache/attestation_data.go b/beacon-chain/cache/attestation_data.go index 1c2e20589d5f..57d1c13a61c6 100644 --- a/beacon-chain/cache/attestation_data.go +++ b/beacon-chain/cache/attestation_data.go @@ -162,7 +162,7 @@ func wrapperToKey(i interface{}) (string, error) { } func reqToKey(req *ethpb.AttestationDataRequest) (string, error) { - return fmt.Sprintf("%d-%d", req.CommitteeIndex, req.Slot), nil + return fmt.Sprintf("%d", req.Slot), nil } type attestationReqResWrapper struct { diff --git a/beacon-chain/rpc/validator/attester.go b/beacon-chain/rpc/validator/attester.go index 50e258b40082..6dd529040b38 100644 --- a/beacon-chain/rpc/validator/attester.go +++ b/beacon-chain/rpc/validator/attester.go @@ -47,6 +47,7 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation return nil, status.Errorf(codes.Internal, "Could not retrieve data from attestation cache: %v", err) } if res != nil { + res.CommitteeIndex = req.CommitteeIndex return res, nil } @@ -59,6 +60,7 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation if res == nil { return nil, status.Error(codes.DataLoss, "A request was in progress and resolved to nil") } + res.CommitteeIndex = req.CommitteeIndex return res, nil } return nil, status.Errorf(codes.Internal, "Could not mark attestation as in-progress: %v", err) From 2ecdb8941c97fcbacdb6d30c37cc7ac7f15c22ee Mon Sep 17 00:00:00 2001 From: terence tsao Date: Wed, 27 May 2020 22:10:11 -0700 Subject: [PATCH 2/5] Fix --- beacon-chain/cache/attestation_data.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beacon-chain/cache/attestation_data.go b/beacon-chain/cache/attestation_data.go index 57d1c13a61c6..69c02e5b0de3 100644 --- a/beacon-chain/cache/attestation_data.go +++ b/beacon-chain/cache/attestation_data.go @@ -11,6 +11,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" + "github.com/prysmaticlabs/prysm/beacon-chain/state" "k8s.io/client-go/tools/cache" ) @@ -97,7 +98,8 @@ func (c *AttestationCache) Get(ctx context.Context, req *ethpb.AttestationDataRe if exists && item != nil && item.(*attestationReqResWrapper).res != nil { attestationCacheHit.Inc() - return item.(*attestationReqResWrapper).res, nil + state.CopyAttestationData(item.(*attestationReqResWrapper).res) + return state.CopyAttestationData(item.(*attestationReqResWrapper).res), nil } attestationCacheMiss.Inc() return nil, nil From a53659437751ca40ab1f3fa6c6e832ad7a1c8057 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Thu, 28 May 2020 06:53:03 -0700 Subject: [PATCH 3/5] Removed extra line --- beacon-chain/cache/attestation_data.go | 1 - 1 file changed, 1 deletion(-) diff --git a/beacon-chain/cache/attestation_data.go b/beacon-chain/cache/attestation_data.go index 69c02e5b0de3..bbb2204e5259 100644 --- a/beacon-chain/cache/attestation_data.go +++ b/beacon-chain/cache/attestation_data.go @@ -98,7 +98,6 @@ func (c *AttestationCache) Get(ctx context.Context, req *ethpb.AttestationDataRe if exists && item != nil && item.(*attestationReqResWrapper).res != nil { attestationCacheHit.Inc() - state.CopyAttestationData(item.(*attestationReqResWrapper).res) return state.CopyAttestationData(item.(*attestationReqResWrapper).res), nil } attestationCacheMiss.Inc() From 5d6ffad7aacfccd7c6927a259e2fb643c661029b Mon Sep 17 00:00:00 2001 From: terence tsao Date: Thu, 28 May 2020 09:43:37 -0700 Subject: [PATCH 4/5] Feature flag --- beacon-chain/cache/attestation_data.go | 11 +++++++++-- beacon-chain/rpc/validator/attester.go | 8 ++++++-- shared/featureconfig/config.go | 5 +++++ shared/featureconfig/flags.go | 7 +++++++ 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/beacon-chain/cache/attestation_data.go b/beacon-chain/cache/attestation_data.go index bbb2204e5259..243c56489cea 100644 --- a/beacon-chain/cache/attestation_data.go +++ b/beacon-chain/cache/attestation_data.go @@ -12,6 +12,7 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" "github.com/prysmaticlabs/prysm/beacon-chain/state" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "k8s.io/client-go/tools/cache" ) @@ -98,7 +99,10 @@ func (c *AttestationCache) Get(ctx context.Context, req *ethpb.AttestationDataRe if exists && item != nil && item.(*attestationReqResWrapper).res != nil { attestationCacheHit.Inc() - return state.CopyAttestationData(item.(*attestationReqResWrapper).res), nil + if featureconfig.Get().ReduceAttesterStateCopy { + return state.CopyAttestationData(item.(*attestationReqResWrapper).res), nil + } + return item.(*attestationReqResWrapper).res, nil } attestationCacheMiss.Inc() return nil, nil @@ -163,7 +167,10 @@ func wrapperToKey(i interface{}) (string, error) { } func reqToKey(req *ethpb.AttestationDataRequest) (string, error) { - return fmt.Sprintf("%d", req.Slot), nil + if featureconfig.Get().ReduceAttesterStateCopy { + return fmt.Sprintf("%d", req.Slot), nil + } + return fmt.Sprintf("%d-%d", req.CommitteeIndex, req.Slot), nil } type attestationReqResWrapper struct { diff --git a/beacon-chain/rpc/validator/attester.go b/beacon-chain/rpc/validator/attester.go index 6dd529040b38..62af39aa4114 100644 --- a/beacon-chain/rpc/validator/attester.go +++ b/beacon-chain/rpc/validator/attester.go @@ -47,7 +47,9 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation return nil, status.Errorf(codes.Internal, "Could not retrieve data from attestation cache: %v", err) } if res != nil { - res.CommitteeIndex = req.CommitteeIndex + if featureconfig.Get().ReduceAttesterStateCopy { + res.CommitteeIndex = req.CommitteeIndex + } return res, nil } @@ -60,7 +62,9 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation if res == nil { return nil, status.Error(codes.DataLoss, "A request was in progress and resolved to nil") } - res.CommitteeIndex = req.CommitteeIndex + if featureconfig.Get().ReduceAttesterStateCopy { + res.CommitteeIndex = req.CommitteeIndex + } return res, nil } return nil, status.Errorf(codes.Internal, "Could not mark attestation as in-progress: %v", err) diff --git a/shared/featureconfig/config.go b/shared/featureconfig/config.go index 4ac9af4de2c9..4a87e31cb2b6 100644 --- a/shared/featureconfig/config.go +++ b/shared/featureconfig/config.go @@ -58,6 +58,7 @@ type Flags struct { WaitForSynced bool // WaitForSynced uses WaitForSynced in validator startup to ensure it can communicate with the beacon node as soon as possible. SkipRegenHistoricalStates bool // SkipRegenHistoricalState skips regenerating historical states from genesis to last finalized. This enables a quick switch over to using new-state-mgmt. EnableInitSyncWeightedRoundRobin bool // EnableInitSyncWeightedRoundRobin enables weighted round robin fetching optimization in initial syncing. + ReduceAttesterStateCopy bool // ReduceAttesterStateCopy reduces head state copies for attester rpc. // DisableForkChoice disables using LMD-GHOST fork choice to update // the head of the chain based on attestations and instead accepts any valid received block @@ -214,6 +215,10 @@ func ConfigureBeaconChain(ctx *cli.Context) { log.Warn("Disabling state reference copy") cfg.EnableStateRefCopy = false } + if ctx.Bool(reduceAttesterStateCopy.Name) { + log.Warn("Enabling feature that reduces attester state copy") + cfg.ReduceAttesterStateCopy = true + } Init(cfg) } diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index e7e18d3326f1..3b706f8befbf 100644 --- a/shared/featureconfig/flags.go +++ b/shared/featureconfig/flags.go @@ -156,6 +156,10 @@ var ( Name: "enable-init-sync-wrr", Usage: "Enables weighted round robin fetching optimization", } + reduceAttesterStateCopy = &cli.BoolFlag{ + Name: "reduce-attester-state-copy", + Usage: "Reduces the amount of state copies for attester rpc", + } ) // devModeFlags holds list of flags that are set when development mode is on. @@ -163,6 +167,7 @@ var devModeFlags = []cli.Flag{ enableFieldTrie, enableNewStateMgmt, enableInitSyncWeightedRoundRobin, + reduceAttesterStateCopy, } // Deprecated flags list. @@ -476,6 +481,7 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{ skipRegenHistoricalStates, enableInitSyncWeightedRoundRobin, disableStateRefCopy, + reduceAttesterStateCopy, }...) // E2EBeaconChainFlags contains a list of the beacon chain feature flags to be tested in E2E. @@ -486,4 +492,5 @@ var E2EBeaconChainFlags = []string{ "--enable-state-field-trie", "--enable-new-state-mgmt", "--enable-init-sync-wrr", + "--reduce-attester-state-copy", } From 885c33b59b2a1c6727a2a0346c93fb2803823172 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Thu, 28 May 2020 09:44:42 -0700 Subject: [PATCH 5/5] Gaz --- beacon-chain/cache/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon-chain/cache/BUILD.bazel b/beacon-chain/cache/BUILD.bazel index 1ed1d377d416..70735c02b6fb 100644 --- a/beacon-chain/cache/BUILD.bazel +++ b/beacon-chain/cache/BUILD.bazel @@ -23,6 +23,7 @@ go_library( deps = [ "//beacon-chain/state:go_default_library", "//proto/beacon/p2p/v1:go_default_library", + "//shared/featureconfig:go_default_library", "//shared/hashutil:go_default_library", "//shared/params:go_default_library", "//shared/sliceutil:go_default_library",