Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
HBase >> mail # dev >> Adding frameworks and coding for testabilitywas [Re: Testing the fix for race condition between Compaction and StoreScanner.init]


+
Jonathan Hsieh 2013-11-20, 23:27
+
Enis Söztutar 2013-11-21, 00:37
+
Andrew Purtell 2013-11-20, 23:40
+
Stack 2013-11-22, 20:54
+
Andrew Purtell 2013-11-23, 18:40
+
Ted Yu 2013-11-23, 18:59
Copy link to this message
-
Re: Adding frameworks and coding for testabilitywas [Re: Testing the fix for race condition between Compaction and StoreScanner.init]
I haven't seen the particular "framework" in question, but I do think
there's a continuum of reasonable choices here, ranging from full DI and
using mocks/spys, to using AspectJ for AOP, to dding specific injections
into the code at key points. I've experimented with probably the full range
of options, and what I found to work best in HDFS was the addition of
"FaultInjector" interfaces for each area of the code. For example:
https://github.com/apache/hadoop-common/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/CheckpointFaultInjector.java
example usage:
https://github.com/apache/hadoop-common/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/TransferFsImage.java#L126

I see Jon's point that this litters the code with explicit injection
points. However, I disagree that that's a bad thing. Seeing a call to
FaultInjector.foo() in the code makes a future developer more wary of where
they need to be careful -- and ensures that, if they change the design of
that area of the code, that they need to consider the same faults as the
original. Additionally, contrary to the DI approach (which typically
involves splitting classes into bunches of smaller units), the changes for
this type of fault injection are well-contained: they add a line here or
there, but they don't affect the higher-level design of the code, so you're
free to code in a way that is more understandable instead of ending up with
CheckpointDownloadStrategyFactoryFactories or something.

Just one guy's opinion :)

-Todd

On Fri, Nov 22, 2013 at 12:54 PM, Stack <[EMAIL PROTECTED]> wrote:

> On Wed, Nov 20, 2013 at 3:27 PM, Jonathan Hsieh <[EMAIL PROTECTED]> wrote:
>
> > tl;dr if we are going to add new frameworks, please make them a separate
> > patch.  another attempt to bring up coding for testability/dep injection
> > again.
> >
> > I've changed the subject because I really meant this to be a bigger
> > discussion about setting up our code to be more testable and using
> > conventions that allow us to do dependency injection (ala guava or in a
> way
> > we can use mockito) or deciding to to include this new-to-me
> > InjectionHandler frameworks from the 89-fb branch.  Porting HBASE-9949's
> > testing code depended this Injector infrastructure that I found
> > distasteful.
> >
> > This particular example smells funny and looked like another stovepiped
> > framework to me --
> > 1) It added references and objects to our core code instead of having it
> > only present for our tests.
> > 2) This made the core code more cumbersome to read.
> > 3) Initial version used a brittle convention.   Initially, the injection
> > point was identified by passing empty object array of particular size (0
> > size mean option 0, size 1 meant injection point 1, etc).  Later changed
> to
> > a enum, As used in the 89-fb branch we have a new global pool of
> unrelated
> > enum/constant values that seemed brittle for further extension.
> >
> >
> How did such a mess even get committed?
> St.Ack
>

--
Todd Lipcon
Software Engineer, Cloudera
+
Andrew Purtell 2013-11-23, 18:51
+
Steve Loughran 2013-11-25, 10:27
+
Enis Söztutar 2013-11-25, 19:06
+
Sergey Shelukhin 2013-11-25, 22:35
+
Jonathan Hsieh 2013-11-26, 17:51