Jiawei Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14776 )

Change subject: IMPALA-7550: Add documentation to profile counters
......................................................................


Patch Set 7:

(4 comments)

Hi Tim, Balazs
Thanks for the review! Here is another round code changes.

http://gerrit.cloudera.org:8080/#/c/14776/5/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:

http://gerrit.cloudera.org:8080/#/c/14776/5/be/src/exec/hdfs-scan-node-base.cc@72
PS5, Line 72:
            : PROFILE_DEFINE_DERIVED_COUNTER(PerReadThreadRawHdfsThrou
> As mentioned for the previous counter, all 'total wall clock time' metrics
Done, also changed to the same pattern on TotalRawHBaseReadTime. Run "git grep" 
and did not find other places that have the same descriptions.


http://gerrit.cloudera.org:8080/#/c/14776/5/be/src/exec/hdfs-scan-node-base.cc@85
PS5, Line 85: average nu
> Yep, there's a single queue (with multiple I/O threads consuming work from
Okay, then the original text makes sense here I think.


http://gerrit.cloudera.org:8080/#/c/14776/5/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/14776/5/be/src/runtime/coordinator.cc@88
PS5, Line 88: ry in exchange no
> I was recommending '(across the network)' in parens specifically because it
Thanks for explanation. Done


http://gerrit.cloudera.org:8080/#/c/14776/5/www/profile_docs.tmpl
File www/profile_docs.tmpl:

http://gerrit.cloudera.org:8080/#/c/14776/5/www/profile_docs.tmpl@29
PS5, Line 29: <th>Name</th>
            :           <th>Significance</th>
            :           <th>Unit</th>
            :           <th>Description</th>
            :         </tr>
            :       </thead>
            :       <tbody>
            :         {{#profile_docs}}
            :         <tr>
            :           <td>
            :           {{name}}
            :           </td>
            :           <td>
            :           {{significance}}
            :           </td>
            :           <td>
            :           {{unit}}
            :           </td>
            :           <td>
            :           {{description}}
            :           </td>
            :         </tr>
> IIUC, right now, the table will only have the significance level (e.g. STAB
Done. Add the section.
Screenshot here:
https://drive.google.com/open?id=1hu1tRe2fFwrb_CIkce_g7sflI7FVZTLk



--
To view, visit http://gerrit.cloudera.org:8080/14776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc03faddb27754001290bb6d899840e2cbe7ccb7
Gerrit-Change-Number: 14776
Gerrit-PatchSet: 7
Gerrit-Owner: Jiawei Wang <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Balazs Jeszenszky <[email protected]>
Gerrit-Reviewer: David Rorke <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jiawei Wang <[email protected]>
Gerrit-Reviewer: Jiawei Wang <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Wed, 11 Dec 2019 23:18:39 +0000
Gerrit-HasComments: Yes

Reply via email to