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

Percentiles rounding error #331

Closed
pior opened this issue Aug 27, 2015 · 5 comments
Closed

Percentiles rounding error #331

pior opened this issue Aug 27, 2015 · 5 comments
Labels

Comments

@pior
Copy link

pior commented Aug 27, 2015

 Name                                                           # reqs    50%    66%    75%    80%    90%    95%    98%    99%   100%
--------------------------------------------------------------------------------------------------------------------------------------------
 POST /projects/<>/devices                                          50    230    240    250    270    320    340    600    600    596

Note that all latencies are multiple of 10. And 99% is higher than 100%.

@jalan
Copy link

jalan commented Sep 11, 2015

I noticed this as well. Seems to be deliberate: locust/stats.py#L160

The numbers are accurate enough for my purposes, so no complaint here

@cgoldberg
Copy link
Member

@jalan 99% > 100% is confusing. Perhaps the 100th percentile should be rounded also?

@jalan
Copy link

jalan commented Sep 13, 2015

@cgoldberg: Agreed. One option would be to round the min and max times just like all the other times:

diff --git a/locust/stats.py b/locust/stats.py
index 267dee4..79d71a5 100644
--- a/locust/stats.py
+++ b/locust/stats.py
@@ -151,12 +151,6 @@ class StatsEntry(object):
     def _log_response_time(self, response_time):
         self.total_response_time += response_time

-        if self.min_response_time is None:
-            self.min_response_time = response_time
-
-        self.min_response_time = min(self.min_response_time, response_time)
-        self.max_response_time = max(self.max_response_time, response_time)
-
         # to avoid to much data that has to be transfered to the master node when
         # running in distributed mode, we save the response time rounded in a dict
         # so that 147 becomes 150, 3432 becomes 3400 and 58760 becomes 59000
@@ -173,6 +167,12 @@ class StatsEntry(object):
         self.response_times.setdefault(rounded_response_time, 0)
         self.response_times[rounded_response_time] += 1

+        # update min and max response times
+        if self.min_response_time is None:
+            self.min_response_time = rounded_response_time
+        self.min_response_time = min(self.min_response_time, rounded_response_time)
+        self.max_response_time = max(self.max_response_time, rounded_response_time)
+
     def log_error(self, error):
         self.num_failures += 1
         self.stats.num_failures += 1

@justiniso justiniso added the bug label Apr 2, 2016
heyman added a commit that referenced this issue Oct 1, 2017
#331:  Use rounded_response_time for min/max/total response times
@Jonnymcc
Copy link
Contributor

Jonnymcc commented Dec 3, 2018

@pior Looks like a fix was merged for this. Can this bug be closed?

@pior
Copy link
Author

pior commented Dec 3, 2018

I can't really test that currently.
But from the discussion and the fix, it looks to me like the issue I reported is properly addressed.
👌
/cc @Jonnymcc

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

No branches or pull requests

6 participants