Benchmarks dev guide #13
Conversation
MiguelWeezardo
force-pushed
the
michal/benchmarks_dev_guide
branch
from
62d2520 to
6fcd742
Compare
| @@ -0,0 +1 @@ | |||
| 1.8 | |||
There was a problem hiding this comment.
What uses this file? Shouldn't it be enough to set the required JDK in the POM?
There was a problem hiding this comment.
POM only impacts Maven, but in the README we tell developers to run java, so we can leverage jenv if they have it.
There was a problem hiding this comment.
The README should say explicitly which JDK version to use, without assuming how users manage their JDK installations.
| url: http://localhost:8080 | ||
| username: na | ||
| password: na |
There was a problem hiding this comment.
The password should not be required by default. Maybe change the example user to benchto so it's a bit more informative than na.
| --activeBenchmarks=presto/tpch --profile=presto-devenv \ | ||
| --overrides overrides.yaml | ||
| # set JAVA_HOME to the JDK 8 home dir | ||
| local_repo=$(./mvnw -B help:evaluate -Dexpression=settings.localRepository -q -DforceStdout) |
There was a problem hiding this comment.
Can we assume it's always $HOME/.m2/repository? That'll make the example simpler.
| local_repo=$(./mvnw -B help:evaluate -Dexpression=settings.localRepository -q -DforceStdout) | ||
| java -Xmx1g \ | ||
| -jar "$local_repo/io/trino/benchto/benchto-driver/0.18/benchto-driver-0.18.jar" \ | ||
| --profiles.directory "." \ |
There was a problem hiding this comment.
This should not be necessary, the application.yaml needs to be in the current directory anyway. Maybe also remove other flags, like -Xmx1g and the time limit and paths to sql and benchmark dirs.
|
|
||
| ### Automated benchmarks | ||
|
|
||
| A complete script which runs a small suite of benchmarks can be found in `perftest/benchto.sh`. |
There was a problem hiding this comment.
Isn't the script developed on the benchmarks branch where this PR is aimed at?
There was a problem hiding this comment.
Yes, but it should be separated from that effort so we can get this in faster.
| @@ -48,8 +52,10 @@ data-sources: | |||
| environment: | |||
| name: TRINO-DEV | |||
|
|
|||
| presto: | |||
| trino: | |||
There was a problem hiding this comment.
You have to also either change the data source name to presto or update all benchmarks.
There was a problem hiding this comment.
Guess I'll have to return to the old name. We can rename in a separate commit/PR
MiguelWeezardo
force-pushed
the
michal/benchmarks_dev_guide
branch
from
6fcd742 to
8eab4aa
Compare
nineinchnick
force-pushed
the
benchmarks
branch
from
3302e2e to
57f2bb2
Compare
MiguelWeezardo
force-pushed
the
michal/benchmarks_dev_guide
branch
from
8eab4aa to
da97cd2
Compare
MiguelWeezardo
force-pushed
the
michal/benchmarks_dev_guide
branch
from
da97cd2 to
de5c3c8
Compare
|
Making a separate PR not based on |
|
Continued on trinodb#13988 |
Description
Related issues, pull requests, and links
Documentation
( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
( ) No release notes entries required.
( ) Release notes entries required with the following suggested text: