Scheduler metrics: binding rate limiter saturation rate #17673

hongchaodeng · 2015-11-23T19:24:31Z

This PR exposes the pod binding rate. We are exposing it because there is a rate limiter. We want to measure and see how long before it becomes a bottleneck.

k8s-bot · 2015-11-23T19:25:09Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

k8s-bot · 2015-11-23T19:26:09Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

k8s-github-robot · 2015-11-23T19:38:16Z

Labelling this PR as size/M

xiang90 · 2015-11-23T19:47:25Z

plugin/pkg/scheduler/scheduler.go

@@ -107,6 +107,13 @@ func New(c *Config) *Scheduler {

 // Run begins watching and scheduling. It starts a goroutine and returns immediately.
 func (s *Scheduler) Run() {
+	go func() {


can we move this to metrics.go and add a func called ReportRateLoop or something like that?

k8s-github-robot · 2015-11-23T19:48:16Z

Labelling this PR as size/S

k8s-github-robot · 2015-11-23T21:08:17Z

Labelling this PR as size/M

hongchaodeng · 2015-11-24T00:04:43Z

Made some wrong assumptions on rate limiter.
Working on better solutions now.

hongchaodeng · 2015-11-24T02:21:50Z

Once juju/ratelimit#10 is merged, I should be able to continue on this work:

bump up juju/ratelimit version
expose Available() and Capacity() and use it to export rate limiter's saturation metrics

davidopp · 2015-11-24T06:48:49Z

@wojtek-t

davidopp · 2015-11-24T06:51:54Z

@gmarek

wojtek-t · 2015-11-24T09:22:44Z

plugin/pkg/scheduler/metrics/metrics.go

 	"github.com/prometheus/client_golang/prometheus"
 )

 const schedulerSubsystem = "scheduler"

+var BindingRateObserveInterval = 200 * time.Millisecond


make it const

wojtek-t · 2015-11-30T10:25:54Z

@hongchaodeng - what's the status of it? Do you want to proceed with it?

hongchaodeng · 2015-11-30T17:36:56Z

@wojtek-t
I'm currently using it along with other metrics for insights.
Let's just close this.
I will send it in another PR once I have more complete story.
Thanks!

xiang90 · 2015-11-30T17:41:22Z

@hongchaodeng I think we should push in the rate limiter saturation metrics anyway.

wojtek-t · 2015-12-02T18:19:00Z

Thanks - LGTM

wojtek-t · 2015-12-03T08:05:36Z

@k8s-bot ok to test

k8s-bot · 2015-12-03T08:37:04Z

GCE e2e test build/test passed for commit ee39839d8ec7ab3e2fd9d6061c4e6645c8911975.

gmarek · 2015-12-03T08:41:40Z

Test failure looks like a real thing:

I1203 08:30:12.853367   22739 factory.go:173] creating scheduler with fit predicates 'map[NoDiskConflict:{} MatchNodeSelector:{} HostName:{} PodFitsHostPorts:{} PodFitsResources:{}]' and priority functions 'map[LeastRequestedPriority:{} BalancedResourceAllocation:{} SelectorSpreadPriority:{}]
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x28 pc=0x745de1]

goroutine 55 [running]:
k8s.io/kubernetes/plugin/pkg/scheduler.func·002()
    /workspace/kubernetes/_output/local/go/src/k8s.io/kubernetes/plugin/pkg/scheduler/scheduler.go:111 +0x41
k8s.io/kubernetes/pkg/util.func·006()
    /workspace/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/util/util.go:126 +0x61
k8s.io/kubernetes/pkg/util.Until(0xc2088fc1a0, 0x3b9aca00, 0xc20804c480)
    /workspace/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/util/util.go:127 +0x8f
k8s.io/kubernetes/pkg/util.Forever(0xc2088fc1a0, 0x3b9aca00)
    /workspace/kubernetes/_output/local/go/src/k8s.io/kubernetes/pkg/util/util.go:109 +0x3e
created by k8s.io/kubernetes/plugin/pkg/scheduler.(*Scheduler).Run
    /workspace/kubernetes/_output/local/go/src/k8s.io/kubernetes/plugin/pkg/scheduler/scheduler.go:113 +0xa7

hongchaodeng · 2015-12-03T16:43:08Z

How to reproduce that?

gmarek · 2015-12-03T16:47:39Z

It happened during the integration test, so hack/test-integration.sh should do the trick.

hongchaodeng · 2015-12-03T18:41:55Z

ratelimiter could be nil. See:

kubernetes/test/integration/extender_test.go

Line 241 in 41ebe6f

schedulerConfigFactory := factory.NewConfigFactory(restClient, nil)

We should add a caveat comment.

hongchaodeng · 2015-12-03T18:48:29Z

Fixed and tested
@gmarek

k8s-bot · 2015-12-03T19:21:13Z

GCE e2e test build/test passed for commit c4fdb7a.

wojtek-t · 2015-12-03T19:52:15Z

@hongchaodeng - thanks!

LGTM

k8s-github-robot · 2015-12-04T11:15:56Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2015-12-04T11:53:32Z

GCE e2e build/test failed for commit c4fdb7a.

gmarek · 2015-12-04T12:37:48Z

@k8s-bot test this

k8s-bot · 2015-12-04T13:13:13Z

GCE e2e test build/test passed for commit c4fdb7a.

k8s-github-robot · 2015-12-05T12:56:57Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2015-12-05T13:29:54Z

GCE e2e test build/test passed for commit c4fdb7a.

k8s-github-robot · 2015-12-05T13:30:08Z

Automatic merge from submit-queue

Auto commit by PR queue bot

googlebot added the cla: yes label Nov 23, 2015

hongchaodeng force-pushed the metrics branch from 138b381 to 1929a04 Compare November 23, 2015 19:30

k8s-github-robot assigned davidopp Nov 23, 2015

k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 23, 2015

hongchaodeng force-pushed the metrics branch from 1929a04 to 80cfde7 Compare November 23, 2015 19:46

xiang90 reviewed Nov 23, 2015
View reviewed changes

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 23, 2015

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 23, 2015

hongchaodeng force-pushed the metrics branch from 2567f6a to 84a5b0b Compare November 23, 2015 21:10

hongchaodeng changed the title ~~Metrics: expose pod binding rate~~ [WIP] Metrics: expose pod binding rate Nov 24, 2015

davidopp assigned wojtek-t and unassigned davidopp Nov 24, 2015

wojtek-t reviewed Nov 24, 2015
View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 26, 2015

hongchaodeng closed this Nov 30, 2015

wojtek-t closed this Dec 2, 2015

wojtek-t reopened this Dec 2, 2015

wojtek-t added lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-merge labels Dec 3, 2015

gmarek removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2015

hongchaodeng added 3 commits December 3, 2015 08:39

godep: bump up juju/ratelimit

2edcd3a

change tickRateLimiter to tokenBucketRateLimiter

9781a83

Rate limiter: Add Saturation() method

ba80892

Scheduler metrics: binding rate limiter saturation

c4fdb7a

hongchaodeng force-pushed the metrics branch from ee39839 to c4fdb7a Compare December 3, 2015 18:43

wojtek-t added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2015

k8s-github-robot pushed a commit that referenced this pull request Dec 5, 2015

Merge pull request #17673 from hongchaodeng/metrics

3180b00

Auto commit by PR queue bot

k8s-github-robot merged commit 3180b00 into kubernetes:master Dec 5, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scheduler metrics: binding rate limiter saturation rate #17673

Scheduler metrics: binding rate limiter saturation rate #17673

hongchaodeng commented Nov 23, 2015

k8s-bot commented Nov 23, 2015

k8s-bot commented Nov 23, 2015

k8s-github-robot commented Nov 23, 2015

xiang90 Nov 23, 2015

k8s-github-robot commented Nov 23, 2015

k8s-github-robot commented Nov 23, 2015

hongchaodeng commented Nov 24, 2015

hongchaodeng commented Nov 24, 2015

davidopp commented Nov 24, 2015

davidopp commented Nov 24, 2015

wojtek-t Nov 24, 2015

wojtek-t commented Nov 30, 2015

hongchaodeng commented Nov 30, 2015

xiang90 commented Nov 30, 2015

wojtek-t commented Dec 2, 2015

wojtek-t commented Dec 3, 2015

k8s-bot commented Dec 3, 2015

gmarek commented Dec 3, 2015

hongchaodeng commented Dec 3, 2015

gmarek commented Dec 3, 2015

hongchaodeng commented Dec 3, 2015

hongchaodeng commented Dec 3, 2015

k8s-bot commented Dec 3, 2015

wojtek-t commented Dec 3, 2015

k8s-github-robot commented Dec 4, 2015

k8s-bot commented Dec 4, 2015

gmarek commented Dec 4, 2015

k8s-bot commented Dec 4, 2015

k8s-github-robot commented Dec 5, 2015

k8s-bot commented Dec 5, 2015

k8s-github-robot commented Dec 5, 2015

Scheduler metrics: binding rate limiter saturation rate #17673

Scheduler metrics: binding rate limiter saturation rate #17673

Conversation

hongchaodeng commented Nov 23, 2015

k8s-bot commented Nov 23, 2015

k8s-bot commented Nov 23, 2015

k8s-github-robot commented Nov 23, 2015

xiang90 Nov 23, 2015

Choose a reason for hiding this comment

k8s-github-robot commented Nov 23, 2015

k8s-github-robot commented Nov 23, 2015

hongchaodeng commented Nov 24, 2015

hongchaodeng commented Nov 24, 2015

davidopp commented Nov 24, 2015

davidopp commented Nov 24, 2015

wojtek-t Nov 24, 2015

Choose a reason for hiding this comment

wojtek-t commented Nov 30, 2015

hongchaodeng commented Nov 30, 2015

xiang90 commented Nov 30, 2015

wojtek-t commented Dec 2, 2015

wojtek-t commented Dec 3, 2015

k8s-bot commented Dec 3, 2015

gmarek commented Dec 3, 2015

hongchaodeng commented Dec 3, 2015

gmarek commented Dec 3, 2015

hongchaodeng commented Dec 3, 2015

hongchaodeng commented Dec 3, 2015

k8s-bot commented Dec 3, 2015

wojtek-t commented Dec 3, 2015

k8s-github-robot commented Dec 4, 2015

k8s-bot commented Dec 4, 2015

gmarek commented Dec 4, 2015

k8s-bot commented Dec 4, 2015

k8s-github-robot commented Dec 5, 2015

k8s-bot commented Dec 5, 2015

k8s-github-robot commented Dec 5, 2015