From 96bd8e668adaeead2e2455cdd323816968fdf9a2 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sat, 20 Feb 2016 11:56:48 -0800 Subject: [PATCH 1/3] nomad: adding simple bitmap implementation --- nomad/structs/bitmap.go | 39 ++++++++++++++++++++++++ nomad/structs/bitmap_test.go | 58 ++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 nomad/structs/bitmap.go create mode 100644 nomad/structs/bitmap_test.go diff --git a/nomad/structs/bitmap.go b/nomad/structs/bitmap.go new file mode 100644 index 000000000..22a4aedb8 --- /dev/null +++ b/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 + } +} diff --git a/nomad/structs/bitmap_test.go b/nomad/structs/bitmap_test.go new file mode 100644 index 000000000..076d6c6d4 --- /dev/null +++ b/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") + } + } +} From 1e5b230e4a32d11f6fa19503d3b865535fc74925 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sat, 20 Feb 2016 12:08:27 -0800 Subject: [PATCH 2/3] nomad: use bitmap for port collision checking --- nomad/structs/network.go | 36 +++++++++++++++++++++++++---------- nomad/structs/network_test.go | 12 ++++++------ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/nomad/structs/network.go b/nomad/structs/network.go index 1cbcce855..bf5d4bcba 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -16,22 +16,25 @@ const ( // maxRandPortAttempts is the maximum number of attempt // to assign a random port maxRandPortAttempts = 20 + + // maxValidPort is the max valid port number + maxValidPort = 65536 ) // 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), } } @@ -92,16 +95,20 @@ 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{}) + 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 +161,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 +194,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 } diff --git a/nomad/structs/network_test.go b/nomad/structs/network_test.go index 3ec453df5..55d59d9ab 100644 --- a/nomad/structs/network_test.go +++ b/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") } From f1c7e2739436c187d786ec8659840b6fc1bc1449 Mon Sep 17 00:00:00 2001 From: Armon Dadgar Date: Sat, 20 Feb 2016 12:18:22 -0800 Subject: [PATCH 3/3] nomad: cache bitmaps to avoid GC pressure --- nomad/structs/funcs.go | 1 + nomad/structs/network.go | 26 +++++++++++++++++++++++++- scheduler/rank.go | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/nomad/structs/funcs.go b/nomad/structs/funcs.go index 09b391fe3..bf32e6f06 100644 --- a/nomad/structs/funcs.go +++ b/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 } diff --git a/nomad/structs/network.go b/nomad/structs/network.go index bf5d4bcba..c55f71b72 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -4,6 +4,7 @@ import ( "fmt" "math/rand" "net" + "sync" ) const ( @@ -21,6 +22,14 @@ const ( 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 { @@ -39,6 +48,14 @@ func NewNetworkIndex() *NetworkIndex { } } +// 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 { @@ -95,7 +112,14 @@ func (idx *NetworkIndex) AddReserved(n *NetworkResource) (collide bool) { // Add the port usage used := idx.UsedPorts[n.IP] if used == nil { - used, _ = NewBitmap(maxValidPort) + // 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 } diff --git a/scheduler/rank.go b/scheduler/rank.go index 1677187ad..b3610078c 100644 --- a/scheduler/rank.go +++ b/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