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

Refactor metrics to have sharded shared state #445

Merged
merged 3 commits into from
Aug 16, 2023

Conversation

jamesbornholt
Copy link
Member

Description of change

I wanted to add some new metrics but they are gauges, and our current
thread-local approach makes gauges hard -- each thread gets its own copy
of the gauge. So instead, this change refactors the metrics
infrastructure to have just one copy of each metric, stored in a sharded
map to hopefully reduce contention.

I didn't actually add any new metrics, so this should be a pure
refactoring change. I beefed up the tests a little bit, too.

Does this change impact existing behavior?

No, refactoring only.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

I wanted to add some new metrics but they are gauges, and our current
thread-local approach makes gauges hard -- each thread gets its own copy
of the gauge. So instead, this change refactors the metrics
infrastructure to have just one copy of each metric, stored in a sharded
map to hopefully reduce contention.

I didn't actually add any new metrics, so this should be a pure
refactoring change. I beefed up the tests a little bit, too.

Signed-off-by: James Bornholt <[email protected]>
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 11, 2023 04:01 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 11, 2023 04:01 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 11, 2023 04:01 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 11, 2023 04:01 — with GitHub Actions Inactive
Signed-off-by: James Bornholt <[email protected]>
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 11, 2023 05:04 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 11, 2023 05:04 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 11, 2023 05:04 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 11, 2023 05:04 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt marked this pull request as ready for review August 11, 2023 05:24
@dannycjones dannycjones added the performance PRs to run benchmarks on label Aug 11, 2023
@dannycjones dannycjones temporarily deployed to PR benchmarks August 11, 2023 13:43 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR benchmarks August 11, 2023 13:43 — with GitHub Actions Inactive
@dannycjones dannycjones requested review from monthonk and removed request for dannycjones August 14, 2023 10:57
Copy link
Contributor

@monthonk monthonk left a comment

Choose a reason for hiding this comment

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

LGTM, just one typo.

Signed-off-by: James Bornholt <[email protected]>
@jamesbornholt jamesbornholt temporarily deployed to PR benchmarks August 16, 2023 01:59 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR benchmarks August 16, 2023 01:59 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 16, 2023 01:59 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 16, 2023 01:59 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 16, 2023 01:59 — with GitHub Actions Inactive
@jamesbornholt jamesbornholt temporarily deployed to PR integration tests August 16, 2023 01:59 — with GitHub Actions Inactive
@monthonk monthonk enabled auto-merge August 16, 2023 08:49
@monthonk monthonk added this pull request to the merge queue Aug 16, 2023
Merged via the queue into awslabs:main with commit 35d23e9 Aug 16, 2023
muddyfish added a commit to muddyfish/mountpoint-s3 that referenced this pull request Sep 18, 2024
Revert "Revert "Implement runtime check on libcrypto linkage (awslabs#186)" (awslabs#195)

Co-authored-by: WillChilds-Klein <[email protected]>
Bump the minimum stack size to at least 1MB (awslabs#1139)

BUILDER_VERSION: v0.9.55 (awslabs#66)

adapt change from "TLS deliver buffer data during shutdown" (awslabs#474)

Use const pointers in `secure_channel_tls_handler.c`. (awslabs#664)

Co-authored-by: Waqar Ahmed Khan <[email protected]>
Make options more const (awslabs#445)

Remove is ipv4/ipv6 functions utils from sdkutils to c-common (awslabs#39)

BUILDER_VERSION: v0.9.55 (awslabs#80)

Prepare release v1.32.0 (#1700)

### Description of changes:
Preparing for AWS-LC v1.32.0 release.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
Make `Dockerfile.source` actually build from source

- Dockerfile.source now builds based on centos:7
- Add `/` to path so you can run `mount-s3` without a /
- Adds `build-image` and `push-image` targets to makefile

Signed-off-by: Simon Beal <[email protected]>
chore: Bump Rust bindings v1.4.18 (#4656)
Signed-off-by: Simon Beal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance PRs to run benchmarks on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants