Non-locked accessors to common Node fields #3195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chelseakomlo
approved these changes
Sep 14, 2017
client/client.go
Outdated
| c.configLock.RLock() | ||
| dc := c.configCopy.Node.Datacenter | ||
| c.configLock.RUnlock() | ||
| dc := c.config.Node.Datacenter |
There was a problem hiding this comment.
Return c.config.Node.Datacenter instead of assigning first to a variable
|
|
||
| // secretNodeID returns the secret node ID for the given client | ||
| func (c *Client) secretNodeID() string { | ||
| return c.config.Node.SecretID |
There was a problem hiding this comment.
Could Node ever be nil?
There was a problem hiding this comment.
Nope the agent instantiates
| @@ -92,6 +92,9 @@ func NewHostStatsCollector(logger *log.Logger, allocDir string) *HostStatsCollec | |||
|
|
|||
| // Collect collects stats related to resource usage of a host | |||
| func (h *HostStatsCollector) Collect() error { | |||
| h.hostStatsLock.Lock() | |||
| defer h.hostStatsLock.Unlock() | |||
This PR removes locking around commonly accessed node attributes that do not need to be locked. The locking could cause nodes to TTL as the heartbeat code path was acquiring a lock that could be held for an excessively long time. An example of this is when Vault is inaccessible, since the fingerprint is run with a lock held but the Vault fingerprinter makes the API calls with a large timeout. Fixes #2689
dadgar
force-pushed
the
b-node-locking
branch
from
10cc678 to
98c47c7
Compare
hsmade
added a commit
to hsmade/nomad
that referenced
this pull request
Oct 12, 2017
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes locking around commonly accessed node attributes that do
not need to be locked. The locking could cause nodes to TTL as the
heartbeat code path was acquiring a lock that could be held for an
excessively long time. An example of this is when Vault is inaccessible,
since the fingerprint is run with a lock held but the Vault
fingerprinter makes the API calls with a large timeout.
Fixes #2689