drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sudheesh Katkam" <skat...@maprtech.com>
Subject Re: Review Request 32327: DRILL-2437: enhance exception injection to support session level injections
Date Thu, 26 Mar 2015 22:55:33 GMT


> On March 26, 2015, 8:04 p.m., Chris Westin wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java,
line 180
> > <https://reviews.apache.org/r/32327/diff/1/?file=901649#file901649line180>
> >
> >     Is the default value validated somewhere else?

The default value is not validated. In fact it might not be valid. In the case of DRILLBIT_EXCEPTION_INJECTIONS,
you used the empty string (causes an ExpressionParsingException) as a quick check to not inject
anything.

I will change it to enforce a valid default value (and change the default for DRILLBIT_EXCEPTION_INJECTIONS).


> On March 26, 2015, 8:04 p.m., Chris Westin wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java,
line 121
> > <https://reviews.apache.org/r/32327/diff/1/?file=901651#file901651line121>
> >
> >     Is there any way to check if requestedPort was actually specified (if requestedAddress
is non-null), and isn't just defaulting to zero?

Sadly, no. Jackson assigns zero as a default value to int.


> On March 26, 2015, 8:04 p.m., Chris Westin wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java,
line 144
> > <https://reviews.apache.org/r/32327/diff/1/?file=901655#file901655line144>
> >
> >     Do we know that there's only one drillbit running for this test, or should we
just make the test injections here global?

(1) We do not know.

(2) This test just checks that DummyClass throws an unchecked exception in any bit context
(happens to be the context of bits[0]). So no.


- Sudheesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32327/#review77309
-----------------------------------------------------------


On March 20, 2015, 8:20 p.m., Sudheesh Katkam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32327/
> -----------------------------------------------------------
> 
> (Updated March 20, 2015, 8:20 p.m.)
> 
> 
> Review request for drill and Chris Westin.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> [DRILL-2437](https://issues.apache.org/jira/browse/DRILL-2437): Moved SimulatedExceptions
to UserSession
> 
> + [DRILL-2382](https://issues.apache.org/jira/browse/DRILL-2382): Added address and port
to InjectionOption to specify drillbit
> + JSONStringValidator to TypeValidators
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java cd0a0a2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java efb0cdf

>   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/TypeValidators.java
b9721cc 
>   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/SimulatedExceptions.java
0292c08 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java bfb6de8

>   exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java
9bc0552 
>   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 
> 
> Diff: https://reviews.apache.org/r/32327/diff/
> 
> 
> Testing
> -------
> 
> Added test case.
> 
> Passes all other tests, and builds.
> 
> 
> Thanks,
> 
> Sudheesh Katkam
> 
>


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