impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Sherman (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5394: Handle blocked HS2 connections
Date Thu, 27 Jul 2017 16:30:43 GMT
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 <jfs@arcadiadata.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.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: No

Mime
View raw message