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

client: prevent watching stale alloc state #18612

Merged
merged 4 commits into from
Sep 29, 2023
Merged

Conversation

schmichael
Copy link
Member

When waiting on a previous alloc we must query against the leader before switching to a stale query with index set.

Also check to ensure the response is fresh before using it like #18269

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

However, note the semgrep error on time.After. The NewSafeTimer helper provides a safer abstraction that provides compiler support in avoiding leaking the goroutine.

@schmichael schmichael added backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line labels Sep 29, 2023
When waiting on a previous alloc we must query against the leader before
switching to a stale query with index set.

Also check to ensure the response is fresh before using it like #18269
Comment on lines -2433 to +2434
case <-time.After(retry):
case <-timer.C:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this is an unrelated fix from a linting error I introduced in my previous PR: #18601

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@schmichael schmichael merged commit 3f9bd17 into main Sep 29, 2023
24 of 25 checks passed
@schmichael schmichael deleted the b-allocwatcher-index branch September 29, 2023 19:46
schmichael added a commit that referenced this pull request Sep 29, 2023
When waiting on a previous alloc we must query against the leader before
switching to a stale query with index set.

Also check to ensure the response is fresh before using it like #18269
schmichael added a commit that referenced this pull request Sep 29, 2023
When waiting on a previous alloc we must query against the leader before
switching to a stale query with index set.

Also check to ensure the response is fresh before using it like #18269
schmichael added a commit that referenced this pull request Sep 29, 2023
When waiting on a previous alloc we must query against the leader before
switching to a stale query with index set.

Also check to ensure the response is fresh before using it like #18269
@schmichael
Copy link
Member Author

Backports failed: https://github.com/hashicorp/nomad/actions/runs/6356400700

Manual 1.4.x backport: e531bf1

Manual 1.5.x backport: 376a350

Manual 1.6.x backport: b6f62d5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants