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

[CLIPPER-227] Fix EWMA behavior for meters #228

Merged
merged 4 commits into from
Jun 24, 2017
Merged

[CLIPPER-227] Fix EWMA behavior for meters #228

merged 4 commits into from
Jun 24, 2017

Conversation

Corey-Zumar
Copy link
Contributor

@Corey-Zumar Corey-Zumar commented Jun 24, 2017 •
edited by dcrankshaw
Loading

Fixes #227

@Corey-Zumar Corey-Zumar requested a review from dcrankshaw June 24, 2017 00:27
@Corey-Zumar Corey-Zumar mentioned this pull request Jun 24, 2017
Closed
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/476/
Test PASSed.

dcrankshaw
dcrankshaw approved these changes Jun 24, 2017
View reviewed changes
Copy link
Contributor

@dcrankshaw dcrankshaw left a comment

Choose a reason for hiding this comment

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

This looks fine. What was the problem?

@Corey-Zumar
Copy link
Contributor Author

The issue was that marks would compound until they were all included in the EWMA at once upon invocation of get_n_minute_rate_seconds. This resulted in much higher, inaccurate EWMA rates. This was fixed by ensuring that we tick every time a mark occurs.

@dcrankshaw dcrankshaw merged commit 02f42b4 into ucbrise:develop Jun 24, 2017
dcrankshaw pushed a commit that referenced this pull request Jun 27, 2017
@Corey-Zumar @dcrankshaw
[CLIPPER-227] Fix EWMA behavior for meters (#228)
a3ee768 
* ewma fixes

* Add missing rate ticks, use chrono to ensure temporal correctness

* Format code

(cherry picked from commit 02f42b4)
feynmanliang added a commit to feynmanliang/clipper that referenced this pull request Jul 3, 2017
@feynmanliang
Merge branch 'develop' into k8s
6590457 
* develop:
  Wording fix (ucbrise#234)
  RPC container content fix (ucbrise#232)
  [CLIPPER-227] Fix EWMA behavior for meters (ucbrise#228)
  One-line app registration and model deployment (ucbrise#223)
  R model deployment (ucbrise#195)
  Allow model versions to be strings (ucbrise#197)
  Base64 decoding for JSON byte data (ucbrise#214)
  Restarting on containers is no longer the default behavior (ucbrise#213)
  fixed backslash escape issue for removing remote containers (ucbrise#210)
  removed pip install findspark from run_unittests.sh (ucbrise#211)
  Fix example code in README (ucbrise#205)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcrankshaw dcrankshaw dcrankshaw approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@Corey-Zumar @AmplabJenkins @dcrankshaw