hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonathan Hsieh <...@cloudera.com>
Subject Re: Testing the fix for race condition between Compaction and StoreScanner.init
Date Mon, 02 Dec 2013 19:06:50 GMT
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

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


On Mon, Dec 2, 2013 at 9:54 AM, Ted Yu <yuzhihong@gmail.com> wrote:

> Is the proposed approach below acceptable ?
> Cheers
> On Wed, Nov 20, 2013 at 9:48 AM, Ted Yu <yuzhihong@gmail.com> 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
// jon@cloudera.com

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