> On April 7, 2015, 2:19 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java, > > line 73 > > <https://reviews.apache.org/r/32795/diff/3/?file=918270#file918270line73> > > > > Please move all this functionality to the option managers and option > > validators. For example, have a hook in the validator to execute a command > > against the option manager. Add which types of options are allowed at > > which levels, etc. (I think the framing is already there for this.)
Step (1) can be moved, which is the TODO. Step (2) is setting the option, same as else clause. Since ExecutionControls lives inside UserSession (accessed through QueryContext), step (3) cannot be moved unless the hook also has QueryCotext as a parameter. > On April 7, 2015, 2:19 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjection.java, > > line 47 > > <https://reviews.apache.org/r/32795/diff/3/?file=918283#file918283line47> > > > > Can you please move to using JsonCreator and doing all validation at > > deserialization time? This should also remove the need for two separate > > classes. > > Sudheesh Katkam wrote: > Incorrect values should throw a ValidationException. Validation at > deserialization time would not allow that, which is why there are two classes. Or can you provide more details? - Sudheesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32795/#review79108 ----------------------------------------------------------- On April 6, 2015, 10:53 p.m., Sudheesh Katkam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32795/ > ----------------------------------------------------------- > > (Updated April 6, 2015, 10:53 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): Inject > exceptions and pauses in various components of Drill > > + Controls can be introduced in any class that has access to FragmentContext > or QueryContext > + Controls can be fired by altering the DRILLBIT_CONTROL_INJECTIONS session > option > + Renames: SimulatedExceptions => ExecutionControls, ExceptionInjector => > ExecutionControlsInjector > + Instructions to add other types of injections are in Injection > + ExecutionControls are added as a side effect of altering > DRILLBIT_CONTROL_INJECTIONS session option > + 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 UserSession > + [DRILL-2382](https://issues.apache.org/jira/browse/DRILL-2382): Added > address and port to InjectionOption to specify drillbit > > Other bugs fixed: > > + BUG: Only one SystemRecord (static instance) exists per node. This causes > race conditions when multiple > drillbits run on the same node. FIX: create a new instance per request. > > Other edits: > > + Added QueryId back to QueryContext > + 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 > + Show explicitly that submitWork() returns queryId in UserServer > + Added an alternative way to generate sources in protocol/readme.txt > + Added JSONStringValidator to TypeValidators > > > ===== > 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", > "drillbitAddress": "10.10.10.10", > "userPort": 31019 > }, > { > "type":"pause", > "siteClass": "org.apache.drill.exec.work.foreman.Foreman", > "desc": "pause-run-plan", > "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) drillbitAddress and userPort 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 > 9a948fb > > 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 > a4ac724 > exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java > 3b51a69 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/PhysicalPlanReader.java > 8d77136 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java > 66ba229 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java > d6f25fb > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java > dc63ef9 > > 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/SystemOptionManager.java > 608fac7 > > 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/store/sys/MemoryRecord.java > 9cb001d > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTable.java > 2c338ca > > exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/ThreadsRecord.java > b184880 > > 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/InjectionSite.java > 9e19fdd > > 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/test/java/org/apache/drill/BaseTestQuery.java 725594a > exec/java-exec/src/test/java/org/apache/drill/PlanningBase.java e673230 > exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java > 5703bf9 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java > ba905c4 > > 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/DrillResultSet.java fb27d2d > protocol/readme.txt bd516d3 > protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 813d961 > protocol/src/main/java/org/apache/drill/exec/proto/SchemaBitControl.java > 5e7562e > protocol/src/main/java/org/apache/drill/exec/proto/beans/PlanFragment.java > f6fbce1 > protocol/src/main/protobuf/BitControl.proto 0424725 > > Diff: https://reviews.apache.org/r/32795/diff/ > > > Testing > ------- > > Unit tests in TestDrillbitResilience, TestExceptionInjection and > TestPauseInjection. > Successful Jenkins builds. > > > Thanks, > > Sudheesh Katkam > >
