Skip to content

Commit

Permalink
Defer Snowman Bootstrapper parser initialization to Start (ava-labs#1442
Browse files Browse the repository at this point in the history
)
  • Loading branch information
StephenButtolph authored Apr 28, 2023
1 parent 188049c commit b007bdd
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 54 deletions.
3 changes: 0 additions & 3 deletions chains/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,6 @@ func (m *manager) createAvalancheChain(
VM: vmWrappingProposerVM,
}
snowmanBootstrapper, err := smbootstrap.New(
context.TODO(),
bootstrapCfg,
snowmanEngine.Start,
)
Expand Down Expand Up @@ -939,7 +938,6 @@ func (m *manager) createAvalancheChain(
}

avalancheBootstrapper, err := avbootstrap.New(
context.TODO(),
avalancheBootstrapperConfig,
snowmanBootstrapper.Start,
)
Expand Down Expand Up @@ -1223,7 +1221,6 @@ func (m *manager) createSnowmanChain(
Bootstrapped: bootstrapFunc,
}
bootstrapper, err := smbootstrap.New(
context.TODO(),
bootstrapCfg,
engine.Start,
)
Expand Down
39 changes: 20 additions & 19 deletions snow/engine/avalanche/bootstrap/bootstrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const (
var _ common.BootstrapableEngine = (*bootstrapper)(nil)

func New(
ctx context.Context,
config Config,
onFinished func(ctx context.Context, lastReqID uint32) error,
) (common.BootstrapableEngine, error) {
Expand All @@ -57,29 +56,13 @@ func New(
return nil, err
}

if err := b.VtxBlocked.SetParser(ctx, &vtxParser{
log: config.Ctx.Log,
numAccepted: b.numAcceptedVts,
numDropped: b.numDroppedVts,
manager: b.Manager,
}); err != nil {
return nil, err
}

if err := b.TxBlocked.SetParser(&txParser{
log: config.Ctx.Log,
numAccepted: b.numAcceptedTxs,
numDropped: b.numDroppedTxs,
vm: b.VM,
}); err != nil {
return nil, err
}

config.Config.Bootstrapable = b
b.Bootstrapper = common.NewCommonBootstrapper(config.Config)
return b, nil
}

// Note: To align with the Snowman invariant, it should be guaranteed the VM is
// not used until after the bootstrapper has been Started.
type bootstrapper struct {
Config

Expand Down Expand Up @@ -315,6 +298,24 @@ func (b *bootstrapper) Start(ctx context.Context, startReqID uint32) error {
err)
}

if err := b.VtxBlocked.SetParser(ctx, &vtxParser{
log: b.Ctx.Log,
numAccepted: b.numAcceptedVts,
numDropped: b.numDroppedVts,
manager: b.Manager,
}); err != nil {
return err
}

if err := b.TxBlocked.SetParser(&txParser{
log: b.Ctx.Log,
numAccepted: b.numAcceptedTxs,
numDropped: b.numDroppedTxs,
vm: b.VM,
}); err != nil {
return err
}

b.Config.SharedCfg.RequestID = startReqID

// If the network was already linearized, don't attempt to linearize it
Expand Down
7 changes: 0 additions & 7 deletions snow/engine/avalanche/bootstrap/bootstrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ func TestBootstrapperSingleFrontier(t *testing.T) {
}

bs, err := New(
context.Background(),
config,
func(context.Context, uint32) error {
config.Ctx.State.Set(snow.EngineState{
Expand Down Expand Up @@ -253,7 +252,6 @@ func TestBootstrapperByzantineResponses(t *testing.T) {
}

bs, err := New(
context.Background(),
config,
func(context.Context, uint32) error {
config.Ctx.State.Set(snow.EngineState{
Expand Down Expand Up @@ -421,7 +419,6 @@ func TestBootstrapperTxDependencies(t *testing.T) {
}

bs, err := New(
context.Background(),
config,
func(context.Context, uint32) error {
config.Ctx.State.Set(snow.EngineState{
Expand Down Expand Up @@ -548,7 +545,6 @@ func TestBootstrapperIncompleteAncestors(t *testing.T) {
}

bs, err := New(
context.Background(),
config,
func(context.Context, uint32) error {
config.Ctx.State.Set(snow.EngineState{
Expand Down Expand Up @@ -660,7 +656,6 @@ func TestBootstrapperFinalized(t *testing.T) {
}

bs, err := New(
context.Background(),
config,
func(context.Context, uint32) error {
config.Ctx.State.Set(snow.EngineState{
Expand Down Expand Up @@ -788,7 +783,6 @@ func TestBootstrapperAcceptsAncestorsParents(t *testing.T) {
}

bs, err := New(
context.Background(),
config,
func(context.Context, uint32) error {
config.Ctx.State.Set(snow.EngineState{
Expand Down Expand Up @@ -953,7 +947,6 @@ func TestRestartBootstrapping(t *testing.T) {
}

bsIntf, err := New(
context.Background(),
config,
func(context.Context, uint32) error {
config.Ctx.State.Set(snow.EngineState{
Expand Down
2 changes: 0 additions & 2 deletions snow/engine/snowman/bootstrap/block_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ func (p *parser) Parse(ctx context.Context, blkBytes []byte) (queue.Job, error)
return nil, err
}
return &blockJob{
parser: p,
log: p.log,
numAccepted: p.numAccepted,
numDropped: p.numDropped,
Expand All @@ -45,7 +44,6 @@ func (p *parser) Parse(ctx context.Context, blkBytes []byte) (queue.Job, error)
}

type blockJob struct {
parser *parser
log logging.Logger
numAccepted, numDropped prometheus.Counter
blk snowman.Block
Expand Down
25 changes: 13 additions & 12 deletions snow/engine/snowman/bootstrap/bootstrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ var (
errUnexpectedTimeout = errors.New("unexpected timeout fired")
)

// Invariant: The VM is not guaranteed to be initialized until Start has been
// called, so it must be guaranteed the VM is not used until after Start.
type bootstrapper struct {
Config

Expand Down Expand Up @@ -81,7 +83,7 @@ type bootstrapper struct {
bootstrappedOnce sync.Once
}

func New(ctx context.Context, config Config, onFinished func(ctx context.Context, lastReqID uint32) error) (common.BootstrapableEngine, error) {
func New(config Config, onFinished func(ctx context.Context, lastReqID uint32) error) (common.BootstrapableEngine, error) {
metrics, err := newMetrics("bs", config.Ctx.Registerer)
if err != nil {
return nil, err
Expand All @@ -103,16 +105,6 @@ func New(ctx context.Context, config Config, onFinished func(ctx context.Context
executedStateTransitions: math.MaxInt32,
}

b.parser = &parser{
log: config.Ctx.Log,
numAccepted: b.numAccepted,
numDropped: b.numDropped,
vm: b.VM,
}
if err := b.Blocked.SetParser(ctx, b.parser); err != nil {
return nil, err
}

config.Bootstrapable = b
b.Bootstrapper = common.NewCommonBootstrapper(config.Config)

Expand All @@ -131,6 +123,16 @@ func (b *bootstrapper) Start(ctx context.Context, startReqID uint32) error {
err)
}

b.parser = &parser{
log: b.Ctx.Log,
numAccepted: b.numAccepted,
numDropped: b.numDropped,
vm: b.VM,
}
if err := b.Blocked.SetParser(ctx, b.parser); err != nil {
return err
}

// Set the starting height
lastAcceptedID, err := b.VM.LastAccepted(ctx)
if err != nil {
Expand Down Expand Up @@ -456,7 +458,6 @@ func (b *bootstrapper) process(ctx context.Context, blk snowman.Block, processin
}

pushed, err := b.Blocked.Push(ctx, &blockJob{
parser: b.parser,
log: b.Ctx.Log,
numAccepted: b.numAccepted,
numDropped: b.numDropped,
Expand Down
Loading

0 comments on commit b007bdd

Please sign in to comment.