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

stack: Add BoxCloneSyncService #2523

Merged
merged 4 commits into from
Nov 17, 2023
Merged

stack: Add BoxCloneSyncService #2523

merged 4 commits into from
Nov 17, 2023

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Nov 16, 2023

tower::util::BoxCloneService doesn't implement Sync, which is necessary when we may want to share these services across threads (which is entirely the point of making a service cloneable, really).

This change replicates BoxCloneService as BoxCloneSyncService and updates many of Linkerd's stacks to return this new type.

This continues on the work begun in #2510 and #2514.


Building proxy with full debug symbols:

:; RUSTFLAGS='-C debuginfo=2' just profile=release build              

Before:

:; du -h target/x86_64-unknown-linux-gnu/release/linkerd2-proxy{,.dbg}
20M     target/x86_64-unknown-linux-gnu/release/linkerd2-proxy
1.5G    target/x86_64-unknown-linux-gnu/release/linkerd2-proxy.dbg

After:

:; du -h target/x86_64-unknown-linux-gnu/release/linkerd2-proxy{,.dbg}
20M     target/x86_64-unknown-linux-gnu/release/linkerd2-proxy
904M    target/x86_64-unknown-linux-gnu/release/linkerd2-proxy.dbg

Result: >500MB saved for debug symbols.

tower::util::BoxCloneService doesn't implement Sync, which is necessary
when we may want to share these services across threads (which is
entirely the point of making a service cloneable, really).

This change replicates BoxCloneService as BoxCloneSyncService and
updates many of Linkerd's stacks to return this new type.
@olix0r olix0r requested a review from a team as a code owner November 16, 2023 20:55
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.

looks great!

@olix0r olix0r merged commit e969ae8 into main Nov 17, 2023
15 checks passed
@olix0r olix0r deleted the ver/box-clown branch November 17, 2023 19:19
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Nov 30, 2023
* Add a .codecov.yml (linkerd/linkerd2-proxy#2527)
* stack: Add BoxCloneSyncService (linkerd/linkerd2-proxy#2523)
* ci: Compute coverage over all tests (linkerd/linkerd2-proxy#2528)
* stack-metrics: Implement Clone for TrackService (linkerd/linkerd2-proxy#2524)
* ci: Fetch tarpaulin binary instead of compiling (linkerd/linkerd2-proxy#2532)
* ci: Enable coverage on main and all PRs (linkerd/linkerd2-proxy#2533)
* test: Cleanup consecutive_failures_accrue (linkerd/linkerd2-proxy#2531)
* test: Improve error reporting in gauges_endpoints (linkerd/linkerd2-proxy#2530)
* test: Cleanup outbound_balancer_waits_for_ready_endpoint (linkerd/linkerd2-proxy#2529)
* meshtls: Consolidate TLS ID verification (linkerd/linkerd2-proxy#2534)
* build(deps): bump DavidAnson/markdownlint-cli2-action (linkerd/linkerd2-proxy#2537)
* build(deps): bump tj-actions/changed-files from 40.1.1 to 40.2.0 (linkerd/linkerd2-proxy#2536)

Signed-off-by: Oliver Gould <[email protected]>
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Nov 30, 2023
* Add a .codecov.yml (linkerd/linkerd2-proxy#2527)
* stack: Add BoxCloneSyncService (linkerd/linkerd2-proxy#2523)
* ci: Compute coverage over all tests (linkerd/linkerd2-proxy#2528)
* stack-metrics: Implement Clone for TrackService (linkerd/linkerd2-proxy#2524)
* ci: Fetch tarpaulin binary instead of compiling (linkerd/linkerd2-proxy#2532)
* ci: Enable coverage on main and all PRs (linkerd/linkerd2-proxy#2533)
* test: Cleanup consecutive_failures_accrue (linkerd/linkerd2-proxy#2531)
* test: Improve error reporting in gauges_endpoints (linkerd/linkerd2-proxy#2530)
* test: Cleanup outbound_balancer_waits_for_ready_endpoint (linkerd/linkerd2-proxy#2529)
* meshtls: Consolidate TLS ID verification (linkerd/linkerd2-proxy#2534)
* build(deps): bump DavidAnson/markdownlint-cli2-action (linkerd/linkerd2-proxy#2537)
* build(deps): bump tj-actions/changed-files from 40.1.1 to 40.2.0 (linkerd/linkerd2-proxy#2536)

Signed-off-by: Oliver Gould <[email protected]>
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.

2 participants