From d2f0287fa3a76c36fe749c6c8a036e5cdd7de161 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 10 Oct 2019 11:00:30 +0000 Subject: [PATCH] Handle range wrap-around when checking for allocations Ring wraps around at zero but Space doesn't know about this, so we have to normalise ranges coming out of Ring. In turn we may need to split one range into two, across the end of the Ring. --- ipam/allocator.go | 12 +++++++++--- ipam/ring/ring.go | 12 ++++++------ ipam/ring/ring_test.go | 20 +++++++++++--------- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index ae178452..831b9862 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -814,8 +814,14 @@ func (alloc *Allocator) sendRingUpdate(dest mesh.PeerName) { alloc.gossip.GossipUnicast(dest, msg) } -func (alloc *Allocator) checkRangeHasAllocations(r address.Range) bool { - return alloc.space.NumFreeAddressesInRange(r) != r.Size() +func (alloc *Allocator) hasAllocations(rs []address.Range) bool { + for _, r := range rs { + alloc.debugf("hasAllocations: %v %v vs %v", r, alloc.space.NumFreeAddressesInRange(r), r.Size()) + if alloc.space.NumFreeAddressesInRange(r) != r.Size() { + return true + } + } + return false } func (alloc *Allocator) update(sender mesh.PeerName, msg []byte) error { @@ -836,7 +842,7 @@ func (alloc *Allocator) update(sender mesh.PeerName, msg []byte) error { // If someone sent us a ring, merge it into ours. Note this will move us // out of the awaiting-consensus state if we didn't have a ring already. case data.Ring != nil: - updated, err := alloc.ring.Merge(*data.Ring, alloc.checkRangeHasAllocations) + updated, err := alloc.ring.Merge(*data.Ring, alloc.hasAllocations) switch err { case nil: if updated { diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index cc0f4dc6..1eeee069 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -213,7 +213,7 @@ func (r *Ring) GrantRangeToHost(start, end address.Address, peer mesh.PeerName) // Merge the given ring into this ring and indicate whether this ring // got updated as a result. -func (r *Ring) Merge(gossip Ring, checkRangeHasAllocations func(r address.Range) bool) (bool, error) { +func (r *Ring) Merge(gossip Ring, hasAllocations func(r []address.Range) bool) (bool, error) { r.assertInvariants() defer r.trackUpdates()() @@ -238,7 +238,7 @@ func (r *Ring) Merge(gossip Ring, checkRangeHasAllocations func(r address.Range) return false, ErrDifferentRange } - result, updated, err := r.Entries.merge(gossip.Entries, r.Peer, r, checkRangeHasAllocations) + result, updated, err := r.Entries.merge(gossip.Entries, r.Peer, r, hasAllocations) if err != nil { return false, err @@ -270,7 +270,7 @@ func (r *Ring) Merge(gossip Ring, checkRangeHasAllocations func(r address.Range) // entries belonging to ourPeer. Returns the merged entries and an // indication whether the merge resulted in any changes, i.e. the // result differs from the original. -func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring, checkRangeHasAllocations func(r address.Range) bool) (result entries, updated bool, err error) { +func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring, hasAllocations func(r []address.Range) bool) (result entries, updated bool, err error) { var mine, theirs *entry var previousOwner *mesh.PeerName addToResult := func(e entry) { result = append(result, &e) } @@ -294,7 +294,7 @@ func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring, checkRang // insert, checking that a range owned by us hasn't been split if previousOwner != nil && *previousOwner == ourPeer && theirs.Peer != ourPeer { // check we have no allocations in the range that got split - if checkRangeHasAllocations(address.Range{Start: theirs.Token, End: mine.Token}) { + if hasAllocations(r.makeRanges(theirs.Token, mine.Token)) { err = errEntryInMyRange(theirs) return } @@ -309,7 +309,7 @@ func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring, checkRang if mine.Version == theirs.Version && !mine.Equal(theirs) { if mine.Peer == ourPeer { // if we own the entry and has allocations - if checkRangeHasAllocations(address.Range{Start: mine.Token, End: es.entry(i + 1).Token}) { + if hasAllocations(r.makeRanges(theirs.Token, es.entry(i+1).Token)) { err = errInconsistentEntry(mine, theirs) return } @@ -332,7 +332,7 @@ func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring, checkRang // in the range effectively given away, or it belongs to our own peer, in // which case we should set our version to the one received plus one, // effectively imposing our existing entry. - if theirs.Peer != ourPeer && !checkRangeHasAllocations(address.Range{Start: mine.Token, End: es.entry(i + 1).Token}) { + if theirs.Peer != ourPeer && !hasAllocations(r.makeRanges(mine.Token, es.entry(i+1).Token)) { addTheirs(*theirs) } else if theirs.Peer == ourPeer { mine.Version = theirs.Version + 1 diff --git a/ipam/ring/ring_test.go b/ipam/ring/ring_test.go index 859a411e..c137de65 100644 --- a/ipam/ring/ring_test.go +++ b/ipam/ring/ring_test.go @@ -38,19 +38,21 @@ func merge(r1, r2 *Ring) error { } return address.Count((r1.End - start) + (end - r1.Start)) } - checkEntryHasAllocations := func(r address.Range) bool { - size := distance(r.Start, r.End) - entry, found := r1.Entries.get(r.Start) - if !found { - return false - } - if entry.Free < size { - return true + check := func(rs []address.Range) bool { + for _, r := range rs { + size := distance(r.Start, r.End) + entry, found := r1.Entries.get(r.Start) + if !found { + continue + } + if entry.Free < size { + return true + } } return false } - _, err := r1.Merge(*r2, checkEntryHasAllocations) + _, err := r1.Merge(*r2, check) return err }