Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14388 )
Change subject: IMPALA-9026: Use resolved IP address for statestore subscriber ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/14388/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14388/2//COMMIT_MSG@19 PS2, Line 19: > nit: line too long Done http://gerrit.cloudera.org:8080/#/c/14388/2/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/14388/2/be/src/runtime/exec-env.cc@222 PS2, Line 222: tmp_file_mgr_(new TmpFileMgr), : frontend_(new Frontend()), : async_rpc_pool_(new CallableThreadPool("rpc-pool", "async-rpc-sender", 8, 10000)), : query_exec_mgr_(new QueryExecMgr()), : rpc_metrics_(metrics_->GetOrCreateChildG > whats the reason for moving these from Init() to the constructor? Failing in Init() below or here are both fatal (i.e. Impala will not start) so I may as well remove the complication of splitting the initialization of krpc_address_ into two functions. In a previous version of the patch I needed the IP address in this function to initialize the statestore subscriber but has since modified the code but I feel the simplification above is still warranted. http://gerrit.cloudera.org:8080/#/c/14388/2/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/14388/2/be/src/statestore/statestore-subscriber.cc@262 PS2, Line 262: hostname > document that the hostname can now either be the actual hostname, or the ip Comments updated in the header filer. Seems simpler to just resolve the hostname here instead of having the server choose between multiple addresses. -- To view, visit http://gerrit.cloudera.org:8080/14388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb8302dec0e52beb9f0b88306a51c38ff42a63a2 Gerrit-Change-Number: 14388 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Comment-Date: Wed, 23 Oct 2019 02:16:40 +0000 Gerrit-HasComments: Yes
