[
https://issues.apache.org/jira/browse/HBASE-18962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16244690#comment-16244690
]
Umesh Agashe commented on HBASE-18962:
--------------------------------------
Thanks [~appy] for you review. While revisiting the test based on review
comments, I spotted some improvements which I've included in patch 002. Please
see my comments below:
bq. Single unit test should never test multiple code paths simultaneously.
I agree with the point in it's general sense. But the current test doesn't test
it simultaneously. Its sequential. IMO some discretion is required regarding
"multiple code paths". E.g. if/ switch statement with a few cases. There are
different code paths per case, but writing per case unit test may not be
helpful. As it is, the test is testing atomic batch behavior. A couple of
tests just above this test also has success and failure cases in single test.
There is also initialization, that will have to be run for each tests. If you
insist, I can create multiple tests.
bq. how it was *needed* to order them in that particular order otherwise it
wouldn't have worked
It was *not* needed. Its coincidental. Test can be written in any order with a
few changes to asserts and not using ExpectedException and it will work.
bq. when middle one is in separate test, you won't need separate thread for
region.batchMutate()
Separate thread is needed because getRowLock() is reentrant. It can be called
repeatedly from same thread. It has nothing to do with ordering in the test.
Patch 002 has some additional changes. Let me know your feedback. Thanks!
> Support atomic BatchOperations through batchMutate()
> ----------------------------------------------------
>
> Key: HBASE-18962
> URL: https://issues.apache.org/jira/browse/HBASE-18962
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver
> Affects Versions: 2.0.0-alpha-3
> Reporter: Umesh Agashe
> Assignee: Umesh Agashe
> Fix For: 2.0.0-beta-1
>
> Attachments: hbase-18962.master.001.patch,
> hbase-18962.master.002.patch
>
>
> Support all mutations in BatchOperations to be applied atomically (all or
> none) by locking all rows corresponding to mutations exclusively.
> mutateRows() which uses MultiRowMutationProcessor applies all mutations
> atomically and batchMutate() is non-atomic. To unify code paths, isAtomic()
> attribute can be added to BatchOperations.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
