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

Show mean throughput in command line report #1146

Merged

Conversation

danielmitterdorfer
Copy link
Member

With this commit Rally shows also the mean throughput in the command
line report. The mean is useful e.g. for calculating summary statistics
across several races or for conducting statistical significance tests.

With this commit Rally shows also the mean throughput in the command
line report. The mean is useful e.g. for calculating summary statistics
across several races or for conducting statistical significance tests.
@danielmitterdorfer danielmitterdorfer added enhancement Improves the status quo :Reporting Command line reporting labels Jan 12, 2021
@danielmitterdorfer danielmitterdorfer added this to the 2.0.4 milestone Jan 12, 2021
@danielmitterdorfer danielmitterdorfer self-assigned this Jan 12, 2021
Copy link
Contributor

@DJRickyB DJRickyB left a comment

Choose a reason for hiding this comment

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

since means will almost always be greater than median, I'd suggest either putting it above min (as its own "class") or between median and max. in fact I think I'd prefer the former (put it into its own)
Edit: actually I guess I don't know for sure which way the mean will go

@danielmitterdorfer
Copy link
Member Author

since means will almost always be greater than median, I'd suggest either putting it above min (as its own "class") or between median and max. in fact I think I'd prefer the former (put it into its own)
Edit: actually I guess I don't know for sure which way the mean will go

Yes, the mean can either be smaller or larger than the median, depending on how the distribution is skewed. I am fine either way but I propose that we leave it as is, indicating at least that mean and median are in between min and max.

Also one integration test has failed. In the Elasticsearch log we see:

[2021-01-12T10:22:39,196][ERROR][o.e.b.Bootstrap          ] [rally-node-0] Exception
org.elasticsearch.http.BindHttpException: Failed to bind to [39200]
	at org.elasticsearch.http.netty4.Netty4HttpServerTransport.bindAddress(Netty4HttpServerTransport.java:487) ~[?:?]
	at org.elasticsearch.http.netty4.Netty4HttpServerTransport.createBoundHttpAddress(Netty4HttpServerTransport.java:384) ~[?:?]
	at org.elasticsearch.http.netty4.Netty4HttpServerTransport.doStart(Netty4HttpServerTransport.java:361) ~[?:?]
	at org.elasticsearch.common.component.AbstractLifecycleComponent.start(AbstractLifecycleComponent.java:72) ~[elasticsearch-6.8.0.jar:6.8.0]
	at org.elasticsearch.node.Node.start(Node.java:809) ~[elasticsearch-6.8.0.jar:6.8.0]
	at org.elasticsearch.bootstrap.Bootstrap.start(Bootstrap.java:269) ~[elasticsearch-6.8.0.jar:6.8.0]
	at org.elasticsearch.bootstrap.Bootstrap.init(Bootstrap.java:342) [elasticsearch-6.8.0.jar:6.8.0]
	at org.elasticsearch.bootstrap.Elasticsearch.init(Elasticsearch.java:159) [elasticsearch-6.8.0.jar:6.8.0]
	at org.elasticsearch.bootstrap.Elasticsearch.execute(Elasticsearch.java:150) [elasticsearch-6.8.0.jar:6.8.0]
	at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:86) [elasticsearch-6.8.0.jar:6.8.0]
	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:124) [elasticsearch-cli-6.8.0.jar:6.8.0]
	at org.elasticsearch.cli.Command.main(Command.java:90) [elasticsearch-cli-6.8.0.jar:6.8.0]
	at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:116) [elasticsearch-6.8.0.jar:6.8.0]
	at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:93) [elasticsearch-6.8.0.jar:6.8.0]
Caused by: java.net.BindException: Address already in use

i.e. this is unrelated to this PR and considered transient. @elasticmachine test this please

Copy link
Contributor

@DJRickyB DJRickyB left a comment

Choose a reason for hiding this comment

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

LGTM

@danielmitterdorfer danielmitterdorfer merged commit 7461c05 into elastic:master Jan 19, 2021
@danielmitterdorfer danielmitterdorfer deleted the report-mean-throughput branch January 19, 2021 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo :Reporting Command line reporting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants