bq. The lastest submitted trunk patch The proposal here wouldn't use minicluster.
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. > > > > > 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] > > > > > > > > > -- > // Jonathan Hsieh (shay) > // Software Engineer, Cloudera > // [email protected] >
