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

metrics: Make Counter & Gauge atomic #415

Merged
merged 6 commits into from
Jan 27, 2020
Merged

metrics: Make Counter & Gauge atomic #415

merged 6 commits into from
Jan 27, 2020

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Jan 26, 2020

The Counter and Gauge metrics currently hold u64 values. This
means that all uses need to gaurd access with a Mutex.

By changing both types to hold an AtomicU64, we can avoid using locks
in some cases.

@olix0r olix0r requested a review from a team January 26, 2020 20:43
@olix0r olix0r self-assigned this Jan 26, 2020
The `Counter` and `Gauge` metrics currently hold `u64` values. This
means that all uses need to gaurd access with a `Mutex`.

By changing both types to hold an `AtomicU64`, we can avoid using locks
when sharing counters between stacks.
@olix0r
Copy link
Member Author

olix0r commented Jan 26, 2020

Hmm, I probably need to correct this per #139? I'm not sure how to handle this with atomics...

Copy link
Contributor

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

yayyy! I believe this was originally waiting on AtomicU64 stabilizing, but that happened a long time ago.

I left some notes, hopefully they're helpful.

linkerd/metrics/src/gauge.rs Show resolved Hide resolved
linkerd/metrics/src/counter.rs Show resolved Hide resolved
linkerd/metrics/src/counter.rs Outdated Show resolved Hide resolved
linkerd/metrics/src/gauge.rs Show resolved Hide resolved
linkerd/metrics/src/counter.rs Show resolved Hide resolved
linkerd/metrics/src/counter.rs Outdated Show resolved Hide resolved
linkerd/metrics/src/counter.rs Outdated Show resolved Hide resolved
linkerd/app/core/src/handle_time.rs Show resolved Hide resolved
@olix0r olix0r requested a review from a team January 27, 2020 21:43
Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@olix0r olix0r merged commit 175d6f3 into master Jan 27, 2020
@olix0r olix0r deleted the ver/atomic-counter branch January 27, 2020 22:44
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Feb 4, 2020
This release fixes a bug in the proxy's logging subsystem that could
cause the proxy to consume memory until the process is OOMKilled,
especially when the proxy was configured to log diagnostic information.

The proxy also now properly emits `grpc-status` headers when signaling
proxy errors to gRPC clients.

This release upgrades the proxy's Rust version, the `http` crate
dependency to address RUSTSEC-2019-0033 and RUSTSEC-2019-0034, and the
`prost` crate dependency has been patched to address RUSTSEC-2020-02.

---

* internal: Introduce a locking middleware (linkerd/linkerd2-proxy#408)
* Update to Rust 1.40 with new Cargo.lock format (linkerd/linkerd2-proxy#410)
* Update http to v0.1.21 (linkerd/linkerd2-proxy#412)
* internal: Split retry, http-classify, and http-metrics (linkerd/linkerd2-proxy#409)
* Actually update http to v0.1.21 (linkerd/linkerd2-proxy#413)
* patch `prost` 0.5 to pick up security fix (linkerd/linkerd2-proxy#414)
* metrics: Make Counter & Gauge atomic (linkerd/linkerd2-proxy#415)
* Set grpc-status headers on dispatch errors (linkerd/linkerd2-proxy#416)
* trace: update `tracing-subscriber` to 0.2.0-alpha.4 (linkerd/linkerd2-proxy#418)
* discover: Warn on discovery error (linkerd/linkerd2-proxy#422)
* router: Avoid large up-front allocations (linkerd/linkerd2-proxy#421)
* errors: Set correct HTTP version on responses (linkerd/linkerd2-proxy#424)
* app: initialize tracing prior to parsing env vars (linkerd/linkerd2-proxy#425)
* trace: update tracing-subscriber to 0.2.0-alpha.6 (linkerd/linkerd2-proxy#423)
adleong pushed a commit to linkerd/linkerd2 that referenced this pull request Feb 4, 2020
This release fixes a bug in the proxy's logging subsystem that could
cause the proxy to consume memory until the process is OOMKilled,
especially when the proxy was configured to log diagnostic information.

The proxy also now properly emits `grpc-status` headers when signaling
proxy errors to gRPC clients.

This release upgrades the proxy's Rust version, the `http` crate
dependency to address RUSTSEC-2019-0033 and RUSTSEC-2019-0034, and the
`prost` crate dependency has been patched to address RUSTSEC-2020-02.

---

* internal: Introduce a locking middleware (linkerd/linkerd2-proxy#408)
* Update to Rust 1.40 with new Cargo.lock format (linkerd/linkerd2-proxy#410)
* Update http to v0.1.21 (linkerd/linkerd2-proxy#412)
* internal: Split retry, http-classify, and http-metrics (linkerd/linkerd2-proxy#409)
* Actually update http to v0.1.21 (linkerd/linkerd2-proxy#413)
* patch `prost` 0.5 to pick up security fix (linkerd/linkerd2-proxy#414)
* metrics: Make Counter & Gauge atomic (linkerd/linkerd2-proxy#415)
* Set grpc-status headers on dispatch errors (linkerd/linkerd2-proxy#416)
* trace: update `tracing-subscriber` to 0.2.0-alpha.4 (linkerd/linkerd2-proxy#418)
* discover: Warn on discovery error (linkerd/linkerd2-proxy#422)
* router: Avoid large up-front allocations (linkerd/linkerd2-proxy#421)
* errors: Set correct HTTP version on responses (linkerd/linkerd2-proxy#424)
* app: initialize tracing prior to parsing env vars (linkerd/linkerd2-proxy#425)
* trace: update tracing-subscriber to 0.2.0-alpha.6 (linkerd/linkerd2-proxy#423)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants