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.

Sign up for GitHub

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

Jump to bottom

Improve CPU performance measurement precision #132

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Improve CPU performance measurement precision #132

wants to merge 4 commits into from

Conversation

mergenci
Copy link
Contributor

@mergenci mergenci commented Sep 4, 2023

Description of your changes

This PR addresses issues raised in [1] and comments that follow. Results are reported in [2].

  1. Switch from node_exporter's per-node CPU metrics to cadvisor's per-container metrics.
  2. Query only experiment duration for CPU metrics.
  3. Very short running experiments may not have enough measurements for performance to be reported. Handle such cases by querying outside the experiment duration.

I have:

How has this code been tested

I ran the following command against my cluster, which has necessary software (UXP, provider-aws, prometheus) installed.

cd cmd/perf
go run main.go --mrs manifests/ecr.yaml=5 --provider-pods "$(kubectl get pods -o name | grep provider-aws | cut -d/ -f2)" --provider-namespace upbound-system --node "$(k get nodes -o jsonpath='{.items[0].status.addresses[?(@.type=="InternalIP")].address}'):9100" --step-duration 1s

@mergenci
Add missing space to sample command invocation.
3d4a958 
Signed-off-by: Cem Mergenci <cmergenci@gmail.com>
@mergenci
Clarify error message returned on assertion failure.
be99402 
Signed-off-by: Cem Mergenci <cmergenci@gmail.com>
@mergenci
Calculate duration in a method.
066f909 
Signed-off-by: Cem Mergenci <cmergenci@gmail.com>
@mergenci mergenci mentioned this pull request Sep 4, 2023
Merged
1 task
sttts
sttts reviewed Sep 5, 2023
View reviewed changes
cmd/perf/internal/quantify.go Outdated Show resolved Hide resolved
@mergenci
Report container's CPU utilization, rather than node's.
7a82110 
So far, we've been using node_exporter's metrics, which reported
metrics per node. We switch to using cadvisor's per container metrics,
which measure provider performance in isolation.

So far, we've been reporting CPU utilization as a percentage of total
CPU resources. 100% CPU utilization on an 8-CPU machine meant all
cores were fully utilized. We switch to reporting CPU utilization as a
percentage of one CPU. Full utilization of an 8-CPU machine is now
reported as 800%.

Signed-off-by: Cem Mergenci <cmergenci@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts sttts left review comments

@ulucinar ulucinar Awaiting requested review from ulucinar ulucinar is a code owner

@sergenyalcin sergenyalcin Awaiting requested review from sergenyalcin sergenyalcin is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mergenci @sttts