[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has removed Jim Apple from this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates .. Removed reviewer Jim Apple. -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 15 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jinchul Kim Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan
[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has removed Jinchul Kim from this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates .. Removed reviewer Jinchul Kim. -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 15 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan
[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has removed Mostafa Mokhtar from this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates .. Removed reviewer Mostafa Mokhtar. -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 15 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan
[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates .. Patch Set 12: (5 comments) http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py File tests/benchmark/perf_result_datastore.py: http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@23 PS12, Line 23: from subprocess import * > "*" is an antipattern for imports. It is better to be explicit about import Done Removed this import since it is not used in the file http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@63 PS12, Line 63: def insert_workload_summary(self, run_info_id): : self._insert_workload_summary(run_info_id) > This doesn't seem to do anything useful, so just rename _insert_workload_su Done. Good catch http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@85 PS12, Line 85: dont_save_profiles): > Negatives are weird. It would be better to call this save_profiles and flip Done http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@88 PS12, Line 88: temp_profile_folder = os.path.join(home_dir,"workspace","impala-workload-runner","profiles") > This is badly formated. There should be spaces between parameters. impala-f Done http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@155 PS12, Line 155: self._cache: Dictionary of with run_info/user as key/value > This isn't quite right. self._cache appears to be a hash in which a tuple o Done -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 12 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Jinchul Kim Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan Gerrit-Comment-Date: Fri, 20 Jul 2018 18:11:01 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates .. Patch Set 12: (5 comments) I think the changes are OK for the most part, but they could stand to use better Python conventions. One of my inline comments explains how to do that. http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py File tests/benchmark/perf_result_datastore.py: http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@23 PS12, Line 23: from subprocess import * "*" is an antipattern for imports. It is better to be explicit about imported symbols. http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@63 PS12, Line 63: def insert_workload_summary(self, run_info_id): : self._insert_workload_summary(run_info_id) This doesn't seem to do anything useful, so just rename _insert_workload_summary to insert_workload_summary and remove this. http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@85 PS12, Line 85: dont_save_profiles): Negatives are weird. It would be better to call this save_profiles and flip conditions. The reason being, readability for stuff like: if not dont_save_profiles gets confusing. http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@88 PS12, Line 88: temp_profile_folder = os.path.join(home_dir,"workspace","impala-workload-runner","profiles") This is badly formated. There should be spaces between parameters. impala-flake8 perf_result_datastore.py should help find problems like this. Please run it against this file. You don't have to fix all the existing flake8 errors. But for your own submissions, you should format them better. You can do that via "git diff HEAD^ | impala-flake8 --diff" from the root Impala directory. http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@155 PS12, Line 155: self._cache: Dictionary of with run_info/user as key/value This isn't quite right. self._cache appears to be a hash in which a tuple of (run_info, user) is the key. -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 12 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Jinchul Kim Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan Gerrit-Comment-Date: Wed, 18 Jul 2018 23:38:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates .. IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates Updates to the scripts in benchmark folder to - Store test execution results to ExecutionResults table - Store workload metrics to WorkloadMetrics - Option to mark a performance run as official - Option to skip saving of profiles Testing: Executed these script to create the database and populate the tables with the above mentioned data Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c --- M tests/benchmark/create_database.py M tests/benchmark/perf_result_datastore.py M tests/benchmark/report_benchmark_results.py 3 files changed, 303 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10100/12 -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 12 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Jinchul Kim Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan
[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/10100/5/tests/benchmark/report_benchmark_results.py File tests/benchmark/report_benchmark_results.py: http://gerrit.cloudera.org:8080/#/c/10100/5/tests/benchmark/report_benchmark_results.py@735 PS5, Line 735: if not first_exec_summary: > Add comment. Done http://gerrit.cloudera.org:8080/#/c/10100/5/tests/benchmark/report_benchmark_results.py@1058 PS5, Line 1058: if exec_summaries[0] is None: > Same as comment above, please add a comment line explaining what this does. Done -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 5 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan Gerrit-Comment-Date: Mon, 14 May 2018 23:59:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates .. Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/10100/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10100/8//COMMIT_MSG@7 PS8, Line 7: Update > Please add a space in front of this. Done http://gerrit.cloudera.org:8080/#/c/10100/5/tests/benchmark/perf_result_datastore.py File tests/benchmark/perf_result_datastore.py: http://gerrit.cloudera.org:8080/#/c/10100/5/tests/benchmark/perf_result_datastore.py@298 PS5, Line 298: LOG.debug('Inserting File Type: {0}, {1}, {2}'.format( > Fix indentation Done http://gerrit.cloudera.org:8080/#/c/10100/8/tests/benchmark/report_benchmark_results.py File tests/benchmark/report_benchmark_results.py: http://gerrit.cloudera.org:8080/#/c/10100/8/tests/benchmark/report_benchmark_results.py@141 PS8, Line 141: Dont > I think either "Don't" or "Do not" is better than Dont. This is used for us Done http://gerrit.cloudera.org:8080/#/c/10100/8/tests/benchmark/report_benchmark_results.py@1058 PS8, Line 1058: if exec_summaries[0] is None: : return "" > I think this is a redundant special handling. I believe L1054-1055 can be Done -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 8 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan Gerrit-Comment-Date: Mon, 14 May 2018 23:52:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6923:Update scripts in benchmark folder to store workload and few minor updates
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923:Update scripts in benchmark folder to store workload and few minor updates .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/10100/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10100/8//COMMIT_MSG@7 PS8, Line 7: Update Please add a space in front of this. http://gerrit.cloudera.org:8080/#/c/10100/8/tests/benchmark/report_benchmark_results.py File tests/benchmark/report_benchmark_results.py: http://gerrit.cloudera.org:8080/#/c/10100/8/tests/benchmark/report_benchmark_results.py@141 PS8, Line 141: Dont I think either "Don't" or "Do not" is better than Dont. This is used for user's helper message. http://gerrit.cloudera.org:8080/#/c/10100/8/tests/benchmark/report_benchmark_results.py@1058 PS8, Line 1058: if exec_summaries[0] is None: : return "" I think this is a redundant special handling. I believe L1054-1055 can be merged into the L1058-1059 because there is no symbol dependent issue. e.g.) if options.hive_results or exec_summaries[0] is None: return "" -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 8 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan Gerrit-Comment-Date: Thu, 10 May 2018 00:12:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6923:Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923:Update scripts in benchmark folder to store workload and few minor updates .. IMPALA-6923:Update scripts in benchmark folder to store workload and few minor updates Updates to the scripts in benchmark folder to - Store test execution results to ExecutionResults table - Store workload metrics to WorkloadMetrics - Option to mark a performance run as official - Option to skip saving of profiles Testing: Executed these script to create the database and populate the tables with the above mentioned data Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c --- M tests/benchmark/create_database.py M tests/benchmark/perf_result_datastore.py M tests/benchmark/report_benchmark_results.py 3 files changed, 252 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10100/8 -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 8 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan
[Impala-ASF-CR] IMPALA-6923:Update scripts in benchmark folder to store workload and few minor updates
Nithya Janarthanan has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923:Update scripts in benchmark folder to store workload and few minor updates .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10100/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10100/5//COMMIT_MSG@7 PS5, Line 7: CDH-65183 Update scripts in benchmark folder to store workload and few minor updates > Please use IMPALA tickets. People who do not work at Cloudera cannot read C Done -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 5 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Nithya Janarthanan Gerrit-Comment-Date: Tue, 24 Apr 2018 22:36:08 + Gerrit-HasComments: Yes