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

Hystrix metrics as streams #1047

Merged
merged 45 commits into from
Jan 14, 2016
Merged

Conversation

mattrjacobs
Copy link
Contributor

This (large) set of commits is intended to solve #854 and #943.

The premise of these changes is that the Hystrix library is not the optimal place to decide how metrics should be consumed. By representing metrics as event streams, consumers gain flexibility in choosing their consumption strategy.

In 1.4.x, all events get aggregated in command-level data structures (HystrixRollingNumber and HystrixRollingPercentile). This is lossy in the sense that only aggregate queries are possible against these data sources.

At the moment, I've achieved feature and performance parity (AFAIK) with 1.4.x while ripping the usage of those data structures out. The metrics consumers that existed in 1.4.x are now wired against the new metrics streams. I haven't written new metrics streams that make better use of the new functionality, but that will be coming shortly.

I'm going to start a Wiki page with more details on the 1.5 metrics architecture. Will link that here when it's ready.

I've run this code in the Netflix production environment and haven't seen any show-stopping bugs, so this is very likely to become 1.5.0-RC1. When it does, I'd love feedback on design/usage. Any sort of code review would be much appreciated, as well.

Matt Jacobs added 30 commits January 12, 2016 14:52
… to RollingNumber/RollingPercentile as well)
* Added HystrixServoMetricsPublisherCommandTest as a concrete unit test that behavior is still correct
* They get calculated only on health count intervals
…el, to thread-level.

* Added global-level stream as a way to recover command-level streams
@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #305 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #306 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #307 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

NetflixOSS » Hystrix » Hystrix-pull-requests #308 SUCCESS
This pull request looks good

@mattrjacobs
Copy link
Contributor Author

JMH comparison between 1.4.22 and this PR are here: https://docs.google.com/spreadsheets/d/1a0ERBQJZzlmVqMpuvvdSwbJuXwt0UmkPsFZ97pvgA8o/edit

Highlights are:

  • command throughput for both HystrixCommand and HystrixObservableCommand increased by 10-20%
  • In a case where 7 threads are busy executing commands and 1 thread is busy reading metrics as fast as possible (the writeHeavy scenario), command throughput is up by 3-5% and metrics read throughput is up by 200%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants