metrics: add more metrics for golang gc #32094

hawkingrei · 2022-02-02T16:04:11Z

Signed-off-by: Weizhen Wang wangweizhen@pingcap.com

What problem does this PR solve?

Issue Number: ref #32090

Problem Summary:

What is changed and how it works?

Intervals between garbage collections

useful to know if we can still tune. For instance, Go forces a garbage collection every 2 minutes. If your service is still having high GC impact, but you already see 120s for this graph, it means that you can no longer tune using GOGC. In this case you would need to optimize your allocations.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

> $ curl http://127.0.0.1:10080/metrics |grep golang_gc

# HELP tidb_system_golang_gc_intervals_ns intervals between GC
# TYPE tidb_system_golang_gc_intervals_ns gauge
tidb_system_golang_gc_intervals_ns 1.32817e+08
# HELP tidb_system_golang_gc_pause_ns Current GC pause percentage
# TYPE tidb_system_golang_gc_pause_ns gauge
tidb_system_golang_gc_pause_ns 0.0005715342772578194

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

ti-chi-bot · 2022-02-02T16:04:13Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

jackysp

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

sre-bot · 2022-02-02T17:08:15Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/36a13bbd07d308e135767c58e46d2cc561aadb45

util/gogc.go

jackysp

REST LGTM, but where are the unit test cases?

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

hawkingrei · 2022-02-07T10:28:09Z

REST LGTM, but where are the unit test cases?

it is difficult for this feature to add the unit tests. so I add the manual test to the PR description.

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

hawkingrei · 2022-02-08T03:28:10Z

/run-unit-test

tiancaiamao

The prometheus client has already register the Go GC metrics,
and (when displayed in grafana) they are much more sensible than a guage value like GCPausePercent and GCIntervals
https://povilasv.me/prometheus-go-metrics/

tiancaiamao · 2022-02-08T12:07:44Z

util/gogc.go

+	debug.ReadGCStats(&gc)
+	now := time.Now().UnixNano()
+	dur := float64(now - gss.last.now)
+	gcPauseRatio := float64(uint64(gc.PauseTotal)-gss.last.gcPauseTotal) / dur


If you want to know GCPausePercent as a guage metric, you don't need to record the last.gcPauseTotal
Just use: time.Since(program start) / gc.PauseTotal

it is copied from CRDB https://github.com/cockroachdb/cockroach/blob/e5eda9f716a6e1a30f17adaf96de953187030ff8/pkg/server/status/runtime.go#L523. I think this is to show the rate of change of GC duration.

tiancaiamao · 2022-02-08T12:10:33Z

util/gogc.go

+	if gss.last.lastGC.Before(gc.LastGC) {
+		gcIntervals := gc.LastGC.Sub(gss.last.lastGC)
+		gss.last.lastGC = gc.LastGC
+		metrics.GCIntervals.Set(float64(gcIntervals.Nanoseconds()))


If you want to know when GC happens, it's already there in the grafana...
And from the image the GCIntervals are obvious

You are right. I will remove it.

GC durations? I think it is different with GC intervals.

tiancaiamao · 2022-02-08T12:12:36Z

util/gogc.go

+
+// Start is used to start the sampler.
+func (gss *GOGCStatSampler) Start() {
+	ticker := time.NewTicker(100 * time.Millisecond)


debug.ReadGCStats need to hold the heap lock, so the less frequent of the calling, the better.

ti-chi-bot · 2022-02-12T21:54:54Z

@hawkingrei: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

metrics: add metrics for golang gc

afedbbd

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 2, 2022

hawkingrei added 2 commits February 3, 2022 00:08

metrics: add metrics for golang gc

38d5b96

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

metrics: add metrics for golang gc

73abc79

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

hawkingrei closed this Feb 2, 2022

hawkingrei reopened this Feb 2, 2022

metrics: add metrics for golang gc

22c5258

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

hawkingrei changed the title ~~metrics: add metrics for golang gc~~ metrics: add more metrics for golang gc Feb 2, 2022

metrics: add metrics for golang gc

36a13bb

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

jackysp reviewed Feb 7, 2022

View reviewed changes

util/gogc.go Outdated Show resolved Hide resolved

jackysp reviewed Feb 7, 2022

View reviewed changes

util/gogc.go Outdated Show resolved Hide resolved

jackysp reviewed Feb 7, 2022

View reviewed changes

fix

6537f76

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

fix

cc4e79b

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

jackysp approved these changes Feb 7, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 7, 2022

tiancaiamao reviewed Feb 8, 2022

View reviewed changes

hawkingrei added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 8, 2022

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 12, 2022

hawkingrei closed this Dec 23, 2022

hawkingrei deleted the add_gogc_metrics branch December 23, 2022 05:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

metrics: add more metrics for golang gc #32094

metrics: add more metrics for golang gc #32094

hawkingrei commented Feb 2, 2022 •

edited

Loading

ti-chi-bot commented Feb 2, 2022 •

edited

Loading

sre-bot commented Feb 2, 2022

jackysp left a comment •

edited

Loading

hawkingrei commented Feb 7, 2022

hawkingrei commented Feb 8, 2022

tiancaiamao left a comment

tiancaiamao Feb 8, 2022

hawkingrei Feb 8, 2022

tiancaiamao Feb 8, 2022

hawkingrei Feb 8, 2022

jackysp Feb 8, 2022

tiancaiamao Feb 8, 2022

ti-chi-bot commented Feb 12, 2022

metrics: add more metrics for golang gc #32094

metrics: add more metrics for golang gc #32094

Conversation

hawkingrei commented Feb 2, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Intervals between garbage collections

Check List

Release note

ti-chi-bot commented Feb 2, 2022 • edited Loading

sre-bot commented Feb 2, 2022

jackysp left a comment • edited Loading

Choose a reason for hiding this comment

hawkingrei commented Feb 7, 2022

hawkingrei commented Feb 8, 2022

tiancaiamao left a comment

Choose a reason for hiding this comment

tiancaiamao Feb 8, 2022

Choose a reason for hiding this comment

hawkingrei Feb 8, 2022

Choose a reason for hiding this comment

tiancaiamao Feb 8, 2022

Choose a reason for hiding this comment

hawkingrei Feb 8, 2022

Choose a reason for hiding this comment

jackysp Feb 8, 2022

Choose a reason for hiding this comment

tiancaiamao Feb 8, 2022

Choose a reason for hiding this comment

ti-chi-bot commented Feb 12, 2022

hawkingrei commented Feb 2, 2022 •

edited

Loading

ti-chi-bot commented Feb 2, 2022 •

edited

Loading

jackysp left a comment •

edited

Loading