Return-Path: X-Original-To: apmail-drill-dev-archive@www.apache.org Delivered-To: apmail-drill-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 5485110AE4 for ; Tue, 5 May 2015 23:15:56 +0000 (UTC) Received: (qmail 23434 invoked by uid 500); 5 May 2015 23:15:56 -0000 Delivered-To: apmail-drill-dev-archive@drill.apache.org Received: (qmail 23389 invoked by uid 500); 5 May 2015 23:15:56 -0000 Mailing-List: contact dev-help@drill.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@drill.apache.org Delivered-To: mailing list dev@drill.apache.org Received: (qmail 23373 invoked by uid 500); 5 May 2015 23:15:55 -0000 Delivered-To: apmail-incubator-drill-dev@incubator.apache.org Received: (qmail 23366 invoked by uid 99); 5 May 2015 23:15:55 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 May 2015 23:15:55 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id EF4BB1DBEC7; Tue, 5 May 2015 23:15:54 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============4873098960589480621==" MIME-Version: 1.0 Subject: Re: Review Request 33770: DRILL-2697 Pause injections should pause indefinitely until signalled From: "Sudheesh Katkam" To: "Jacques Nadeau" , "Chris Westin" Cc: "Sudheesh Katkam" , "drill" Date: Tue, 05 May 2015 23:15:54 -0000 Message-ID: <20150505231554.20177.91770@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Sudheesh Katkam" X-ReviewGroup: drill-git X-ReviewRequest-URL: https://reviews.apache.org/r/33770/ X-Sender: "Sudheesh Katkam" References: <20150505213129.20176.84097@reviews.apache.org> In-Reply-To: <20150505213129.20176.84097@reviews.apache.org> Reply-To: "Sudheesh Katkam" X-ReviewRequest-Repository: drill-git --===============4873098960589480621== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On May 5, 2015, 9:31 p.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java, line 58 > > > > > > I'm not sure about this waiting uninterruptibly. We're going to start using Thread.interrupt() to regain control of threads that are blocked waiting on queues or sockets if the fragment they are running on behalf of is cancelled. Seems like this should be handled in the same way. I can talk to you about that more, and Venki can tell you about what he's doing in this area. This should not be a problem since this feature is used for testing (in a controlled environment). I feel it will in fact expose bugs; if not, we can remove this in the future. > On May 5, 2015, 9:31 p.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java, line 142 > > > > > > Does this mean that I can't have multiple pauses in the execution thread that will all work for a single query? For example, suppose I inject two pauses at different phases of execution: one just after planning and remote fragments are set up, and another after the first batch of results are returned. Will they both work, or will only the first one work? Yes, you can't. They will both be unpaused. I will re-submit a patch with client that unpauses a list of pause sites. - Sudheesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33770/#review82573 ----------------------------------------------------------- On May 2, 2015, 1:09 a.m., Sudheesh Katkam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33770/ > ----------------------------------------------------------- > > (Updated May 2, 2015, 1:09 a.m.) > > > Review request for drill, Chris Westin and Jacques Nadeau. > > > Repository: drill-git > > > Description > ------- > > DRILL-2697: Pauses sites wait indefinitely for a resume signal > DrillClient sends a resume signal to UserServer. UserServer triggers a resume call in the correct Foreman. Foreman resumes all pauses related to the query through the Control layer. > > + Fix for bug in FragmentExecutor#closeOutResources > + Better error messages and more tests in TestDrillbitResilience and TestPauseInjection > + Added execution controls to operator context > + Removed ControlMessageHandler interface, renamed ControlHandlerImpl to ControlMessageHandler > > > Diffs > ----- > > exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ae0f580 > exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControlRpcConfig.java 37730e3 > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControlTunnel.java a4f9fdf > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserRpcConfig.java 88592d4 > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java 9e929de > exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java b7ffbf0 > exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControls.java 1171bf8 > exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java e5f9c9c > exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java a3ceb8f > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlHandlerImpl.java b6c6852 > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlMessageHandler.java c5d78cc > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 34fa639 > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentManager.java 0ba91b4 > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java f526fbe > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java b1c3fe0 > exec/java-exec/src/main/java/org/apache/drill/exec/work/user/UserWorker.java 8854ef3 > exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java 2698701 > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java 508b10c > protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 813d961 > protocol/src/main/java/org/apache/drill/exec/proto/UserProtos.java c072a47 > protocol/src/main/java/org/apache/drill/exec/proto/beans/RpcType.java 4d03073 > protocol/src/main/protobuf/BitControl.proto 0424725 > protocol/src/main/protobuf/User.proto 59e22ae > > Diff: https://reviews.apache.org/r/33770/diff/ > > > Testing > ------- > > Passes all unit tests. > > > Thanks, > > Sudheesh Katkam > > --===============4873098960589480621==--