impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5394: Set socket timeouts while opening TSaslTransport
Date Mon, 05 Jun 2017 23:35:20 GMT
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 <jfs@arcadiadata.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: John Sherman <jfs@arcadiadata.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message