----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32795/#review80037 -----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java <https://reviews.apache.org/r/32795/#comment129788> Why do FragmentContext and QueryContext both need their own set of ExecutionControls? exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java <https://reviews.apache.org/r/32795/#comment129789> newBuilder() goes on the previous line, since it's not actually setting an attribute in the builder. Do we really need these empty end-of-line comments? exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java <https://reviews.apache.org/r/32795/#comment129790> If you are going to use these end-of-line comments, then always space them the same way, and do it on all lines exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java <https://reviews.apache.org/r/32795/#comment129791> This doesn't look like an exact substitution; what's this change about? exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java <https://reviews.apache.org/r/32795/#comment129792> What's going on here? Can we get some comments explaining this? I would have expected the state to be FINISHED or CANCELLED. exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java <https://reviews.apache.org/r/32795/#comment129793> Can we get a comment describing what this is? exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java <https://reviews.apache.org/r/32795/#comment129794> Comments for the arguments? exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java <https://reviews.apache.org/r/32795/#comment129795> Should this be validated? Is any value ok? exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java <https://reviews.apache.org/r/32795/#comment129796> -1 has a special meaning? exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java <https://reviews.apache.org/r/32795/#comment129798> This looks stateless -- could it be static? - Chris Westin On April 13, 2015, 9:45 p.m., Sudheesh Katkam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32795/ > ----------------------------------------------------------- > > (Updated April 13, 2015, 9:45 p.m.) > > > Review request for drill, abdelhakim deneche, Chris Westin, and Jacques > Nadeau. > > > Repository: drill-git > > > Description > ------- > > [DRILL-2383](https://issues.apache.org/jira/browse/DRILL-2383): Support to > inject exceptions and pauses in various components of Drill > > + Controls can be introduced in any class that has access to > FragmentContext/QueryContext > + Controls are fired only if assertions are enabled > + Controls can be fired by altering the DRILLBIT_CONTROL_INJECTIONS session > option > + Renames: SimulatedExceptions => ExecutionControls, ExceptionInjector => > ExecutionControlsInjector > + Added injection sites in Foreman, DrillSqlWorker, FragmentExecutor > + Unit tests in TestDrillbitResilience, TestExceptionInjection and > TestPauseInjection > > Other commits included: > > + [DRILL-2437](https://issues.apache.org/jira/browse/DRILL-2437): Moved > ExecutionControls from DrillbitContext to FragmentContext/QueryContext > + [DRILL-2382](https://issues.apache.org/jira/browse/DRILL-2382): Added > address and port to Injection to specify drillbit > + [DRILL-2384](https://issues.apache.org/jira/browse/DRILL-2384): Added > QueryState to SingleRowListener and assert that state is COMPLETED while > testing > > Other edits: > > + Support for short lived session options in SessionOptionManager (using TTL > in OptionValidator) > + Introduced query count in UserSession > + Added QueryState to queryCompleted() in UserResultsListener to check if > COMPLETED/CANCELED > + Added JSONStringValidator to TypeValidators > + Log query id as string in DrillClient, WorkEventBus, QueryResultHandler > + Use try..catch block only around else clause for OptionList in > FragmentContext > + Fixed drillbitContext spelling error in QueryContext > + Do not call setLocalOption twice in FallbackOptionManager > + Show explicitly that submitWork() returns queryId in UserServer > + Updated protocol/readme.txt to include an alternative way to generate > sources > > > ===== > USAGE: > > Current checked exception sites: > > + Foreman: run-try-beginning (ForemanException), run-try-end > (ForemanException), send-fragments (ForemanException) > + DrillSqlWorker: sql-parsing (ForemanSetupException) > + FragmentExecutor: fragment-execution (IOException) > > Current pause sites: > > + Foreman: pause-run-plan > > To set controls: > ``` > > ALTER SESSION SET `drill.exec.testing.controls`='{ > "injections":[ > { > "type":"exception", > "siteClass": "org.apache.drill.exec.work.fragment.FragmentExecutor", > "desc": "fragment-execution", > "nSkip": 0, > "nFire": 1, > "exceptionClass": "java.io.IOException", > "address": "10.10.10.10", > "port": 31019 > }, > { > "type":"pause", > "siteClass": "org.apache.drill.exec.work.foreman.Foreman", > "desc": "pause-run-plan", > "nSkip": 0, > "nFire": 1, > "millis": 5000 > } > ] }'; > ``` > NOTE: > (1) If controls are specified, they are passed to every fragment as part of > PlanFragment. Then onwards, ExecutionControls live in FragmentContext. And > since FragmentContext is created for every fragment, injections will be fired > on ALL fragments. > (2) address and port are optional. If they are set, that injection will be > fired ONLY on specified drillbit. If they are not set, the injection will be > fired on EVERY drillbit. > > > Diffs > ----- > > exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java > bd93206 > exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java > 579cf7d > > exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java > 98948af > exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java > da2229c > exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java > 2fa0b18 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java > b98778d > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java > a5a5441 > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java > a1be83b > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserResultsListener.java > 934a094 > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java > 877bc08 > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java > 19d77b0 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java > dbf3c74 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java > 45d393c > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java > 4ffe9a3 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValidator.java > 43071e7 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java > c3de190 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java > 1a8559e > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java > b9721cc > > exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java > fbbf0b8 > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjection.java > 68cbf08 > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjector.java > 54bc351 > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControls.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/testing/Injection.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/InjectionConfigurationException.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/InjectionSite.java > 9e19fdd > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/NoOpControlsInjector.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java > 0292c08 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java > 23ef0d3 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java > a7e6c46 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/user/UserWorker.java > 854f474 > exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java 0c2f0e5 > exec/java-exec/src/test/java/org/apache/drill/PlanningBase.java 264123f > exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java > 5703bf9 > > exec/java-exec/src/test/java/org/apache/drill/exec/compile/TestClassTransformation.java > f5f5b8d > > exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java > e03098a > > exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java > 55f0d75 > > exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetPhysicalPlan.java > 882cdbd > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/ControlsInjectionUtil.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/ExceptionInjectionUtil.java > bf93dee > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java > d0c0279 > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java > PRE-CREATION > exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java > 24ef62b > protocol/readme.txt bd516d3 > > Diff: https://reviews.apache.org/r/32795/diff/ > > > Testing > ------- > > Unit tests in TestDrillbitResilience, TestExceptionInjection and > TestPauseInjection. > Successful Jenkins builds. > > > Thanks, > > Sudheesh Katkam > >
