impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <>
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:


Thanks! Not a full review yet. I still need to spend time reading TAcceptQueueThreadedServer.cpp
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?
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.
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
           :  *
           :  *
           :  *
           :  * Unless required by applicable law or agreed to in writing,
           :  * software distributed under the License is distributed on an
           :  * 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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50e728974ef31a9d49132a0b3f7cde2a4f3356d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-HasComments: Yes

View raw message