Jim Apple has posted comments on this change. Change subject: Add a script to test performance on a developer machine ......................................................................
Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/6818/4/bin/single_node_perf_run.py File bin/single_node_perf_run.py: Line 71: """Loads a database with a particular scale factor.""" > Given that you os.chdir(IMPALA_HOME) in main(), can it be removed elsewhere Done PS4, Line 110: > get_git_hash_for_name("HEAD") ? Done Line 241: parser.add_option("--workloads", default="targeted-perf", > Nit: I usually like to refactor the boilerplate option-parsing stuff to ano Done PS4, Line 254: help="load databases for the chosen workloads") : parser.add_option("--start_minicluster", action="store_true", : help="start a new Hadoop minicluster") : parser.add_option("--ninja", action="store_true", : help = "use ninja, rather than Make, as > It's confusing to me to have --no options that default to True that are sto I couldn't possible refuse to eliminate the "no"s! :-D PS4, Line 262: : When one hash is given, measures the performance on the specified workloads. : When two hashes are given, compares their performance. Output is in : $IMPALA_HOME/perf_results/latest. : : WARNING: This script will run git checkout. You should not touch the tree : while the script is running. You should start the script from a clean git : tree. : > Nit: Consider a triple-quoted string and using textwrap.dedent(). Done Line 274: options, args = parser.parse_args() > Seems confusing that 1 git hash is allowed when the docstrings and usage al Done PS4, Line 296: curren > 2 spaces. Done -- To view, visit http://gerrit.cloudera.org:8080/6818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70ba7f3c28f612a370915615600bf8dcebcedbc9 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
