cleanup and add some tests

chain/sync_manager.go

@@ -26,11 +26,16 @@ type SyncManager struct {
 
 	syncStates []*SyncerState
 
-	activeSyncs map[types.TipSetKey]*types.TipSet
-
 	doSync func(context.Context, *types.TipSet) error
 
 	stop chan struct{}
+
+	// Sync Scheduler fields
+	activeSyncs    map[types.TipSetKey]*types.TipSet
+	syncQueue      syncBucketSet
+	activeSyncTips syncBucketSet
+	nextSyncTarget *syncTargetBucket
+	workerChan     chan *types.TipSet
 }
 
 type syncResult struct {
@@ -236,11 +241,6 @@ func (sm *SyncManager) selectSyncTarget() (*types.TipSet, error) {
 }
 
 func (sm *SyncManager) syncScheduler() {
-	var syncQueue syncBucketSet
-	var activeSyncTips syncBucketSet
-
-	var nextSyncTarget *syncTargetBucket
-	var workerChan chan *types.TipSet
 
 	for {
 		select {
@@ -250,67 +250,11 @@ func (sm *SyncManager) syncScheduler() {
 				return
 			}
 
-			var relatedToActiveSync bool
+			sm.scheduleIncoming(ts)
-			for _, acts := range sm.activeSyncs {
-				if ts.Equals(acts) {
-					break
-				}
-
-				if types.CidArrsEqual(ts.Parents(), acts.Cids()) {
-					// sync this next, after that sync process finishes
-					relatedToActiveSync = true
-				}
-			}
-
-			// if this is related to an active sync process, immediately bucket it
-			// we don't want to start a parallel sync process that duplicates work
-			if relatedToActiveSync {
-				log.Info("related to active sync")
-				activeSyncTips.Insert(ts)
-				continue
-			}
-
-			if nextSyncTarget != nil && nextSyncTarget.sameChainAs(ts) {
-				log.Info("new tipset is part of our next sync target")
-				nextSyncTarget.add(ts)
-			} else {
-				log.Info("insert into that queue!")
-				syncQueue.Insert(ts)
-
-				if nextSyncTarget == nil {
-					nextSyncTarget = syncQueue.Pop()
-					workerChan = sm.syncTargets
-					log.Info("setting next sync target")
-				}
-			}
 		case res := <-sm.syncResults:
-			delete(sm.activeSyncs, res.ts.Key())
+			sm.scheduleProcessResult(res)
-			relbucket := activeSyncTips.PopRelated(res.ts)
+		case sm.workerChan <- sm.nextSyncTarget.heaviestTipSet():
-			if relbucket != nil {
+			sm.scheduleWorkSent()
-				if res.success {
-					if nextSyncTarget == nil {
-						nextSyncTarget = relbucket
-						workerChan = sm.syncTargets
-					} else {
-						syncQueue.buckets = append(syncQueue.buckets, relbucket)
-					}
-				} else {
-					// TODO: this is the case where we try to sync a chain, and
-					// fail, and we have more blocks on top of that chain that
-					// have come in since.  The question is, should we try to
-					// sync these? or just drop them?
-				}
-			}
-		case workerChan <- nextSyncTarget.heaviestTipSet():
-			hts := nextSyncTarget.heaviestTipSet()
-			sm.activeSyncs[hts.Key()] = hts
-
-			if len(syncQueue.buckets) > 0 {
-				nextSyncTarget = syncQueue.Pop()
-			} else {
-				nextSyncTarget = nil
-				workerChan = nil
-			}
 		case <-sm.stop:
 			log.Info("sync scheduler shutting down")
 			return
@@ -318,6 +262,70 @@ func (sm *SyncManager) syncScheduler() {
 	}
 }
 
+func (sm *SyncManager) scheduleIncoming(ts *types.TipSet) {
+	var relatedToActiveSync bool
+	for _, acts := range sm.activeSyncs {
+		if ts.Equals(acts) {
+			break
+		}
+
+		if types.CidArrsEqual(ts.Parents(), acts.Cids()) {
+			// sync this next, after that sync process finishes
+			relatedToActiveSync = true
+		}
+	}
+
+	// if this is related to an active sync process, immediately bucket it
+	// we don't want to start a parallel sync process that duplicates work
+	if relatedToActiveSync {
+		sm.activeSyncTips.Insert(ts)
+		return
+	}
+
+	if sm.nextSyncTarget != nil && sm.nextSyncTarget.sameChainAs(ts) {
+		sm.nextSyncTarget.add(ts)
+	} else {
+		sm.syncQueue.Insert(ts)
+
+		if sm.nextSyncTarget == nil {
+			sm.nextSyncTarget = sm.syncQueue.Pop()
+			sm.workerChan = sm.syncTargets
+		}
+	}
+}
+
+func (sm *SyncManager) scheduleProcessResult(res *syncResult) {
+	delete(sm.activeSyncs, res.ts.Key())
+	relbucket := sm.activeSyncTips.PopRelated(res.ts)
+	if relbucket != nil {
+		if res.success {
+			if sm.nextSyncTarget == nil {
+				sm.nextSyncTarget = relbucket
+				sm.workerChan = sm.syncTargets
+			} else {
+				sm.syncQueue.buckets = append(sm.syncQueue.buckets, relbucket)
+			}
+		} else {
+			// TODO: this is the case where we try to sync a chain, and
+			// fail, and we have more blocks on top of that chain that
+			// have come in since.  The question is, should we try to
+			// sync these? or just drop them?
+		}
+	}
+}
+
+func (sm *SyncManager) scheduleWorkSent() {
+	hts := sm.nextSyncTarget.heaviestTipSet()
+	sm.activeSyncs[hts.Key()] = hts
+
+	if len(sm.syncQueue.buckets) > 0 {
+		sm.nextSyncTarget = sm.syncQueue.Pop()
+	} else {
+		sm.nextSyncTarget = nil
+		sm.workerChan = nil
+	}
+}
+
 func (sm *SyncManager) syncWorker(id int) {
 	ss := &SyncerState{}
 	sm.syncStates[id] = ss

chain/sync_manager_test.go

@@ -0,0 +1,123 @@
+package chain
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/filecoin-project/lotus/chain/types"
+	"github.com/filecoin-project/lotus/chain/types/mock"
+)
+
+var genTs = mock.TipSet(mock.MkBlock(nil, 0, 0))
+
+type syncOp struct {
+	ts   *types.TipSet
+	done func()
+}
+
+func runSyncMgrTest(t *testing.T, tname string, thresh int, tf func(*testing.T, *SyncManager, chan *syncOp)) {
+	syncTargets := make(chan *syncOp)
+	sm := NewSyncManager(func(ctx context.Context, ts *types.TipSet) error {
+		ch := make(chan struct{})
+		syncTargets <- &syncOp{
+			ts:   ts,
+			done: func() { close(ch) },
+		}
+		<-ch
+		return nil
+	})
+	sm.bspThresh = thresh
+
+	sm.Start()
+	defer sm.Stop()
+	t.Run(tname+fmt.Sprintf("-%d", thresh), func(t *testing.T) {
+		tf(t, sm, syncTargets)
+	})
+}
+
+func assertTsEqual(t *testing.T, actual, expected *types.TipSet) {
+	t.Helper()
+	if !actual.Equals(expected) {
+		t.Fatalf("got unexpected tipset %s (expected: %s)", actual.Cids(), expected.Cids())
+	}
+}
+
+func assertNoOp(t *testing.T, c chan *syncOp) {
+	t.Helper()
+	select {
+	case <-time.After(time.Millisecond * 20):
+	case <-c:
+		t.Fatal("shouldnt have gotten any sync operations yet")
+	}
+}
+
+func assertGetSyncOp(t *testing.T, c chan *syncOp, ts *types.TipSet) {
+	t.Helper()
+
+	select {
+	case <-time.After(time.Millisecond * 100):
+		t.Fatal("expected sync manager to try and sync to our target")
+	case op := <-c:
+		op.done()
+		if !op.ts.Equals(ts) {
+			t.Fatalf("somehow got wrong tipset from syncer (got %s, expected %s)", op.ts.Cids(), ts.Cids())
+		}
+	}
+}
+
+func TestSyncManager(t *testing.T) {
+	ctx := context.Background()
+
+	a := mock.TipSet(mock.MkBlock(genTs, 1, 1))
+	b := mock.TipSet(mock.MkBlock(a, 1, 2))
+	c1 := mock.TipSet(mock.MkBlock(b, 1, 3))
+	c2 := mock.TipSet(mock.MkBlock(b, 2, 4))
+	d := mock.TipSet(mock.MkBlock(c1, 4, 5))
+
+	runSyncMgrTest(t, "testBootstrap", 1, func(t *testing.T, sm *SyncManager, stc chan *syncOp) {
+		sm.SetPeerHead(ctx, "peer1", c1)
+		assertGetSyncOp(t, stc, c1)
+	})
+
+	runSyncMgrTest(t, "testBootstrap", 2, func(t *testing.T, sm *SyncManager, stc chan *syncOp) {
+		sm.SetPeerHead(ctx, "peer1", c1)
+		assertNoOp(t, stc)
+
+		sm.SetPeerHead(ctx, "peer2", c1)
+		assertGetSyncOp(t, stc, c1)
+	})
+
+	runSyncMgrTest(t, "testSyncAfterBootstrap", 1, func(t *testing.T, sm *SyncManager, stc chan *syncOp) {
+		sm.SetPeerHead(ctx, "peer1", b)
+		assertGetSyncOp(t, stc, b)
+
+		sm.SetPeerHead(ctx, "peer2", c1)
+		assertGetSyncOp(t, stc, c1)
+
+		sm.SetPeerHead(ctx, "peer2", c2)
+		assertGetSyncOp(t, stc, c2)
+	})
+
+	runSyncMgrTest(t, "testCoalescing", 1, func(t *testing.T, sm *SyncManager, stc chan *syncOp) {
+		sm.SetPeerHead(ctx, "peer1", a)
+		assertGetSyncOp(t, stc, a)
+
+		sm.SetPeerHead(ctx, "peer2", b)
+		op := <-stc
+
+		sm.SetPeerHead(ctx, "peer2", c1)
+		sm.SetPeerHead(ctx, "peer2", c2)
+		sm.SetPeerHead(ctx, "peer2", d)
+
+		assertTsEqual(t, op.ts, b)
+
+		// need a better way to 'wait until syncmgr is idle'
+		time.Sleep(time.Millisecond * 20)
+
+		op.done()
+
+		assertGetSyncOp(t, stc, d)
+	})
+}

chain/types/mock/chain.go

@@ -0,0 +1,61 @@
+package mock
+
+import (
+	"fmt"
+
+	"github.com/filecoin-project/lotus/chain/address"
+	"github.com/filecoin-project/lotus/chain/types"
+	"github.com/ipfs/go-cid"
+)
+
+func Address(i uint64) address.Address {
+	a, err := address.NewIDAddress(i)
+	if err != nil {
+		panic(err)
+	}
+	return a
+}
+
+func MkBlock(parents *types.TipSet, weightInc uint64, ticketNonce uint64) *types.BlockHeader {
+	addr := Address(123561)
+
+	c, err := cid.Decode("bafyreicmaj5hhoy5mgqvamfhgexxyergw7hdeshizghodwkjg6qmpoco7i")
+	if err != nil {
+		panic(err)
+	}
+
+	var pcids []cid.Cid
+	var height uint64
+	weight := types.NewInt(weightInc)
+	if parents != nil {
+		pcids = parents.Cids()
+		height = parents.Height() + 1
+		weight = types.BigAdd(parents.Blocks()[0].ParentWeight, weight)
+	}
+
+	return &types.BlockHeader{
+		Miner:         addr,
+		ElectionProof: []byte("cats won the election"),
+		Tickets: []*types.Ticket{
+			{
+				VRFProof: []byte(fmt.Sprintf("====%d=====", ticketNonce)),
+			},
+		},
+		Parents:               pcids,
+		ParentMessageReceipts: c,
+		BLSAggregate:          types.Signature{Type: types.KTBLS, Data: []byte("boo! im a signature")},
+		ParentWeight:          weight,
+		Messages:              c,
+		Height:                height,
+		ParentStateRoot:       c,
+		BlockSig:              types.Signature{Type: types.KTBLS, Data: []byte("boo! im a signature")},
+	}
+}
+
+func TipSet(blks ...*types.BlockHeader) *types.TipSet {
+	ts, err := types.NewTipSet(blks)
+	if err != nil {
+		panic(err)
+	}
+	return ts
+}