Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/19231 )
Change subject: KUDU-3357 endpoints for proxied RPCs ...................................................................... Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/19231/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19231/4//COMMIT_MSG@23 PS4, Line 23: <dns_name|ip_addr>:<port> nit: put quotes around this, the trailing colon makes it look like a part is missing http://gerrit.cloudera.org:8080/#/c/19231/4//COMMIT_MSG@23 PS4, Line 23: dns_name nit: hostname http://gerrit.cloudera.org:8080/#/c/19231/4//COMMIT_MSG@27 PS4, Line 27: outer Maybe we should use "external" and "internal" when specifying the network types for consistency? http://gerrit.cloudera.org:8080/#/c/19231/4//COMMIT_MSG@58 PS4, Line 58: kudu perf scan kudu table scan or kudu perf table_scan? http://gerrit.cloudera.org:8080/#/c/19231/4//COMMIT_MSG@61 PS4, Line 61: NOTE: even if "kudu cluster ksck" tool also worked, it's not yet a goal This is a bit confusing for me, does kudu cluster ksck work? If not, is there a reason why? http://gerrit.cloudera.org:8080/#/c/19231/4/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: http://gerrit.cloudera.org:8080/#/c/19231/4/src/kudu/master/ts_descriptor.cc@220 PS4, Line 220: DCHECK_GT(reg->proxy_rpc_addresses_size(), 0); what's the purpose of this dcheck here? http://gerrit.cloudera.org:8080/#/c/19231/4/src/kudu/tserver/heartbeater.cc File src/kudu/tserver/heartbeater.cc: http://gerrit.cloudera.org:8080/#/c/19231/4/src/kudu/tserver/heartbeater.cc@364 PS4, Line 364: // The RPC proxy-advertised addresses and the proxied RPC addresses should Shouldn't we compare the sizes? Or is it valid for the sizes be different? -- To view, visit http://gerrit.cloudera.org:8080/19231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic300250556d3f6e522a71923bed6aa5cd45375ea Gerrit-Change-Number: 19231 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Thu, 17 Nov 2022 18:52:54 +0000 Gerrit-HasComments: Yes
