From 90ccc4a014a3fada596a31366fd1b1a498573dfa Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Fri, 7 Jun 2019 18:12:35 +0530 Subject: [PATCH] use space to check if entry in a range has allocations while doing ring merge --- ipam/allocator.go | 6 +++++- ipam/ring/ring.go | 20 ++++++-------------- ipam/ring/ring_test.go | 20 +++++++++++++++++++- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index bfff91f0..9a633399 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -814,6 +814,10 @@ 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) update(sender mesh.PeerName, msg []byte) error { reader := bytes.NewReader(msg) decoder := gob.NewDecoder(reader) @@ -832,7 +836,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) + updated, err := alloc.ring.Merge(*data.Ring, alloc.checkRangeHasAllocations) switch err { case nil: if updated { diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index b77bc37c..aeaa5482 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) (bool, error) { +func (r *Ring) Merge(gossip Ring, checkRangeHasAllocations func(r address.Range) bool) (bool, error) { r.assertInvariants() defer r.trackUpdates()() @@ -238,7 +238,7 @@ func (r *Ring) Merge(gossip Ring) (bool, error) { return false, ErrDifferentRange } - result, updated, err := r.Entries.merge(gossip.Entries, r.Peer, r) + result, updated, err := r.Entries.merge(gossip.Entries, r.Peer, r, checkRangeHasAllocations) if err != nil { return false, err @@ -270,7 +270,7 @@ func (r *Ring) Merge(gossip Ring) (bool, error) { // 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) (result entries, updated bool, err error) { +func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring, checkRangeHasAllocations 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) } @@ -280,14 +280,6 @@ func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring) (result e previousOwner = nil } - checkEntryHasAllocations := func(current, next *entry) bool { - size := r.distance(current.Token, next.Token) - if current.Free < size { - return true - } - return false - } - // i is index into es; j is index into other var i, j int for i < len(es) && j < len(other) { @@ -302,7 +294,7 @@ func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring) (result e // 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 checkEntryHasAllocations(theirs, mine) { + if checkRangeHasAllocations(address.Range{Start: theirs.Token, End: mine.Token}) { err = errEntryInMyRange(theirs) return } @@ -317,7 +309,7 @@ func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring) (result e if mine.Version == theirs.Version && !mine.Equal(theirs) { if mine.Peer == ourPeer { // if we own the entry and has allocations - if checkEntryHasAllocations(mine, es.entry(i+1)) { + if checkRangeHasAllocations(address.Range{Start: mine.Token, End: es.entry(i + 1).Token}) { err = errInconsistentEntry(mine, theirs) return } @@ -340,7 +332,7 @@ func (es entries) merge(other entries, ourPeer mesh.PeerName, r *Ring) (result e // 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 && !checkEntryHasAllocations(mine, es.entry(i+1)) { + if theirs.Peer != ourPeer && !checkRangeHasAllocations(address.Range{Start: mine.Token, End: 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 3a79869c..859a411e 100644 --- a/ipam/ring/ring_test.go +++ b/ipam/ring/ring_test.go @@ -32,7 +32,25 @@ func ParseIP(s string) address.Address { } func merge(r1, r2 *Ring) error { - _, err := r1.Merge(*r2) + distance := func(start, end address.Address) address.Count { + if end > start { + return address.Count(end - start) + } + 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 + } + return false + } + + _, err := r1.Merge(*r2, checkEntryHasAllocations) return err }