drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Westin" <chriswesti...@gmail.com>
Subject Re: Review Request 32795: DRILL-2383: Add exception and pause injections for testing drillbit stability
Date Tue, 14 Apr 2015 16:28:32 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message