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

Switch to Threaded View
HBase, mail # dev - Testing the fix for race condition between Compaction and StoreScanner.init


Copy link to this message
-
Re: Testing the fix for race condition between Compaction and StoreScanner.init
Jonathan Hsieh 2013-12-03, 05:06
On Mon, Dec 2, 2013 at 7:34 PM, Ted Yu <[EMAIL PROTECTED]> wrote:

> bq. The lastest submitted trunk patch
>
> The proposal here wouldn't use minicluster.
>
> Great!
> Cheers
>
>
> On Mon, Dec 2, 2013 at 7:31 PM, Jonathan Hsieh <[EMAIL PROTECTED]> wrote:
>
> > On Mon, Dec 2, 2013 at 1:13 PM, Ted Yu <[EMAIL PROTECTED]> wrote:
> >
> > > bq.  Would it be possible just to have it focus on just a region/store
> > > level instead?
> > >
> > > TestStoreScanner doesn't start minicluster.
> > >
> > >
> > The lastest submitted trunk patch most certainly did.
> >
> > I think you can just create a standalone region with data with hooks for
> > testing when creating the store scanner
> >
> > +  /*
> > +   *  Verifies that there is no race condition between StoreScanner
> > construction and compaction.
> > +   *  This is done through 3 injection points:
> > +   *  1. before seek operation in StoreScanner ctor
> > +   *  2. after seek operation in StoreScanner ctor
> > +   *  3. after compaction completion
> > +   */
> > +  public void testCompactionRaceCondition() throws Exception {
> > +    HBaseTestingUtility util = new HBaseTestingUtility();
> > +    util.startMiniCluster(1);
> > +    byte[] t = Bytes.toBytes("tbl"), cf = Bytes.toBytes("cf");
> > +
> >
> > bq. Can you add enough info so that we don't need to consult the jira to
> > > figure out what the unit test is testing?
> > >
> > > Sure. I will add comments to the new test.
> > >
> > Looking forwards to it.
> >
> >
> > > Cheers
> > >
> > >
> > > On Mon, Dec 2, 2013 at 11:06 AM, Jonathan Hsieh <[EMAIL PROTECTED]>
> > wrote:
> > >
> > > > Part of the reason the injection stuff was "needed" was because the
> > test
> > > > crafted data using the higher level minicluster.  Would it be
> possible
> > > just
> > > > to have it focus on just a region/store level instead?  Along with
> the
> > > > focused unit test could a separate easy-to-read easy-to-maintain
> system
> > > > test be written as as well that compacts and starts scanners and
> > verifies
> > > > correctness.
> > > >
> > > > At this level, the code does not explain what the potential race we
> are
> > > > checking against is (other than something between storescanner
> > > construction
> > > > and compaction).  Can you add enough info so that we don't need to
> > > consult
> > > > the jira to figure out what the unit test is testing?
> > > >
> > > > Part of the issue is that the current patch and code attached doesn't
> > > make
> > > > it clear what is being verified.  There are no obvious assertions
> about
> > > > what is being checked (its tucked away in the
> > > > StoreScannercompactionRaceCondition InjectionHandler, at the least
> the
> > > > assertion should be at the testXxx method level.)
> > > >
> > > > I think the current test has hygiene issues as well (if the assert
> > > fails, I
> > > > think we leave a minicluster up?)
> > > >
> > > > Jon.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Mon, Dec 2, 2013 at 9:54 AM, Ted Yu <[EMAIL PROTECTED]> wrote:
> > > >
> > > > > Is the proposed approach below acceptable ?
> > > > >
> > > > > Cheers
> > > > >
> > > > >
> > > > > 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.

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