Skip to content

Commit

Permalink
client: prevent using stale allocs
Browse files Browse the repository at this point in the history
Similar to #18269, it is possible that even if Node.GetClientAllocs
retrieves fresh allocs that the subsequent Alloc.GetAllocs call
retrieves stale allocs. While `diffAlloc(existing, updated)` properly
ignores stale alloc *updates*, alloc deletions have no such check.

So if a client retrieves an alloc created at index 123, and then a
subsequent Alloc.GetAllocs call hits a new server which returns results
at index 100, the client will stop the alloc created at 123 because it
will be missing from the stale response.

This change applies the same logic as #18269 and ensures only fresh
responses are used.

Glossary:
* fresh - modified at an index > the query index
* stale - modified at an index <= the query index
  • Loading branch information
schmichael committed Sep 27, 2023
1 parent 868aba5 commit 76ab3a9
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
18 changes: 18 additions & 0 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2419,6 +2419,24 @@ OUTER:
}
}

// It is possible that Alloc.GetAllocs hits a different server than
// Node.GetClientAllocs which returns older results.
if allocsResp.Index <= allocsReq.MinQueryIndex {
retry := c.retryIntv(getAllocRetryIntv)
c.logger.Warn("failed to retrieve updated allocs; retrying",
"req_index", allocsReq.MinQueryIndex,
"resp_index", allocsResp.Index,
"num_allocs", len(pull),
"wait", retry,
)
select {
case <-time.After(retry):
continue
case <-c.shutdownCh:
return
}
}

// Ensure that we received all the allocations we wanted
pulledAllocs = make(map[string]*structs.Allocation, len(allocsResp.Allocs))
for _, alloc := range allocsResp.Allocs {
Expand Down
2 changes: 1 addition & 1 deletion nomad/alloc_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func (a *Alloc) GetAllocs(args *structs.AllocsGetRequest,
reply.Allocs = allocs
reply.Index = maxIndex
} else {
// Use the last index that affected the nodes table
// Use the last index that affected the allocs table
index, err := state.Index("allocs")
if err != nil {
return err
Expand Down

0 comments on commit 76ab3a9

Please sign in to comment.