Skip to content

lock: Generalize to protect a guarded value#431

Merged
olix0r merged 3 commits intomasterfrom
ver/lock-generalized
Feb 19, 2020
Merged

lock: Generalize to protect a guarded value#431
olix0r merged 3 commits intomasterfrom
ver/lock-generalized

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented Feb 19, 2020

We used Tokio's Lock implementation in the router's cache
implementation, though we know it can leak under contention.

This change generalizes the Lock to return a guarded value (like Tokio's
lock). This change simplifies the Lock's state management: the Lock may
no longer hold a value, nor can it fail.

The lock::Service implementation now holds a Result<Service, ServiceError> so that lock services may still broadcast the inner
service's failure.

We used Tokio's Lock implementation in the router's cache
implementation, though we know it can leak under contention.

This change generalizes the Lock to return a guarded value (like Tokio's
lock). This change simplifies the Lock's state management: the Lock may
no longer hold a value, nor can it fail.

The `lock::Service` implementation now holds a `Result<Service,
ServiceError>` so that lock services may still broadcast the inner
service's failure.
@olix0r olix0r requested a review from a team February 19, 2020 18:21
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.

lgtm!

@olix0r olix0r requested a review from seanmonstar February 19, 2020 20:34
@olix0r olix0r merged commit edb1f52 into master Feb 19, 2020
@olix0r olix0r deleted the ver/lock-generalized branch February 19, 2020 21:05
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Feb 19, 2020
This release includes the results from continued profiling & performance
analysis. In addition to modifying internals to prevent unwarranted
memory growth, we've introduced new metrics to aid in debugging and
diagnostics: a new `request_errors_total` metric exposes the number of
requests that receive synthesized responses due to proxy errors; and a
suite of `stack_*` metrics expose proxy internals that can help us
identify unexpected behavior.

---

* trace: update `tracing-subscriber` dependency to 0.2.1 (linkerd/linkerd2-proxy#426)
* Reimplement the Lock middleware with tokio::sync (linkerd/linkerd2-proxy#427)
* Add the request_errors_total metric (linkerd/linkerd2-proxy#417)
* Expose the number of service instances in the proxy (linkerd/linkerd2-proxy#428)
* concurrency-limit: Share a limit across Services (linkerd/linkerd2-proxy#429)
* profiling: add benchmark and profiling scripts (linkerd/linkerd2-proxy#406)
* http-box: Box HTTP payloads via middleware (linkerd/linkerd2-proxy#430)
* lock: Generalize to protect a guarded value (linkerd/linkerd2-proxy#431)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Feb 19, 2020
This release includes the results from continued profiling & performance
analysis. In addition to modifying internals to prevent unwarranted
memory growth, we've introduced new metrics to aid in debugging and
diagnostics: a new `request_errors_total` metric exposes the number of
requests that receive synthesized responses due to proxy errors; and a
suite of `stack_*` metrics expose proxy internals that can help us
identify unexpected behavior.

---

* trace: update `tracing-subscriber` dependency to 0.2.1 (linkerd/linkerd2-proxy#426)
* Reimplement the Lock middleware with tokio::sync (linkerd/linkerd2-proxy#427)
* Add the request_errors_total metric (linkerd/linkerd2-proxy#417)
* Expose the number of service instances in the proxy (linkerd/linkerd2-proxy#428)
* concurrency-limit: Share a limit across Services (linkerd/linkerd2-proxy#429)
* profiling: add benchmark and profiling scripts (linkerd/linkerd2-proxy#406)
* http-box: Box HTTP payloads via middleware (linkerd/linkerd2-proxy#430)
* lock: Generalize to protect a guarded value (linkerd/linkerd2-proxy#431)
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