Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9384/5/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/9384/5/be/src/rpc/rpc-mgr.cc@208
PS5, Line 208:   Value per_conn_metrics(kArrayType);
nit: maybe add a comment similar to the one in L225?


http://gerrit.cloudera.org:8080/#/c/9384/5/be/src/rpc/rpc-mgr.cc@233
PS5, Line 233:   document->AddMember("per_conn_metrics", per_conn_metrics, 
document->GetAllocator());
Move to L222 right after its construction?


http://gerrit.cloudera.org:8080/#/c/9384/5/www/rpcz.tmpl
File www/rpcz.tmpl:

http://gerrit.cloudera.org:8080/#/c/9384/5/www/rpcz.tmpl@96
PS5, Line 96:   </thead>
The other .tmpl files have an empty tbody here, maybe it could increase 
readability to add a comment like <!-- Content will be added using DataTables 
below -->?


http://gerrit.cloudera.org:8080/#/c/9384/5/www/rpcz.tmpl@237
PS5, Line 237:   for (var i = 0; i < json["per_conn_metrics"].length; ++i) {
Can you rewrite this to use two nested calls to map()?

var rows = $.map(json["per_conn_metrics"], function(row) {
  $.map(row, function(cell) { return cell; });
});


Then you could call table.clear().rows.add(rows).draw() atomically.

You probably also inline rows, but that might make it unreadable.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 00:00:33 +0000
Gerrit-HasComments: Yes

Reply via email to