Merge pull request #823 from hashicorp/f-bitmap

Switch port collision checking to use bitmap instead of map

nomad/structs/bitmap.go

@@ -0,0 +1,39 @@
+package structs
+
+import "fmt"
+
+// Bitmap is a simple uncompressed bitmap
+type Bitmap []byte
+
+// NewBitmap returns a bitmap with up to size indexes
+func NewBitmap(size int) (Bitmap, error) {
+	if size <= 0 {
+		return nil, fmt.Errorf("bitmap must be positive size")
+	}
+	if size&7 != 0 {
+		return nil, fmt.Errorf("bitmap must be byte aligned")
+	}
+	b := make([]byte, size>>3)
+	return Bitmap(b), nil
+}
+
+// Set is used to set the given index of the bitmap
+func (b Bitmap) Set(idx uint) {
+	bucket := idx >> 3
+	mask := byte(1 << (idx & 7))
+	b[bucket] |= mask
+}
+
+// Check is used to check the given index of the bitmap
+func (b Bitmap) Check(idx uint) bool {
+	bucket := idx >> 3
+	mask := byte(1 << (idx & 7))
+	return (b[bucket] & mask) != 0
+}
+
+// Clear is used to efficiently clear the bitmap
+func (b Bitmap) Clear() {
+	for i := range b {
+		b[i] = 0
+	}
+}

nomad/structs/bitmap_test.go

@@ -0,0 +1,58 @@
+package structs
+
+import "testing"
+
+func TestBitmap(t *testing.T) {
+	// Check invalid sizes
+	_, err := NewBitmap(0)
+	if err == nil {
+		t.Fatalf("bad")
+	}
+	_, err = NewBitmap(7)
+	if err == nil {
+		t.Fatalf("bad")
+	}
+
+	// Create a normal bitmap
+	b, err := NewBitmap(256)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	// Set a few bits
+	b.Set(0)
+	b.Set(255)
+
+	// Verify the bytes
+	if b[0] == 0 {
+		t.Fatalf("bad")
+	}
+	if !b.Check(0) {
+		t.Fatalf("bad")
+	}
+
+	// Verify the bytes
+	if b[len(b)-1] == 0 {
+		t.Fatalf("bad")
+	}
+	if !b.Check(255) {
+		t.Fatalf("bad")
+	}
+
+	// All other bits should be unset
+	for i := 1; i < 255; i++ {
+		if b.Check(uint(i)) {
+			t.Fatalf("bad")
+		}
+	}
+
+	// Clear
+	b.Clear()
+
+	// All bits should be unset
+	for i := 0; i < 256; i++ {
+		if b.Check(uint(i)) {
+			t.Fatalf("bad")
+		}
+	}
+}

nomad/structs/funcs.go

@@ -72,6 +72,7 @@ func AllocsFit(node *Node, allocs []*Allocation, netIdx *NetworkIndex) (bool, st
 	// Create the network index if missing
 	if netIdx == nil {
 		netIdx = NewNetworkIndex()
+		defer netIdx.Release()
 		if netIdx.SetNode(node) || netIdx.AddAllocs(allocs) {
 			return false, "reserved port collision", used, nil
 		}

nomad/structs/network.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"math/rand"
 	"net"
+	"sync"
 )
 
 const (
@@ -16,26 +17,45 @@ const (
 	// maxRandPortAttempts is the maximum number of attempt
 	// to assign a random port
 	maxRandPortAttempts = 20
+
+	// maxValidPort is the max valid port number
+	maxValidPort = 65536
+)
+
+var (
+	// bitmapPool is used to pool the bitmaps used for port collision
+	// checking. They are fairly large (8K) so we can re-use them to
+	// avoid GC pressure. Care should be taken to call Clear() on any
+	// bitmap coming from the pool.
+	bitmapPool = new(sync.Pool)
 )
 
 // NetworkIndex is used to index the available network resources
 // and the used network resources on a machine given allocations
 type NetworkIndex struct {
-	AvailNetworks  []*NetworkResource          // List of available networks
-	AvailBandwidth map[string]int              // Bandwidth by device
-	UsedPorts      map[string]map[int]struct{} // Ports by IP
-	UsedBandwidth  map[string]int              // Bandwidth by device
+	AvailNetworks  []*NetworkResource // List of available networks
+	AvailBandwidth map[string]int     // Bandwidth by device
+	UsedPorts      map[string]Bitmap  // Ports by IP
+	UsedBandwidth  map[string]int     // Bandwidth by device
 }
 
 // NewNetworkIndex is used to construct a new network index
 func NewNetworkIndex() *NetworkIndex {
 	return &NetworkIndex{
 		AvailBandwidth: make(map[string]int),
-		UsedPorts:      make(map[string]map[int]struct{}),
+		UsedPorts:      make(map[string]Bitmap),
 		UsedBandwidth:  make(map[string]int),
 	}
 }
 
+// Release is called when the network index is no longer needed
+// to attempt to re-use some of the memory it has allocated
+func (idx *NetworkIndex) Release() {
+	for _, b := range idx.UsedPorts {
+		bitmapPool.Put(b)
+	}
+}
+
 // Overcommitted checks if the network is overcommitted
 func (idx *NetworkIndex) Overcommitted() bool {
 	for device, used := range idx.UsedBandwidth {
@@ -92,16 +112,27 @@ func (idx *NetworkIndex) AddReserved(n *NetworkResource) (collide bool) {
 	// Add the port usage
 	used := idx.UsedPorts[n.IP]
 	if used == nil {
-		used = make(map[int]struct{})
+		// Try to get a bitmap from the pool, else create
+		raw := bitmapPool.Get()
+		if raw != nil {
+			used = raw.(Bitmap)
+			used.Clear()
+		} else {
+			used, _ = NewBitmap(maxValidPort)
+		}
 		idx.UsedPorts[n.IP] = used
 	}
 
 	for _, ports := range [][]Port{n.ReservedPorts, n.DynamicPorts} {
 		for _, port := range ports {
-			if _, ok := used[port.Value]; ok {
+			// Guard against invalid port
+			if port.Value < 0 || port.Value >= maxValidPort {
+				return true
+			}
+			if used.Check(uint(port.Value)) {
 				collide = true
 			} else {
-				used[port.Value] = struct{}{}
+				used.Set(uint(port.Value))
 			}
 		}
 	}
@@ -154,7 +185,15 @@ func (idx *NetworkIndex) AssignNetwork(ask *NetworkResource) (out *NetworkResour
 
 		// Check if any of the reserved ports are in use
 		for _, port := range ask.ReservedPorts {
-			if _, ok := idx.UsedPorts[ipStr][port.Value]; ok {
+			// Guard against invalid port
+			if port.Value < 0 || port.Value >= maxValidPort {
+				err = fmt.Errorf("invalid port %d (out of range)", port.Value)
+				return
+			}
+
+			// Check if in use
+			used := idx.UsedPorts[ipStr]
+			if used != nil && used.Check(uint(port.Value)) {
 				err = fmt.Errorf("reserved port collision")
 				return
 			}
@@ -179,7 +218,8 @@ func (idx *NetworkIndex) AssignNetwork(ask *NetworkResource) (out *NetworkResour
 			}
 
 			randPort := MinDynamicPort + rand.Intn(MaxDynamicPort-MinDynamicPort)
-			if _, ok := idx.UsedPorts[ipStr][randPort]; ok {
+			used := idx.UsedPorts[ipStr]
+			if used != nil && used.Check(uint(randPort)) {
 				goto PICK
 			}
 

nomad/structs/network_test.go

@@ -85,7 +85,7 @@ func TestNetworkIndex_SetNode(t *testing.T) {
 	if idx.UsedBandwidth["eth0"] != 1 {
 		t.Fatalf("Bad")
 	}
-	if _, ok := idx.UsedPorts["192.168.0.100"][22]; !ok {
+	if !idx.UsedPorts["192.168.0.100"].Check(22) {
 		t.Fatalf("Bad")
 	}
 }
@@ -130,13 +130,13 @@ func TestNetworkIndex_AddAllocs(t *testing.T) {
 	if idx.UsedBandwidth["eth0"] != 70 {
 		t.Fatalf("Bad")
 	}
-	if _, ok := idx.UsedPorts["192.168.0.100"][8000]; !ok {
+	if !idx.UsedPorts["192.168.0.100"].Check(8000) {
 		t.Fatalf("Bad")
 	}
-	if _, ok := idx.UsedPorts["192.168.0.100"][9000]; !ok {
+	if !idx.UsedPorts["192.168.0.100"].Check(9000) {
 		t.Fatalf("Bad")
 	}
-	if _, ok := idx.UsedPorts["192.168.0.100"][10000]; !ok {
+	if !idx.UsedPorts["192.168.0.100"].Check(10000) {
 		t.Fatalf("Bad")
 	}
 }
@@ -158,10 +158,10 @@ func TestNetworkIndex_AddReserved(t *testing.T) {
 	if idx.UsedBandwidth["eth0"] != 20 {
 		t.Fatalf("Bad")
 	}
-	if _, ok := idx.UsedPorts["192.168.0.100"][8000]; !ok {
+	if !idx.UsedPorts["192.168.0.100"].Check(8000) {
 		t.Fatalf("Bad")
 	}
-	if _, ok := idx.UsedPorts["192.168.0.100"][9000]; !ok {
+	if !idx.UsedPorts["192.168.0.100"].Check(9000) {
 		t.Fatalf("Bad")
 	}
 

scheduler/rank.go

@@ -193,6 +193,7 @@ OUTER:
 				if offer == nil {
 					iter.ctx.Metrics().ExhaustedNode(option.Node,
 						fmt.Sprintf("network: %s", err))
+					netIdx.Release()
 					continue OUTER
 				}
 
@@ -215,6 +216,7 @@ OUTER:
 
 		// Check if these allocations fit, if they do not, simply skip this node
 		fit, dim, util, _ := structs.AllocsFit(option.Node, proposed, netIdx)
+		netIdx.Release()
 		if !fit {
 			iter.ctx.Metrics().ExhaustedNode(option.Node, dim)
 			continue