Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13818 )
Change subject: IMPALA-8748: Always pass hostname to RpcMgr::GetProxy() ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/13818/1/be/src/rpc/rpc-mgr.h File be/src/rpc/rpc-mgr.h: http://gerrit.cloudera.org:8080/#/c/13818/1/be/src/rpc/rpc-mgr.h@148 PS1, Line 148: has to match > if Kerberos is enabled Done http://gerrit.cloudera.org:8080/#/c/13818/1/be/src/rpc/rpc-mgr.h@149 PS1, Line 149: f Kerberos is enabled. 'P' must descend fr > The "usually" here seems to suggest that the DCHECK could fail in some case Removed in PS2. Please see replies in rpc-mgr.inline.h http://gerrit.cloudera.org:8080/#/c/13818/1/be/src/rpc/rpc-mgr.inline.h File be/src/rpc/rpc-mgr.inline.h: http://gerrit.cloudera.org:8080/#/c/13818/1/be/src/rpc/rpc-mgr.inline.h@44 PS1, Line 44: // Talk to self via loopback. > I don't think this DCHECK is valid -- it's OK to run things in an environme Yes, was having a hard time coming up with a DCHECK which works for all cases. I suspect Impala may fail elsewhere if DNS is disabled but there is nothing from stopping a user from configuring the hostname as 1.2.3.4 for whatever reason. Anyhow, removing it for now. FWIW, I confirmed that this DCHECK actually catches the bug which this patch is trying to fix. -- To view, visit http://gerrit.cloudera.org:8080/13818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I85b661c8c3b3b67bfc1ce9e29aecb90a862666c0 Gerrit-Change-Number: 13818 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 09 Jul 2019 05:03:55 +0000 Gerrit-HasComments: Yes
