Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
danuio committed May 25, 2021
1 parent 0c361f6 commit 9e2a2b6
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 33 deletions.
5 changes: 5 additions & 0 deletions module/mock/compliance_metrics.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions network/mocknetwork/message_processor.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions state/protocol/badger/mutator.go
Expand Up @@ -571,6 +571,11 @@ func (m *FollowerState) Finalize(blockID flow.Identifier) error {

// FOURTH: metrics and events

if epochStatus.NextEpoch.SetupID != flow.ZeroID {
// true iff EpochSetup event for NEXT epoch was already included before
m.metrics.CommittedEpochFinalView(setup.FinalView)
}

m.metrics.FinalizedHeight(header.Height)
m.metrics.SealedHeight(sealed.Height)
m.metrics.BlockFinalized(block)
Expand Down Expand Up @@ -775,9 +780,6 @@ func (m *FollowerState) handleServiceEvents(block *flow.Block) ([]func(*transact
return nil, state.NewInvalidExtensionErrorf("invalid epoch commit: %s", err)
}

// update `committed_epoch_final_view` metric with new final view
m.metrics.CommittedEpochFinalView(setup.FinalView)

// prevents multiple setup events for same Epoch (including multiple setup events in payload of same block)
epochStatus.NextEpoch.CommitID = ev.ID()

Expand Down
69 changes: 42 additions & 27 deletions state/protocol/badger/state.go
Expand Up @@ -52,33 +52,6 @@ func Bootstrap(
}
state := newState(metrics, db, headers, seals, results, blocks, setups, commits, statuses)

phase, err := state.Final().Phase()
if err != nil {
return nil, fmt.Errorf("could not get epoch phase from state: %w", err)
}

// update metric based of epoch phase
switch phase {
case flow.EpochPhaseStaking, flow.EpochPhaseSetup:

// if we are in Staking or Setup phase, then set the metric value to the current epoch's final view
finalView, err := state.Final().Epochs().Current().FinalView()
if err != nil {
return nil, fmt.Errorf("could not get current epoch final view from state: %w", err)
}
state.metrics.CommittedEpochFinalView(finalView)
case flow.EpochPhaseCommitted:

// if we are in Committed phase, then set the metric value to the next epoch's final view
finalView, err := state.Final().Epochs().Next().FinalView()
if err != nil {
return nil, fmt.Errorf("could not get next epoch final view from state: %w", err)
}
state.metrics.CommittedEpochFinalView(finalView)
default:
return nil, fmt.Errorf("invalid phase: %s", phase)
}

if err := isValidRootSnapshot(root); err != nil {
return nil, fmt.Errorf("cannot bootstrap invalid root snapshot: %w", err)
}
Expand Down Expand Up @@ -416,6 +389,12 @@ func OpenState(
}
state := newState(metrics, db, headers, seals, results, blocks, setups, commits, statuses)

// update committed final view metric
err = updateCommittedEpochFinalView(state)
if err != nil {
return nil, fmt.Errorf("failed to update committed epoch final view: %w", err)
}

return state, nil
}

Expand Down Expand Up @@ -503,3 +482,39 @@ func IsBootstrapped(db *badger.DB) (bool, error) {
}
return true, nil
}

// updateCommittedEpochFinalView updates the `committed_epoch_final_view` metric based on
// the current epoch phase. For example, suppose we have epochs N and N+1.
// If we are in epoch N's Setup Phase, then epoch N's final view should be the value of the metric.
// If we are in epoch N's Committed Phase, then epoch N+1's final view should be the value of the metric.
func updateCommittedEpochFinalView(state *State) error {

phase, err := state.Final().Phase()
if err != nil {
return fmt.Errorf("could not get epoch phase from state: %w", err)
}

// update metric based of epoch phase
switch phase {
case flow.EpochPhaseStaking, flow.EpochPhaseSetup:

// if we are in Staking or Setup phase, then set the metric value to the current epoch's final view
finalView, err := state.Final().Epochs().Current().FinalView()
if err != nil {
return fmt.Errorf("could not get current epoch final view from state: %w", err)
}
state.metrics.CommittedEpochFinalView(finalView)
case flow.EpochPhaseCommitted:

// if we are in Committed phase, then set the metric value to the next epoch's final view
finalView, err := state.Final().Epochs().Next().FinalView()
if err != nil {
return fmt.Errorf("could not get next epoch final view from state: %w", err)
}
state.metrics.CommittedEpochFinalView(finalView)
default:
return fmt.Errorf("invalid phase: %s", phase)
}

return nil
}
8 changes: 5 additions & 3 deletions state/protocol/badger/state_test.go
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/onflow/flow-go/model/flow"
"github.com/onflow/flow-go/module/metrics"
mock "github.com/onflow/flow-go/module/mock"
"github.com/onflow/flow-go/state/protocol"
bprotocol "github.com/onflow/flow-go/state/protocol/badger"
"github.com/onflow/flow-go/state/protocol/inmem"
Expand All @@ -33,10 +34,11 @@ func TestBootstrapAndOpen(t *testing.T) {

protoutil.RunWithBootstrapState(t, rootSnapshot, func(db *badger.DB, _ *bprotocol.State) {
// protocol state has been bootstrapped, now open a protocol state with the database
metrics := new(metrics.NoopCollector)
all := storagebadger.InitAll(metrics, db)
complianceMetrics := mock.ComplianceMetrics{}
noopMetrics := new(metrics.NoopCollector)
all := storagebadger.InitAll(noopMetrics, db)
state, err := bprotocol.OpenState(
metrics,
complianceMetrics,
db,
all.Headers,
all.Seals,
Expand Down

0 comments on commit 9e2a2b6

Please sign in to comment.