Skip to content

Commit

Permalink
Make number of scheduler workers reloadable (#11593)
Browse files Browse the repository at this point in the history
## Development Environment Changes
* Added stringer to build deps

## New HTTP APIs
* Added scheduler worker config API
* Added scheduler worker info API

## New Internals
* (Scheduler)Worker API refactor—Start(), Stop(), Pause(), Resume()
* Update shutdown to use context
* Add mutex for contended server data
    - `workerLock` for the `workers` slice
    - `workerConfigLock` for the `Server.Config.NumSchedulers` and
      `Server.Config.EnabledSchedulers` values

## Other
* Adding docs for scheduler worker api
* Add changelog message

Co-authored-by: Derek Strickland <1111455+DerekStrickland@users.noreply.github.com>
  • Loading branch information
angrycub and DerekStrickland committed Jan 6, 2022
1 parent d5e8081 commit 6e61606
Show file tree
Hide file tree
Showing 21 changed files with 2,211 additions and 101 deletions.
3 changes: 3 additions & 0 deletions .changelog/11593.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
server: Make num_schedulers and enabled_schedulers hot reloadable; add agent API endpoint to enable dynamic modifications of these values.
```
57 changes: 57 additions & 0 deletions .tours/scheduler-worker---hot-reload.tour
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
{
"$schema": "https://aka.ms/codetour-schema",
"title": "Scheduler Worker - Hot Reload",
"steps": [
{
"file": "nomad/server.go",
"description": "## Server.Reload()\n\nServer configuration reloads start here.",
"line": 782,
"selection": {
"start": {
"line": 780,
"character": 4
},
"end": {
"line": 780,
"character": 10
}
}
},
{
"file": "nomad/server.go",
"description": "## Did NumSchedulers change?\nIf the number of schedulers has changed between the running configuration and the new one we need to adopt that change in realtime.",
"line": 812
},
{
"file": "nomad/server.go",
"description": "## Server.setupNewWorkers()\n\nsetupNewWorkers performs three tasks:\n\n- makes a copy of the existing worker pointers\n\n- creates a fresh array and loads a new set of workers into them\n\n- iterates through the \"old\" workers and shuts them down in individual\n goroutines for maximum parallelism",
"line": 1482,
"selection": {
"start": {
"line": 1480,
"character": 4
},
"end": {
"line": 1480,
"character": 12
}
}
},
{
"file": "nomad/server.go",
"description": "Once all of the work in setupNewWorkers is complete, we stop the old ones.",
"line": 1485
},
{
"file": "nomad/server.go",
"description": "The `stopOldWorkers` function iterates through the array of workers and calls their `Shutdown` method\nas a goroutine to prevent blocking.",
"line": 1505
},
{
"file": "nomad/worker.go",
"description": "The `Shutdown` method sets `w.stop` to true signaling that we intend for the `Worker` to stop the next time we consult it. We also manually unpause the `Worker` by setting w.paused to false and sending a `Broadcast()` via the cond.",
"line": 110
}
],
"ref": "f-reload-num-schedulers"
}
66 changes: 66 additions & 0 deletions .tours/scheduler-worker---pause.tour
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
{
"$schema": "https://aka.ms/codetour-schema",
"title": "Scheduler Worker - Pause",
"steps": [
{
"file": "nomad/leader.go",
"description": "## Server.establishLeadership()\n\nUpon becoming a leader, the server pauses a subset of the workers to allow for the additional burden of the leader's goroutines. The `handlePausableWorkers` function takes a boolean that states whether or not the current node is a leader or not. Because we are in `establishLeadership` we use `true` rather than calling `s.IsLeader()`",
"line": 233,
"selection": {
"start": {
"line": 233,
"character": 4
},
"end": {
"line": 233,
"character": 12
}
}
},
{
"file": "nomad/leader.go",
"description": "## Server.handlePausableWorkers()\n\nhandlePausableWorkers ranges over a slice of Workers and manipulates their paused state by calling their `SetPause` method.",
"line": 443,
"selection": {
"start": {
"line": 443,
"character": 18
},
"end": {
"line": 443,
"character": 26
}
}
},
{
"file": "nomad/leader.go",
"description": "## Server.pausableWorkers()\n\nThe pausableWorkers function provides a consistent slice of workers that the server can pause and unpause. Since the Worker array is never mutated, the same slice is returned by pausableWorkers on every invocation.\nThis comment is interesting/potentially confusing\n\n```golang\n // Disabling 3/4 of the workers frees CPU for raft and the\n\t// plan applier which uses 1/2 the cores.\n``` \n\nHowever, the key point is that it will return a slice containg 3/4th of the workers.",
"line": 1100,
"selection": {
"start": {
"line": 1104,
"character": 1
},
"end": {
"line": 1105,
"character": 43
}
}
},
{
"file": "nomad/worker.go",
"description": "## Worker.SetPause()\n\nThe `SetPause` function is used to signal an intention to pause the worker. Because the worker's work is happening in the `run()` goroutine, pauses happen asynchronously.",
"line": 91
},
{
"file": "nomad/worker.go",
"description": "## Worker.dequeueEvaluation()\n\nCalls checkPaused, which will be the function we wait in if the scheduler is set to be paused. \n\n> **NOTE:** This is called here rather than in run() because this function loops in case of an error fetching a evaluation.",
"line": 206
},
{
"file": "nomad/worker.go",
"description": "## Worker.checkPaused()\n\nWhen `w.paused` is `true`, we call the `Wait()` function on the condition. Execution of this goroutine will stop here until it receives a `Broadcast()` or a `Signal()`. At this point, the `Worker` is paused.",
"line": 104
}
]
}
51 changes: 51 additions & 0 deletions .tours/scheduler-worker---unpause.tour
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
{
"$schema": "https://aka.ms/codetour-schema",
"title": "Scheduler Worker - Unpause",
"steps": [
{
"file": "nomad/leader.go",
"description": "## revokeLeadership()\n\nAs a server transistions from leader to non-leader, the pausableWorkers are resumed since the other leader goroutines are stopped providing extra capacity.",
"line": 1040,
"selection": {
"start": {
"line": 1038,
"character": 10
},
"end": {
"line": 1038,
"character": 20
}
}
},
{
"file": "nomad/leader.go",
"description": "## handlePausableWorkers()\n\nThe handlePausableWorkers method is called with `false`. We fetch the pausableWorkers and call their SetPause method with `false`.\n",
"line": 443,
"selection": {
"start": {
"line": 443,
"character": 18
},
"end": {
"line": 443,
"character": 27
}
}
},
{
"file": "nomad/worker.go",
"description": "## Worker.SetPause()\n\nDuring unpause, p is false. We update w.paused in the mutex, and then call Broadcast on the cond. This wakes the goroutine sitting in the Wait() inside of `checkPaused()`",
"line": 91
},
{
"file": "nomad/worker.go",
"description": "## Worker.checkPaused()\n\nOnce the goroutine receives the `Broadcast()` message from `SetPause()`, execution continues here. Now that `w.paused == false`, we exit the loop and return to the caller (the `dequeueEvaluation()` function).",
"line": 104
},
{
"file": "nomad/worker.go",
"description": "## Worker.dequeueEvaluation\n\nWe return back into dequeueEvaluation after the call to checkPaused. At this point the worker will either stop (if that signal boolean has been set) or continue looping after returning to run().",
"line": 207
}
]
}
36 changes: 36 additions & 0 deletions .tours/scheduler-worker.tour
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"$schema": "https://aka.ms/codetour-schema",
"title": "Scheduler Worker - Start",
"steps": [
{
"file": "nomad/server.go",
"description": "## Server.NewServer()\n\nScheduler workers are started as the agent starts the `server` go routines.",
"line": 402
},
{
"file": "nomad/server.go",
"description": "## Server.setupWorkers()\n\nThe `setupWorkers()` function validates that there are enabled Schedulers by type and count. It then creates s.config.NumSchedulers by calling `NewWorker()`\n\nThe `_core` scheduler _**must**_ be enabled. **TODO: why?**\n",
"line": 1443,
"selection": {
"start": {
"line": 1442,
"character": 4
},
"end": {
"line": 1442,
"character": 12
}
}
},
{
"file": "nomad/worker.go",
"description": "## Worker.NewWorker\n\nNewWorker creates the Worker and starts `run()` in a goroutine.",
"line": 78
},
{
"file": "nomad/worker.go",
"description": "## Worker.run()\n\nThe `run()` function runs in a loop until it's paused, it's stopped, or the server indicates that it is shutting down. All of the work the `Worker` performs should be\nimplemented in or called from here.\n",
"line": 152
}
]
}
1 change: 1 addition & 0 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ deps: ## Install build and development dependencies
go install github.com/hashicorp/go-msgpack/codec/codecgen@v1.1.5
go install github.com/bufbuild/buf/cmd/buf@v0.36.0
go install github.com/hashicorp/go-changelog/cmd/changelog-build@latest
go install golang.org/x/tools/cmd/stringer@v0.1.8

.PHONY: lint-deps
lint-deps: ## Install linter dependencies
Expand Down
75 changes: 75 additions & 0 deletions api/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,3 +494,78 @@ type HostDataResponse struct {
AgentID string
HostData *HostData `json:",omitempty"`
}

// GetSchedulerWorkerConfig returns the targeted agent's worker pool configuration
func (a *Agent) GetSchedulerWorkerConfig(q *QueryOptions) (*SchedulerWorkerPoolArgs, error) {
var resp AgentSchedulerWorkerConfigResponse
_, err := a.client.query("/v1/agent/schedulers/config", &resp, q)
if err != nil {
return nil, err
}

return &SchedulerWorkerPoolArgs{NumSchedulers: resp.NumSchedulers, EnabledSchedulers: resp.EnabledSchedulers}, nil
}

// SetSchedulerWorkerConfig attempts to update the targeted agent's worker pool configuration
func (a *Agent) SetSchedulerWorkerConfig(args SchedulerWorkerPoolArgs, q *WriteOptions) (*SchedulerWorkerPoolArgs, error) {
req := AgentSchedulerWorkerConfigRequest(args)
var resp AgentSchedulerWorkerConfigResponse

_, err := a.client.write("/v1/agent/schedulers/config", &req, &resp, q)
if err != nil {
return nil, err
}

return &SchedulerWorkerPoolArgs{NumSchedulers: resp.NumSchedulers, EnabledSchedulers: resp.EnabledSchedulers}, nil
}

type SchedulerWorkerPoolArgs struct {
NumSchedulers int
EnabledSchedulers []string
}

// AgentSchedulerWorkerConfigRequest is used to provide new scheduler worker configuration
// to a specific Nomad server. EnabledSchedulers must contain at least the `_core` scheduler
// to be valid.
type AgentSchedulerWorkerConfigRequest struct {
NumSchedulers int `json:"num_schedulers"`
EnabledSchedulers []string `json:"enabled_schedulers"`
}

// AgentSchedulerWorkerConfigResponse contains the Nomad server's current running configuration
// as well as the server's id as a convenience. This can be used to provide starting values for
// creating an AgentSchedulerWorkerConfigRequest to make changes to the running configuration.
type AgentSchedulerWorkerConfigResponse struct {
ServerID string `json:"server_id"`
NumSchedulers int `json:"num_schedulers"`
EnabledSchedulers []string `json:"enabled_schedulers"`
}

// GetSchedulerWorkersInfo returns the current status of all of the scheduler workers on
// a Nomad server.
func (a *Agent) GetSchedulerWorkersInfo(q *QueryOptions) (*AgentSchedulerWorkersInfo, error) {
var out *AgentSchedulerWorkersInfo

_, err := a.client.query("/v1/agent/schedulers", &out, q)
if err != nil {
return nil, err
}

return out, nil
}

// AgentSchedulerWorkersInfo is the response from the scheduler information endpoint containing
// a detailed status of each scheduler worker running on the server.
type AgentSchedulerWorkersInfo struct {
ServerID string `json:"server_id"`
Schedulers []AgentSchedulerWorkerInfo `json:"schedulers"`
}

// AgentSchedulerWorkerInfo holds the detailed status information for a single scheduler worker.
type AgentSchedulerWorkerInfo struct {
ID string `json:"id"`
EnabledSchedulers []string `json:"enabled_schedulers"`
Started string `json:"started"`
Status string `json:"status"`
WorkloadStatus string `json:"workload_status"`
}
48 changes: 48 additions & 0 deletions api/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package api

import (
"fmt"
"net/http"
"reflect"
"sort"
"strings"
Expand Down Expand Up @@ -456,3 +457,50 @@ func TestAgentProfile(t *testing.T) {
require.Nil(t, resp)
}
}

func TestAgent_SchedulerWorkerConfig(t *testing.T) {
t.Parallel()

c, s := makeClient(t, nil, nil)
defer s.Stop()
a := c.Agent()

config, err := a.GetSchedulerWorkerConfig(nil)
require.NoError(t, err)
require.NotNil(t, config)
newConfig := SchedulerWorkerPoolArgs{NumSchedulers: 0, EnabledSchedulers: []string{"_core", "system"}}
resp, err := a.SetSchedulerWorkerConfig(newConfig, nil)
require.NoError(t, err)
assert.NotEqual(t, config, resp)
}

func TestAgent_SchedulerWorkerConfig_BadRequest(t *testing.T) {
t.Parallel()

c, s := makeClient(t, nil, nil)
defer s.Stop()
a := c.Agent()

config, err := a.GetSchedulerWorkerConfig(nil)
require.NoError(t, err)
require.NotNil(t, config)
newConfig := SchedulerWorkerPoolArgs{NumSchedulers: -1, EnabledSchedulers: []string{"_core", "system"}}
_, err = a.SetSchedulerWorkerConfig(newConfig, nil)
require.Error(t, err)
require.Contains(t, err.Error(), fmt.Sprintf("%v (%s)", http.StatusBadRequest, "Invalid request"))
}

func TestAgent_SchedulerWorkersInfo(t *testing.T) {
t.Parallel()
c, s := makeClient(t, nil, nil)
defer s.Stop()
a := c.Agent()

info, err := a.GetSchedulerWorkersInfo(nil)
require.NoError(t, err)
require.NotNil(t, info)
defaultSchedulers := []string{"batch", "system", "sysbatch", "service", "_core"}
for _, worker := range info.Schedulers {
require.ElementsMatch(t, defaultSchedulers, worker.EnabledSchedulers)
}
}
Loading

0 comments on commit 6e61606

Please sign in to comment.