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

Jump to bottom

notify-rate-limiter assumes apps, no pods #4863

Closed
jdef opened this issue Dec 16, 2016 · 2 comments · Fixed by dcos/dcos#1086
Closed

notify-rate-limiter assumes apps, no pods #4863

jdef opened this issue Dec 16, 2016 · 2 comments · Fixed by dcos/dcos#1086
Assignees
@jasongilanfarr

Comments

@jdef
Copy link
Contributor

jdef commented Dec 16, 2016 •
edited

rate limiting should work for BOTH apps and pods

xref

marathon/src/main/scala/mesosphere/marathon/core/task/update/impl/steps/NotifyRateLimiterStepImpl.scala

Line 36 in 0f13200

appRepository.getVersion(runSpecId, version).map { maybeApp =>

@jdef jdef added this to the Marathon 1.4 milestone Dec 16, 2016
@jasongilanfarr
Copy link
Contributor

I have a funny feeling we don't do this for scale checks, health checks, etc... definitely a blocker

@jasongilanfarr jasongilanfarr self-assigned this Dec 16, 2016
@jasongilanfarr
Copy link
Contributor

https://phabricator.mesosphere.com/D352

jasongilanfarr pushed a commit that referenced this issue Dec 20, 2016
Inform the rate limiter for pods and apps (#4863)
c8924aa 
Summary: The rate limiter incorrectly assumed that all runSpecs are apps.

Test Plan: sbt test with new test

Reviewers: meichstedt, jdef, timcharper, aquamatthias

Reviewed By: jdef, timcharper, aquamatthias

Subscribers: aquamatthias, jenkins, marathon-team

Differential Revision: https://phabricator.mesosphere.com/D352
aquamatthias pushed a commit that referenced this issue Dec 21, 2016
@aquamatthias
Inform the rate limiter for pods and apps (#4863)
5069189 
Summary: The rate limiter incorrectly assumed that all runSpecs are apps.

Test Plan: sbt test with new test

Reviewers: meichstedt, jdef, timcharper, aquamatthias

Reviewed By: jdef, timcharper, aquamatthias

Subscribers: aquamatthias, jenkins, marathon-team

Differential Revision: https://phabricator.mesosphere.com/D352

(cherry picked from commit c8924aa)
This was referenced Dec 21, 2016
Closed
Merged
@unterstein unterstein mentioned this issue Mar 3, 2017
Closed
@mesosphere mesosphere locked and limited conversation to collaborators Mar 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jasongilanfarr jasongilanfarr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use Marathon 1.4.0-RC4. aquamatthias/dcos
3 participants
@aquamatthias @jasongilanfarr @jdef