Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eval delete: move batching of deletes into RPC handler and state #15117

Merged
merged 7 commits into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/15117.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
cli: Improved performance of eval delete with large filter sets
```
15 changes: 15 additions & 0 deletions api/evaluations.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ func (e *Evaluations) Delete(evalIDs []string, w *WriteOptions) (*WriteMeta, err
return wm, nil
}

// DeleteOpts is used to batch delete evaluations using a filter.
func (e *Evaluations) DeleteOpts(req *EvalDeleteRequest, w *WriteOptions) (*EvalDeleteResponse, *WriteMeta, error) {
resp := &EvalDeleteResponse{}
wm, err := e.client.delete("/v1/evaluations", &req, resp, w)
if err != nil {
return nil, nil, err
}
return resp, wm, nil
}

// Allocations is used to retrieve a set of allocations given
// an evaluation ID.
func (e *Evaluations) Allocations(evalID string, q *QueryOptions) ([]*AllocationListStub, *QueryMeta, error) {
Expand Down Expand Up @@ -140,9 +150,14 @@ type EvaluationStub struct {

type EvalDeleteRequest struct {
EvalIDs []string
Filter string
WriteRequest
}

type EvalDeleteResponse struct {
Count int
}

type EvalCountResponse struct {
Count int
QueryMeta
Expand Down
21 changes: 14 additions & 7 deletions command/agent/eval_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,19 @@ func (s *HTTPServer) evalsDeleteRequest(resp http.ResponseWriter, req *http.Requ

numIDs := len(args.EvalIDs)

// Ensure the number of evaluation IDs included in the request is within
// bounds.
if numIDs < 1 {
return nil, CodedError(http.StatusBadRequest, "request does not include any evaluation IDs")
} else if numIDs > structs.MaxUUIDsPerWriteRequest {
if args.Filter != "" && numIDs > 0 {
return nil, CodedError(http.StatusBadRequest,
"evals cannot be deleted by both ID and filter")
}
if args.Filter == "" && numIDs == 0 {
return nil, CodedError(http.StatusBadRequest,
"evals must be deleted by either ID or filter")
}

// If an explicit list of evaluation IDs is sent, ensure its within bounds
if numIDs > structs.MaxUUIDsPerWriteRequest {
return nil, CodedError(http.StatusBadRequest, fmt.Sprintf(
"request includes %v evaluations IDs, must be %v or fewer",
"request includes %v evaluation IDs, must be %v or fewer",
numIDs, structs.MaxUUIDsPerWriteRequest))
}

Expand All @@ -73,8 +79,9 @@ func (s *HTTPServer) evalsDeleteRequest(resp http.ResponseWriter, req *http.Requ
if err := s.agent.RPC(structs.EvalDeleteRPCMethod, &args, &reply); err != nil {
return nil, err
}

setIndex(resp, reply.Index)
return nil, nil
return reply, nil
}

func (s *HTTPServer) EvalSpecificRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
Expand Down
10 changes: 6 additions & 4 deletions command/agent/eval_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func TestHTTP_EvalsDelete(t *testing.T) {
// Make the request and check the response.
obj, err := s.Server.EvalsRequest(respW, req)
require.Equal(t,
CodedError(http.StatusBadRequest, "request does not include any evaluation IDs"), err)
CodedError(http.StatusBadRequest, "evals must be deleted by either ID or filter"), err)
require.Nil(t, obj)
})
},
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestHTTP_EvalsDelete(t *testing.T) {
obj, err := s.Server.EvalsRequest(respW, req)
require.Equal(t,
CodedError(http.StatusBadRequest,
"request includes 8000 evaluations IDs, must be 7281 or fewer"), err)
"request includes 8000 evaluation IDs, must be 7281 or fewer"), err)
require.Nil(t, obj)
})
},
Expand Down Expand Up @@ -223,8 +223,10 @@ func TestHTTP_EvalsDelete(t *testing.T) {

// Make the request and check the response.
obj, err := s.Server.EvalsRequest(respW, req)
require.Nil(t, err)
require.Nil(t, obj)
require.NoError(t, err)
require.NotNil(t, obj)
deleteResp := obj.(structs.EvalDeleteResponse)
require.Equal(t, deleteResp.Count, 1)

// Ensure the eval is not found.
readEval, err := s.Agent.server.State().EvalByID(nil, mockEval.ID)
Expand Down
150 changes: 36 additions & 114 deletions command/eval_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"fmt"
"strings"
"time"

"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/api/contexts"
Expand Down Expand Up @@ -148,32 +147,7 @@ func (e *EvalDeleteCommand) Run(args []string) int {
e.deleteByArg = true
exitCode, err = e.handleEvalArgDelete(args[0])
default:

// Track the next token, so we can iterate all pages that match the
// passed filter.
var nextToken string

// It is possible the filter matches a large number of evaluations
// which means we need to run a number of batch deletes. Perform
// iteration here rather than recursion in later function, so we avoid
// any potential issues with stack size limits.
for {
exitCode, nextToken, err = e.handleFlagFilterDelete(nextToken)

// If there is another page of evaluations matching the filter,
// iterate the loop and delete the next batch of evals. We pause
// for a 500ms rather than just run as fast as the code and machine
// possibly can. This means deleting 13million evals will take
// roughly 13-15 mins, which seems reasonable. It is worth noting,
// we do not expect operators to delete this many evals in a single
// run and expect more careful filtering options to be used.
if nextToken != "" {
time.Sleep(500 * time.Millisecond)
continue
} else {
break
}
}
exitCode, err = e.handleDeleteByFilter(e.filter)
}

// Do not exit if we got an error as it's possible this was on the
Expand Down Expand Up @@ -228,93 +202,6 @@ func (e *EvalDeleteCommand) handleEvalArgDelete(evalID string) (int, error) {
return code, err
}

// handleFlagFilterDelete handles deletion of evaluations discovered using
// the filter. It is unknown how many will match the operator criteria so
// this function batches lookup and delete requests into sensible numbers.
func (e *EvalDeleteCommand) handleFlagFilterDelete(nt string) (int, string, error) {

evalsToDelete, nextToken, err := e.batchLookupEvals(nt)
if err != nil {
return 1, "", err
}

numEvalsToDelete := len(evalsToDelete)

// The filter flags are operator controlled, therefore ensure we
// actually found some evals to delete. Otherwise, inform the operator
// their flags are potentially incorrect.
if numEvalsToDelete == 0 {
if e.numDeleted > 0 {
return 0, "", nil
} else {
return 1, "", errors.New("failed to find any evals that matched filter criteria")
}
}

if code, actioned, err := e.batchDelete(evalsToDelete); err != nil {
return code, "", err
} else if !actioned {
return code, "", nil
}

e.Ui.Info(fmt.Sprintf("Successfully deleted batch of %v %s",
numEvalsToDelete, correctGrammar("evaluation", numEvalsToDelete)))

return 0, nextToken, nil
}

// batchLookupEvals handles batched lookup of evaluations using the operator
// provided filter. The lookup is performed a maximum number of 3 times to
// ensure their size is limited and the number of evals to delete doesn't exceed
// the total allowable in a single call.
//
// The JSON serialized evaluation API object is 350-380B in size.
// 2426 * 380B (3.8e-4 MB) = 0.92MB. We may want to make this configurable
// in the future, but this is counteracted by the CLI logic which will loop
// until the user tells it to exit, or all evals matching the filter are
// deleted. 2426 * 3 falls below the maximum limit for eval IDs in a single
// delete request (set by MaxEvalIDsPerDeleteRequest).
func (e *EvalDeleteCommand) batchLookupEvals(nextToken string) ([]*api.Evaluation, string, error) {

var evalsToDelete []*api.Evaluation
currentNextToken := nextToken

// Call List 3 times to accumulate the maximum number if eval IDs supported
// in a single Delete request. See math above.
for i := 0; i < 3; i++ {

// Generate the query options using the passed next token and filter. The
// per page value is less than the total number we can include in a single
// delete request. This keeps the maximum size of the return object at a
// reasonable size.
opts := api.QueryOptions{
Filter: e.filter,
PerPage: 2426,
NextToken: currentNextToken,
}

evalList, meta, err := e.client.Evaluations().List(&opts)
if err != nil {
return nil, "", err
}

if len(evalList) > 0 {
evalsToDelete = append(evalsToDelete, evalList...)
}

// Store the next token no matter if it is empty or populated.
currentNextToken = meta.NextToken

// If there is no next token, ensure we exit and avoid any new loops
// which will result in duplicate IDs.
if currentNextToken == "" {
break
}
}

return evalsToDelete, currentNextToken, nil
}

// batchDelete is responsible for deleting the passed evaluations and asking
// any confirmation questions along the way. It will ask whether the operator
// want to list the evals before deletion, and optionally ask for confirmation
Expand Down Expand Up @@ -404,3 +291,38 @@ func correctGrammar(word string, num int) string {
}
return word
}

func (e *EvalDeleteCommand) handleDeleteByFilter(filterExpr string) (int, error) {

// If the user did not wish to bypass the confirmation step, ask this now
// and handle the response.
if !e.yes && !e.deleteByArg {

resp, _, err := e.client.Evaluations().Count(&api.QueryOptions{
Filter: filterExpr,
})
if err != nil {
return 1, err
}

code, deleteEvals := e.askQuestion(fmt.Sprintf(
"Are you sure you want to delete %d evals? [y/N]",
resp.Count), "Cancelling eval deletion")
e.Ui.Output("")

if !deleteEvals {
return code, nil
}
}

resp, _, err := e.client.Evaluations().DeleteOpts(&api.EvalDeleteRequest{
Filter: filterExpr,
}, nil)
if err != nil {
return 1, err
}
e.numDeleted = resp.Count

return 0, nil

}
Loading