99d46e5a49
When the scheduler tries to find a placement for a new allocation, it iterates over a subset of nodes. For each node, we populate a `NetworkIndex` bitmap with the ports of all existing allocations and any other allocations already proposed as part of this same evaluation via its `SetAllocs` method. Then we make an "ask" of the `NetworkIndex` in `AssignPorts` for any ports we need and receive an "offer" in return. The offer will include both static ports and any dynamic port assignments. The `AssignPorts` method was written to support group networks, and it shares code that selects dynamic ports with the original `AssignTaskNetwork` code. `AssignTaskNetwork` can request multiple ports from the bitmap at a time. But `AssignPorts` requests them one at a time and does not account for possible collisions, and doesn't return an error in that case. What happens next varies: 1. If the scheduler doesn't place the allocation on that node, the port conflict is thrown away and there's no problem. 2. If the node is picked and this is the only allocation (or last allocation), the plan applier will reject the plan when it calls `SetAllocs`, as we'd expect. 3. If the node is picked and there are additional allocations in the same eval that iterate over the same node, their call to `SetAllocs` will detect the impossible state and the node will be rejected. This can have the puzzling behavior where a second task group for the job without any networking at all can hit a port collision error! It looks like this bug has existed since we implemented group networks, but there are several factors that add up to making the issue rare for many users yet frustratingly frequent for others: * You're more likely to hit this bug the more tightly packed your range for dynamic ports is. With 12000 ports in the range by default, many clusters can avoid this for a long time. * You're more likely to hit case (3) for jobs with lots of allocations or if a scheduler has to iterate over a large number of nodes, such as with system jobs, jobs with `spread` blocks, or (sometimes) jobs using `unique` constraints. For unlucky combinations of these factors, it's possible that case (3) happens repeatedly, preventing scheduling of a given job until a client state change (ex. restarting the agent so all its allocations are rescheduled elsewhere) re-opens the range of dynamic ports available. This changeset: * Fixes the bug by accounting for collisions in dynamic port selection in `AssignPorts`. * Adds test coverage for `AssignPorts`, expands coverage of this case for the deprecated `AssignTaskNetwork`, and tightens the dynamic port range in a scheduler test for spread scheduling to more easily detect this kind of problem in the future. * Adds a `String()` method to `Bitmap` so that any future "screaming" log lines have a human-readable list of used ports.
105 lines
2.4 KiB
Go
105 lines
2.4 KiB
Go
package structs
|
|
|
|
import (
|
|
"fmt"
|
|
|
|
"golang.org/x/exp/slices"
|
|
)
|
|
|
|
// Bitmap is a simple uncompressed bitmap
|
|
type Bitmap []byte
|
|
|
|
// NewBitmap returns a bitmap with up to size indexes
|
|
func NewBitmap(size uint) (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
|
|
}
|
|
|
|
// Copy returns a copy of the Bitmap
|
|
func (b Bitmap) Copy() (Bitmap, error) {
|
|
if b == nil {
|
|
return nil, fmt.Errorf("can't copy nil Bitmap")
|
|
}
|
|
|
|
raw := make([]byte, len(b))
|
|
copy(raw, b)
|
|
return Bitmap(raw), nil
|
|
}
|
|
|
|
// Size returns the size of the bitmap
|
|
func (b Bitmap) Size() uint {
|
|
return uint(len(b) << 3)
|
|
}
|
|
|
|
// 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
|
|
}
|
|
|
|
// Unset is used to unset the given index of the bitmap
|
|
func (b Bitmap) Unset(idx uint) {
|
|
bucket := idx >> 3
|
|
// Mask should be all ones minus the idx position
|
|
offset := 1 << (idx & 7)
|
|
mask := byte(offset ^ 0xff)
|
|
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
|
|
}
|
|
}
|
|
|
|
// IndexesInRange returns the indexes in which the values are either set or unset based
|
|
// on the passed parameter in the passed range
|
|
func (b Bitmap) IndexesInRange(set bool, from, to uint) []int {
|
|
var indexes []int
|
|
for i := from; i <= to && i < b.Size(); i++ {
|
|
c := b.Check(i)
|
|
if c && set || !c && !set {
|
|
indexes = append(indexes, int(i))
|
|
}
|
|
}
|
|
|
|
return indexes
|
|
}
|
|
|
|
// IndexesInRangeFiltered returns the indexes in which the values are either set
|
|
// or unset based on the passed parameter in the passed range, and do not appear
|
|
// in the filter slice
|
|
func (b Bitmap) IndexesInRangeFiltered(set bool, from, to uint, filter []int) []int {
|
|
var indexes []int
|
|
for i := from; i <= to && i < b.Size(); i++ {
|
|
c := b.Check(i)
|
|
if c == set {
|
|
if len(filter) < 1 || !slices.Contains(filter, int(i)) {
|
|
indexes = append(indexes, int(i))
|
|
}
|
|
}
|
|
}
|
|
|
|
return indexes
|
|
}
|
|
|
|
// String represents the Bitmap the same as slice of the Bitmap's set values
|
|
func (b Bitmap) String() string {
|
|
return fmt.Sprintf("%v", b.IndexesInRange(true, 0, b.Size()))
|
|
}
|