Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/14234 )
Change subject: IMPALA-7637: Add more hash table stats to profile ...................................................................... Patch Set 2: (3 comments) New patch refactoring code to address the review issues, add support for group aggregation profiling, add more check for tests and re-ran exhaustive tests http://gerrit.cloudera.org:8080/#/c/14234/1/be/src/exec/hash-table.h File be/src/exec/hash-table.h: http://gerrit.cloudera.org:8080/#/c/14234/1/be/src/exec/hash-table.h@741 PS1, Line 741: /// Returns an iterator at the beginning of the hash table. Advancing this iterator > nit: we usually use lower_case for trivial accessor functions (this is allo Agree, number of buckets follows the pattern. I removed these new methods for they are no longer needed. http://gerrit.cloudera.org:8080/#/c/14234/1/be/src/exec/partitioned-hash-join-builder.cc File be/src/exec/partitioned-hash-join-builder.cc: http://gerrit.cloudera.org:8080/#/c/14234/1/be/src/exec/partitioned-hash-join-builder.cc@595 PS1, Line 595: } > Found the place in grouping-aggregator, I will put the similar code in a co Done http://gerrit.cloudera.org:8080/#/c/14234/1/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/14234/1/tests/query_test/test_observability.py@675 PS1, Line 675: assert "Resizes:" in runtime_profile : nprobes = re.search('(?<=Probes: )\d+(\.\d+)?', runtime_profile) : # Probes and travel can be 0. The number can be a integer or float with K : assert float(nprobes.group(0)) >= 0 : ntravel = re.search('(?<=Travel: )\d > That's a good point. I think it would be easy with the way the code is stru Add assert on values, I just found , at least in aggregation, every counter value can be 0. Also, add the test for group by queries. -- To view, visit http://gerrit.cloudera.org:8080/14234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 Gerrit-Change-Number: 14234 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com> Gerrit-Comment-Date: Wed, 18 Sep 2019 13:05:54 +0000 Gerrit-HasComments: Yes
