From 4d4a073d604d9d37948ff144dc01c9fb0e2d69dc Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Fri, 19 Jun 2020 17:57:47 -0400 Subject: [PATCH 1/7] only have 15 outstanding GetAncestors at a time during bootstrapping to not flood the network --- snow/engine/avalanche/bootstrapper.go | 33 +++++++++++++++++++++++++-- snow/engine/common/bootstrapper.go | 3 +++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/snow/engine/avalanche/bootstrapper.go b/snow/engine/avalanche/bootstrapper.go index 3ed58c7..749694e 100644 --- a/snow/engine/avalanche/bootstrapper.go +++ b/snow/engine/avalanche/bootstrapper.go @@ -51,6 +51,11 @@ type bootstrapper struct { // tracks which validators were asked for which containers in which requests outstandingRequests common.Requests + // IDs of vertices that we will send a GetAncestors request for once we are not at the + // max number of outstanding requests + // Invariant: The intersection of needToFetch and outstandingRequests is empty + needToFetch ids.Set + // Contains IDs of vertices that have recently been processed processedCache *cache.LRU @@ -103,11 +108,21 @@ func (b *bootstrapper) FilterAccepted(containerIDs ids.Set) ids.Set { return acceptedVtxIDs } -// Get vertex [vtxID] and its ancestors +// Calls fetch for a pending vertex if there are any +func (b *bootstrapper) fetchANeededVtx() error { + if b.needToFetch.Len() > 0 { + return b.fetch(b.needToFetch.List()[0]) + } + return nil +} + +// Get vertex [vtxID] and its ancestors. +// If [vtxID] has already been requested or is already fetched, and there are +// unrequested vertices, requests one such vertex instead of [vtxID] func (b *bootstrapper) fetch(vtxID ids.ID) error { // Make sure we haven't already requested this block if b.outstandingRequests.Contains(vtxID) { - return nil + return b.fetchANeededVtx() } // Make sure we don't already have this vertex @@ -115,6 +130,13 @@ func (b *bootstrapper) fetch(vtxID ids.ID) error { if numPending := b.outstandingRequests.Len(); numPending == 0 && b.processedStartingAcceptedFrontier { return b.finish() } + b.needToFetch.Remove(vtxID) // we have this vertex. no need to request it. + return b.fetchANeededVtx() + } + + // If we're already at maximum number of outstanding requests, queue for later + if b.outstandingRequests.Len() >= common.MaxOutstandingRequests { + b.needToFetch.Add(vtxID) return nil } @@ -126,6 +148,7 @@ func (b *bootstrapper) fetch(vtxID ids.ID) error { b.RequestID++ b.outstandingRequests.Add(validatorID, b.RequestID, vtxID) + b.needToFetch.Remove(vtxID) // maintains invariant that intersection with outstandingRequests is empty b.BootstrapConfig.Sender.GetAncestors(validatorID, b.RequestID, vtxID) // request vertex and ancestors return nil } @@ -236,9 +259,15 @@ func (b *bootstrapper) MultiPut(vdr ids.ShortID, requestID uint32, vtxs [][]byte b.BootstrapConfig.Context.Log.Verbo("vertex: %s", formatting.DumpBytes{Bytes: vtxBytes}) } else { processVertices = append(processVertices, vtx) + b.needToFetch.Remove(vtx.ID()) // No need to fetch this vertex since we have it now } } + // Now there is one less outstanding request; send another if needed + if err := b.fetchANeededVtx(); err != nil { + return err + } + return b.process(processVertices...) } diff --git a/snow/engine/common/bootstrapper.go b/snow/engine/common/bootstrapper.go index 49f4051..963a4fb 100644 --- a/snow/engine/common/bootstrapper.go +++ b/snow/engine/common/bootstrapper.go @@ -17,6 +17,9 @@ const ( // StatusUpdateFrequency ... bootstrapper logs "processed X blocks/vertices" every [statusUpdateFrequency] blocks/vertices StatusUpdateFrequency = 2500 + + // MaxOutstandingRequests is the maximum number of GetAncestors sent but not responsded to/failed + MaxOutstandingRequests = 15 ) var ( From 67d92815010bc617c01094ccedd4d307ba57234a Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Fri, 19 Jun 2020 18:06:04 -0400 Subject: [PATCH 2/7] change maximum # outstanding to 8 to reduce load on nodes --- snow/engine/common/bootstrapper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/snow/engine/common/bootstrapper.go b/snow/engine/common/bootstrapper.go index 963a4fb..8c9c745 100644 --- a/snow/engine/common/bootstrapper.go +++ b/snow/engine/common/bootstrapper.go @@ -19,7 +19,7 @@ const ( StatusUpdateFrequency = 2500 // MaxOutstandingRequests is the maximum number of GetAncestors sent but not responsded to/failed - MaxOutstandingRequests = 15 + MaxOutstandingRequests = 8 ) var ( From 55079aa893e91f4d15dab6342a4d02fb1aef0dd3 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Tue, 23 Jun 2020 15:01:55 -0400 Subject: [PATCH 3/7] add CappedList for ids.Set and use it in fetchANeededVtx --- ids/set.go | 23 +++++++++++++- ids/set_test.go | 43 +++++++++++++++++++++++++++ snow/engine/avalanche/bootstrapper.go | 2 +- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/ids/set.go b/ids/set.go index c3aa024..d632949 100644 --- a/ids/set.go +++ b/ids/set.go @@ -78,7 +78,7 @@ func (ids *Set) Clear() { *ids = nil } // List converts this set into a list func (ids Set) List() []ID { - idList := make([]ID, ids.Len(), ids.Len()) + idList := make([]ID, ids.Len()) i := 0 for id := range ids { idList[i] = NewID(id) @@ -87,6 +87,27 @@ func (ids Set) List() []ID { return idList } +// CappedList returns a list of length at most [size]. +// Size should be >= 0. If size < 0, returns nil. +func (ids Set) CappedList(size int) []ID { + if size < 0 { + return nil + } + if l := ids.Len(); l < size { + size = l + } + i := 0 + idList := make([]ID, size) + for id := range ids { + if i >= size { + break + } + idList[i] = NewID(id) + i++ + } + return idList +} + // Equals returns true if the sets contain the same elements func (ids Set) Equals(oIDs Set) bool { if ids.Len() != oIDs.Len() { diff --git a/ids/set_test.go b/ids/set_test.go index 3c7ab15..b4e05db 100644 --- a/ids/set_test.go +++ b/ids/set_test.go @@ -55,3 +55,46 @@ func TestSet(t *testing.T) { t.Fatalf("Sets overlap") } } + +func TestSetCappedList(t *testing.T) { + set := Set{} + + id := Empty + + if list := set.CappedList(0); len(list) != 0 { + t.Fatalf("List should have been empty but was %v", list) + } + + set.Add(id) + + if list := set.CappedList(0); len(list) != 0 { + t.Fatalf("List should have been empty but was %v", list) + } else if list := set.CappedList(1); len(list) != 1 { + t.Fatalf("List should have had length %d but had %d", 1, len(list)) + } else if returnedID := list[0]; !id.Equals(returnedID) { + t.Fatalf("List should have been %s but was %s", id, returnedID) + } else if list := set.CappedList(2); len(list) != 1 { + t.Fatalf("List should have had length %d but had %d", 1, len(list)) + } else if returnedID := list[0]; !id.Equals(returnedID) { + t.Fatalf("List should have been %s but was %s", id, returnedID) + } + + id2 := NewID([32]byte{1}) + set.Add(id2) + + if list := set.CappedList(0); len(list) != 0 { + t.Fatalf("List should have been empty but was %v", list) + } else if list := set.CappedList(1); len(list) != 1 { + t.Fatalf("List should have had length %d but had %d", 1, len(list)) + } else if returnedID := list[0]; !id.Equals(returnedID) && !id2.Equals(returnedID) { + t.Fatalf("List should have been %s but was %s", id, returnedID) + } else if list := set.CappedList(2); len(list) != 2 { + t.Fatalf("List should have had length %d but had %d", 2, len(list)) + } else if list := set.CappedList(3); len(list) != 2 { + t.Fatalf("List should have had length %d but had %d", 2, len(list)) + } else if returnedID := list[0]; !id.Equals(returnedID) && !id2.Equals(returnedID) { + t.Fatalf("list contains unexpected element %s", returnedID) + } else if returnedID := list[1]; !id.Equals(returnedID) && !id2.Equals(returnedID) { + t.Fatalf("list contains unexpected element %s", returnedID) + } +} diff --git a/snow/engine/avalanche/bootstrapper.go b/snow/engine/avalanche/bootstrapper.go index 749694e..352f40b 100644 --- a/snow/engine/avalanche/bootstrapper.go +++ b/snow/engine/avalanche/bootstrapper.go @@ -111,7 +111,7 @@ func (b *bootstrapper) FilterAccepted(containerIDs ids.Set) ids.Set { // Calls fetch for a pending vertex if there are any func (b *bootstrapper) fetchANeededVtx() error { if b.needToFetch.Len() > 0 { - return b.fetch(b.needToFetch.List()[0]) + return b.fetch(b.needToFetch.CappedList(1)[0]) } return nil } From 7f5693dfd33fc31839c28ca5d94f44429b7218bc Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Tue, 23 Jun 2020 15:08:15 -0400 Subject: [PATCH 4/7] reduce MaxTimeFetchingAncestors from 100ms to 50ms --- snow/engine/common/bootstrapper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/snow/engine/common/bootstrapper.go b/snow/engine/common/bootstrapper.go index 8c9c745..f1f58db 100644 --- a/snow/engine/common/bootstrapper.go +++ b/snow/engine/common/bootstrapper.go @@ -24,7 +24,7 @@ const ( var ( // MaxTimeFetchingAncestors is the maximum amount of time to spend fetching vertices during a call to GetAncestors - MaxTimeFetchingAncestors = 100 * time.Millisecond + MaxTimeFetchingAncestors = 50 * time.Millisecond ) // Bootstrapper implements the Engine interface. From 8c7934515c8e1ce1cb080b19ea02baf27ce65140 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Tue, 23 Jun 2020 19:41:22 -0400 Subject: [PATCH 5/7] removed mutually recursive functions for fetching --- snow/engine/avalanche/bootstrapper.go | 105 ++++++++++---------------- 1 file changed, 40 insertions(+), 65 deletions(-) diff --git a/snow/engine/avalanche/bootstrapper.go b/snow/engine/avalanche/bootstrapper.go index 352f40b..d8f1d6b 100644 --- a/snow/engine/avalanche/bootstrapper.go +++ b/snow/engine/avalanche/bootstrapper.go @@ -42,18 +42,16 @@ type bootstrapper struct { metrics common.Bootstrapper - // true if all of the vertices in the original accepted frontier have been processed - processedStartingAcceptedFrontier bool - // number of vertices fetched so far numFetched uint32 // tracks which validators were asked for which containers in which requests outstandingRequests common.Requests - // IDs of vertices that we will send a GetAncestors request for once we are not at the - // max number of outstanding requests - // Invariant: The intersection of needToFetch and outstandingRequests is empty + // IDs of vertices that we will send a GetAncestors request for once we are + // not at the max number of outstanding requests + // Invariant: The intersection of needToFetch and outstandingRequests is + // empty needToFetch ids.Set // Contains IDs of vertices that have recently been processed @@ -108,49 +106,36 @@ func (b *bootstrapper) FilterAccepted(containerIDs ids.Set) ids.Set { return acceptedVtxIDs } -// Calls fetch for a pending vertex if there are any -func (b *bootstrapper) fetchANeededVtx() error { - if b.needToFetch.Len() > 0 { - return b.fetch(b.needToFetch.CappedList(1)[0]) - } - return nil -} +// Fetch vertices and their ancestors from the set of vertices that are needed +// to be fetched. +func (b *bootstrapper) fetch(vtxIDs ...ids.ID) error { + b.needToFetch.Add(vtxIDs...) + for b.needToFetch.Len() > 0 && b.outstandingRequests.Len() < common.MaxOutstandingRequests { + vtxID := b.needToFetch.CappedList(1)[0] + b.needToFetch.Remove(vtxID) -// Get vertex [vtxID] and its ancestors. -// If [vtxID] has already been requested or is already fetched, and there are -// unrequested vertices, requests one such vertex instead of [vtxID] -func (b *bootstrapper) fetch(vtxID ids.ID) error { - // Make sure we haven't already requested this block - if b.outstandingRequests.Contains(vtxID) { - return b.fetchANeededVtx() - } - - // Make sure we don't already have this vertex - if _, err := b.State.GetVertex(vtxID); err == nil { - if numPending := b.outstandingRequests.Len(); numPending == 0 && b.processedStartingAcceptedFrontier { - return b.finish() + // Make sure we haven't already requested this vertex + if b.outstandingRequests.Contains(vtxID) { + continue } - b.needToFetch.Remove(vtxID) // we have this vertex. no need to request it. - return b.fetchANeededVtx() - } - // If we're already at maximum number of outstanding requests, queue for later - if b.outstandingRequests.Len() >= common.MaxOutstandingRequests { - b.needToFetch.Add(vtxID) - return nil - } + // Make sure we don't already have this vertex + if _, err := b.State.GetVertex(vtxID); err == nil { + continue + } - validators := b.BootstrapConfig.Validators.Sample(1) // validator to send request to - if len(validators) == 0 { - return fmt.Errorf("Dropping request for %s as there are no validators", vtxID) - } - validatorID := validators[0].ID() - b.RequestID++ + validators := b.BootstrapConfig.Validators.Sample(1) // validator to send request to + if len(validators) == 0 { + return fmt.Errorf("Dropping request for %s as there are no validators", vtxID) + } + validatorID := validators[0].ID() + b.RequestID++ - b.outstandingRequests.Add(validatorID, b.RequestID, vtxID) - b.needToFetch.Remove(vtxID) // maintains invariant that intersection with outstandingRequests is empty - b.BootstrapConfig.Sender.GetAncestors(validatorID, b.RequestID, vtxID) // request vertex and ancestors - return nil + b.outstandingRequests.Add(validatorID, b.RequestID, vtxID) + b.needToFetch.Remove(vtxID) // maintains invariant that intersection with outstandingRequests is empty + b.BootstrapConfig.Sender.GetAncestors(validatorID, b.RequestID, vtxID) // request vertex and ancestors + } + return b.finish() } // Process vertices @@ -164,14 +149,17 @@ func (b *bootstrapper) process(vtxs ...avalanche.Vertex) error { for toProcess.Len() > 0 { vtx := toProcess.Pop() + vtxID := vtx.ID() + switch vtx.Status() { case choices.Unknown: - if err := b.fetch(vtx.ID()); err != nil { - return err - } + b.fetch(vtxID) case choices.Rejected: + b.needToFetch.Remove(vtxID) return fmt.Errorf("tried to accept %s even though it was previously rejected", vtx.ID()) case choices.Processing: + b.needToFetch.Remove(vtxID) + if err := b.VtxBlocked.Push(&vertexJob{ log: b.BootstrapConfig.Context.Log, numAccepted: b.numBSVtx, @@ -216,10 +204,7 @@ func (b *bootstrapper) process(vtxs ...avalanche.Vertex) error { return err } - if numPending := b.outstandingRequests.Len(); numPending == 0 && b.processedStartingAcceptedFrontier { - return b.finish() - } - return nil + return b.fetch() } // MultiPut handles the receipt of multiple containers. Should be received in response to a GetAncestors message to [vdr] @@ -263,11 +248,6 @@ func (b *bootstrapper) MultiPut(vdr ids.ShortID, requestID uint32, vtxs [][]byte } } - // Now there is one less outstanding request; send another if needed - if err := b.fetchANeededVtx(); err != nil { - return err - } - return b.process(processVertices...) } @@ -293,24 +273,19 @@ func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) error { for _, vtxID := range acceptedContainerIDs.List() { if vtx, err := b.State.GetVertex(vtxID); err == nil { storedVtxs = append(storedVtxs, vtx) - } else if err := b.fetch(vtxID); err != nil { - return err + } else { + b.needToFetch.Add(vtxID) } } if err := b.process(storedVtxs...); err != nil { return err } - b.processedStartingAcceptedFrontier = true - - if numPending := b.outstandingRequests.Len(); numPending == 0 { - return b.finish() - } - return nil + return b.fetch() } // Finish bootstrapping func (b *bootstrapper) finish() error { - if b.finished { + if b.finished || b.outstandingRequests.Len() > 0 || b.needToFetch.Len() > 0 { return nil } b.BootstrapConfig.Context.Log.Info("finished fetching vertices. executing transaction state transitions...") From 16f006edc98aa0421fc0bb7b38c519a858042730 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Tue, 23 Jun 2020 19:43:03 -0400 Subject: [PATCH 6/7] Removed no longer upheld invariant --- snow/engine/avalanche/bootstrapper.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/snow/engine/avalanche/bootstrapper.go b/snow/engine/avalanche/bootstrapper.go index d8f1d6b..a9c4e64 100644 --- a/snow/engine/avalanche/bootstrapper.go +++ b/snow/engine/avalanche/bootstrapper.go @@ -50,8 +50,6 @@ type bootstrapper struct { // IDs of vertices that we will send a GetAncestors request for once we are // not at the max number of outstanding requests - // Invariant: The intersection of needToFetch and outstandingRequests is - // empty needToFetch ids.Set // Contains IDs of vertices that have recently been processed From 26edbc5e6ecfce15d9c1f49e50f3a5fd3a9aa823 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Tue, 23 Jun 2020 19:57:44 -0400 Subject: [PATCH 7/7] cleaned up avalanche bootstrapping --- snow/engine/avalanche/bootstrapper.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/snow/engine/avalanche/bootstrapper.go b/snow/engine/avalanche/bootstrapper.go index a9c4e64..1af48bf 100644 --- a/snow/engine/avalanche/bootstrapper.go +++ b/snow/engine/avalanche/bootstrapper.go @@ -151,7 +151,7 @@ func (b *bootstrapper) process(vtxs ...avalanche.Vertex) error { switch vtx.Status() { case choices.Unknown: - b.fetch(vtxID) + b.needToFetch.Add(vtxID) case choices.Rejected: b.needToFetch.Remove(vtxID) return fmt.Errorf("tried to accept %s even though it was previously rejected", vtx.ID()) @@ -275,10 +275,7 @@ func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) error { b.needToFetch.Add(vtxID) } } - if err := b.process(storedVtxs...); err != nil { - return err - } - return b.fetch() + return b.process(storedVtxs...) } // Finish bootstrapping