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 3:

(2 comments)

Answer some questions while I fix the review issues.

http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h
File be/src/exec/hash-table.h:

http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h@651
PS3, Line 651: StatsCountersAdd
> I think this kinda makes sense - we know the number of buckets earlier than
It is accumulated counters, it can not be added in the close method. Some close 
called from spill, for example, we did not add this counter at close for 
collisions number in the legacy code.


http://gerrit.cloudera.org:8080/#/c/14234/3/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/14234/3/tests/query_test/test_observability.py@676
PS3, Line 676: '(?<=Probes: )\d+(\.\d+)?'
> is the positive lookahead in this regex necessary? would the following rege
I need to get the number only. For example 12 or 12.3. Your search seems will 
get back whole string "Probes:  ...12.3" and will get nothing for integer case.



--
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: 3
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: Thu, 19 Sep 2019 13:06:32 +0000
Gerrit-HasComments: Yes

Reply via email to