Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-1928 : Impala ODBC bad performance with Kerberos 
mechanism
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/2769/2/be/src/rpc/thrift-client.h
File be/src/rpc/thrift-client.h:

Line 144:   transport_ = socket_;
> What is the expected behavior if socket_ is NULL?
At least public operations on this client should fail in a graceful manner :)

Really we should have some initialization method separate from the constructor 
that would be capable of reporting failures to connect, but that's probably 
beyond the scope of your change. For now, I think it'd be best to not set up 
the wrapped transport further because it looks to me like the wrapping 
transports may dereference their underlying transports w/o checking for NULL. I 
think that all of this works because the call to Open() checks if creating the 
socket_ succeeded or it returns with a failure. We should test it though.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iebcf6457091aef1fc0e5bd1549b3fcbafc5560d9
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Mostafa Mokhtar <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Mostafa Mokhtar <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-HasComments: Yes

Reply via email to