impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jim Apple (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Add a script to test performance on a developer machine
Date Sat, 20 May 2017 06:14:31 GMT
Jim Apple has posted comments on this change.

Change subject: Add a script to test performance on a developer machine
......................................................................


Patch Set 3:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/6818/2//COMMIT_MSG
Commit Message:

PS2, Line 12:     bin/single_n
> Remove jenkins.
Done


http://gerrit.cloudera.org:8080/#/c/6818/2/bin/single_node_perf_run.py
File bin/single_node_perf_run.py:

Line 22: # Compares the performance of Impala at two git hashes on
> It might be good to include a large, prominent note saying that this will c
Done


Line 23: # some standard benchmarks. Output is in
> It might also be good to call out load_data.py will get called and could be
Done


PS2, Line 59: import os
> Please make IMPALA_HOME all-caps, as that's a convention for global variabl
Done


PS2, Line 125:     run_workload += ["--query_names={0}".format(options.que
> Do you not want to write this file into the perf_results directory?
Done


PS2, Line 183:         temp_dir, R=True, _out=sys.stdout, _err=sys.stderr)
             :   print "Backed up workloads to {0}".format(temp_
> What happens if one of these sh.cp() commands fail? Will the script complet
An exception is thrown, which causes the script to halt.


PS2, Line 264:                    "some standard benchmarks. Outp
> Maybe a clearer message? "Invalid arguments: either 1 or 2 GIt hashes allow
Done


PS2, Line 268:                    "from a clean git tree.\n\n"
> Maybe: "Working copy is dirty. Consider 'git stash' and try again."
Done


PS2, Line 271: 
> You probably want to chdir(impala_home) first?
Done


http://gerrit.cloudera.org:8080/#/c/6818/2/testdata/datasets/tpcds/preload
File testdata/datasets/tpcds/preload:

PS2, Line 52: if [ -t 1 ]
> Cool. Did you test both?
Yes


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message