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

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


Copy link to this message
-
Adding frameworks and coding for testabilitywas [Re: Testing the fix for race condition between Compaction and StoreScanner.init]
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.

<soapbox>Generally tests should be able to setup custom objects with
constructors so we can use mockito or something similar in unit tests.  If
done well we can instead of adding runtime injector objects to production
code.  I'm making a plea to spend sometime tightening the code up and to
conveying more design in design.   </soapbox>

Recently been spending time in the hlog area and will likely personally
start there improving comments/names, conveying design intent, and
hopefully improving it. ;)

Jon.

On Wed, Nov 20, 2013 at 9:48 AM, Ted Yu <[EMAIL PROTECTED]> wrote:

> As requested by Jonathan, I am posting the following approach for testing
> fix of HBASE-9949 :
>
>
> 1. Introduce config parameter for StoreScanner implementation which would
> be used by HStore for creating scanner
> 2. In TestStoreScanner, add StoreScanner implementation which extends
> StoreScanner and set the above config parameter to this class.
> 3. Register custom ChangedReadersObserver through the following API of
> HStore :
>
>   public void addChangedReaderObserver(ChangedReadersObserver o) {
>
> The BEFORE_SEEK hook would be activated before Store.getScanner() is
> called.
> The AFTER_SEEK hook can be activated at the end of ctor of StoreScanner
> wrapper created in #2
> The custom ChangedReadersObserver would activate the COMPACT_COMPLETE hook.
>
> Comments are welcome.
>

--
// Jonathan Hsieh (shay)
// Software Engineer, Cloudera
// [EMAIL PROTECTED]