John Sherman has posted comments on this change.

Change subject: IMPALA-5394: Handle blocked HS2 connections
......................................................................


Patch Set 3:

I'd like some advice/feedback on the approach for enforcing fe_service_threads. 
In my local diff, I've added a maxTasks, numTasks and a taskLimitMonitor to the 
TAcceptQueueServer class.
>From there it seems like I have two choices:
1)
a) inside the Task constructor, synchronize with the taskLimitMonitor, check 
the passed in server's numTasks against maxTasks and wait if we've reached the 
limit or increment the server's numTasks.
b) inside the Task destructor synchronize with the taskLimitMonitor, decrement 
the server's numTasks and notifyAll if I'm transistioning from maxTasks to < 
maxTasks.
-- I like this because the code is somewhat symmetrical, but I do not know if 
it is a good idea to do these blocking operations inside a 
constructor/destructor.

2)
a) inside the SetupConnection function before I create a new task, synchronize 
with the tasklimitMonitor and check/block/increment numTasks.
b) inside the Tasks run method, decrement/notify where the Task removes itself 
from the TAcceptQueueServer's tasks set.

My local diff uses approach 1 currently but I'm a bit unsure about the best 
practices around that approach.

-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Sherman <j...@arcadiadata.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: John Sherman <j...@arcadiadata.com>
Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-HasComments: No

Reply via email to