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