-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Replaced ClusterMetricSink's cluster name with an atomic.Value. #9252
Conversation
This should permit go-race tests to pass which seal and unseal the core.
Another failure, in an odd place, this is @sgmiller's new code:
Does metricSink get modified by seal or unseal?
|
Ah, that test swaps in a different metricSink after the core is unsealed. Need to think about how to do this in a race-free manner. |
@@ -59,17 +63,30 @@ func (m *ClusterMetricSink) MeasureSinceWithLabels(key []string, start time.Time | |||
func BlackholeSink() *ClusterMetricSink { | |||
sink, _ := metrics.New(metrics.DefaultConfig(""), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we do return NewClusterMetricSink("", sink)
over here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good point, I added the second factory later and then forgot to refactor this one. I'll fix that and commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, I'll commit now and fix later.
* master: (31 commits) changelog++ changelog++ Ui/replication status discoverability (#8705) Update CHANGELOG.md Counter that increments on every secret engine lease creation. (#9244) Add password_policy field to Azure docs (#9249) Replaced ClusterMetricSink's cluster name with an atomic.Value. (#9252) Fix database creds rotation panic for nil resp (#9258) changelog++ changelog++ Move sdk/helper/random -> helper/random (#9226) UI: Disallow kv2 with too large 'max versions' value (#9242) Allow mTLS for mysql secrets engine (#9181) docs: add sample revocation for mongodb (#9245) Add new Telemetry config options (#9238) Add a simple sealed gauge, updated when seal status changes (#9177) Test Shamir-to-Transit and Transit-to-Shamir Seal Migration for post-1.4 Vault. (#9214) Configure metrics wrapper with the "global" object, not just the fanout. (#9099) changelog++ Add backend type to audit logs (#9167) ...
* Replaced ClusterMetricSink's cluster name with an atomic.Value. This should permit go-race tests to pass which seal and unseal the core. * Replace metric sink before unseal to avoid data races.
Replaced ClusterMetricSink's cluster name with an atomic.Value. This should permit go-race tests to pass which seal and unseal the core.
Alternatives considered:
The approach here seems like the least heavyweight solution to the potential race.