Henry Robinson has posted comments on this change. Change subject: IMPALA-5394: Set socket timeouts while opening TSaslTransport ......................................................................
Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7061/2/be/src/transport/TSaslServerTransport.cpp File be/src/transport/TSaslServerTransport.cpp: PS2, Line 158: socket->setRecvTimeout(0); : socket->setSendTimeout(0); > I initially planned to do that, but TSocket doesn't provide a way to get th Ah, that's what I was missing, thanks John. Still trying to make sure I understand the issue - maybe it would better to give up transportMap_mutex_ if the if(..) branch is taken? So you'd have something like: { lock_guard<mutex> l(transportMap_mutex_); // .... if (trans_map != transportMap_.end()) { // return transport and erase entry in map } } boost::shared_ptr<TTransport> wrapped( new TSaslServerTransport(serverDefinitionMap_, trans)); ret_transport.reset(new TBufferedTransport(wrapped, impala::ThriftServer::BufferedTransportFactory::DEFAULT_BUFFER_SIZE_BYTES)); ret_transport.get()->open(); { lock_guard<mutex> l(transportMap_mutex_); transportMap_[trans] = ret_transport; } The most obvious problem with that is that concurrent calls to getTransport() with the same 'trans' object would race. There are ways to address that but it looks like it's assumed the callers only use 'trans' consecutively, not concurrently. -- To view, visit http://gerrit.cloudera.org:8080/7061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John Sherman <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: John Sherman <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
