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

HystrixDashboard - threadpool does not display right TPS #1697

Open
JagmohanSharma opened this issue Oct 16, 2017 · 2 comments
Open

HystrixDashboard - threadpool does not display right TPS #1697

JagmohanSharma opened this issue Oct 16, 2017 · 2 comments

Comments

@JagmohanSharma
Copy link

JagmohanSharma commented Oct 16, 2017

In order to fix issue with TPS in hystrix-dashboard, code changes were made previously in hystrixCommand.js #1251, but as these changes were not done in hystrixThreadPool.js which causing a different value for TPS in threadpool section of dashboard.

 // the following will break when it becomes a compound string if the property is dynamically changed
		convertAvg(data, "propertyValue_metricsRollingStatisticalWindowInMilliseconds", false);

Above line of code was removed from below method in previous changes in hystrixCommand.js:

/**
	 * Since the stream of data can be aggregated from multiple hosts in a tiered manner
	 * the aggregation just sums everything together and provides us the denominator (reportingHosts)
	 * so we must divide by it to get an average per instance value.
	 *
	 * We want to do this on any numerical values where we want per instance rather than cluster-wide sum.
	 */
	function convertAllAvg(data) {
		convertAvg(data, "errorPercentage", true);
		convertAvg(data, "latencyExecute_mean", false);
		convertAvg(data, "latencyTotal_mean", false);

		// the following will break when it becomes a compound string if the property is dynamically changed
		convertAvg(data, "propertyValue_metricsRollingStatisticalWindowInMilliseconds", false);
	}

But same calculation is also being done in hystrixThreadPool.js as below:

function converAllAvg(data) {
		convertAvg(data, "propertyValue_queueSizeRejectionThreshold", false);
		
		// the following will break when it becomes a compound string if the property is dynamically changed
		convertAvg(data, "propertyValue_metricsRollingStatisticalWindowInMilliseconds", false);
	}

I think previously removed line(avg calculation for propertyValue_metricsRollingStatisticalWindowInMilliseconds) is also not required in hystrixThreadPool.js which causing wrong value for TPS in threadPool section of dashboard. I have updated code with this change and started a pull request for review. Please let me know if changes are good to go to fix this issue.

JagmohanSharma pushed a commit to JagmohanSharma/Hystrix that referenced this issue Oct 16, 2017
Code changes to fix TPS calculation issue with hystrix thread pool in dashboard.
@tjuchniewicz
Copy link

Please see #1251
In my opinion this feature is broken. Also calculation for commands is invalid. This is related to Turbine 1.0 and 2.0 incompatibility: Netflix/Turbine#100

@tjuchniewicz
Copy link

Dashboard module moved to https://github.com/Netflix-Skunkworks/hystrix-dashboard

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

Successfully merging a pull request may close this issue.

2 participants