impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4135: Thrift threaded server times-out connections during high load
Date Fri, 23 Sep 2016 18:32:57 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4135: Thrift threaded server times-out connections during high load
......................................................................


Patch Set 1:

(3 comments)

Thanks! Not a full review yet. I still need to spend time reading TAcceptQueueThreadedServer.cpp
carefully.

http://gerrit.cloudera.org:8080/#/c/4519/1//COMMIT_MSG
Commit Message:

PS1, Line 18: This patch has been tested locally with the repro shown in IMPALA-4135
Can you add the test, modified if necessary?


http://gerrit.cloudera.org:8080/#/c/4519/1/be/src/server/CMakeLists.txt
File be/src/server/CMakeLists.txt:

PS1, Line 24: add_library(ThriftAcceptQueueThreadedServer
            :     TAcceptQueueThreadedServer.cpp
            :   )
            : add_dependencies(ThriftAcceptQueueThreadedServer thrift-deps)
Was there a reason you wanted this as a separate library? I think it's fine to put this code
in rpc/ since that's where we have our thrift-server.


http://gerrit.cloudera.org:8080/#/c/4519/1/be/src/server/TAcceptQueueThreadedServer.h
File be/src/server/TAcceptQueueThreadedServer.h:

PS1, Line 1: // This file was copied from apache::thrift::server::TThreadedServer.cpp v0.9.0,
with the
           : // significant changes noted inline below.
           : /*
           :  * Licensed to the Apache Software Foundation (ASF) under one
           :  * or more contributor license agreements. See the NOTICE file
           :  * distributed with this work for additional information
           :  * regarding copyright ownership. The ASF licenses this file
           :  * to you under the Apache License, Version 2.0 (the
           :  * "License"); you may not use this file except in compliance
           :  * with the License. You may obtain a copy of the License at
           :  *
           :  *   http://www.apache.org/licenses/LICENSE-2.0
           :  *
           :  * Unless required by applicable law or agreed to in writing,
           :  * software distributed under the License is distributed on an
           :  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
           :  * KIND, either express or implied. See the License for the
           :  * specific language governing permissions and limitations
           :  * under the License.
           :  */
I think this is fine, but if you haven't already, can you ask Jim or Henry if there's anything
else we have to do for legal?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50e728974ef31a9d49132a0b3f7cde2a4f3356d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message