[ 
https://issues.apache.org/jira/browse/HBASE-7329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13558973#comment-13558973
 ] 

Sergey Shelukhin commented on HBASE-7329:
-----------------------------------------

bq. Have you done much testing of this patch SS?
I've run mvn tests; I will run some integration tests.

bq. Nit: Below looks like it should be class comment rather than internal 
implementation comment or do you think otherwise?
It's actually an implementation comment since it describes the implementation 
:) Class and method javadoc describe the external stuff.

bq.    Initially, the number of operations is 1.
bq. Is it right having it at 1 when we construct the class? Should we wait on 
first beginOp call?
Having 1 at start allows us to maintain a simple invariant that decrement to 0 
is always the last.

bq. It is not your fault but that is sure an ugly name on a method, 
getCompleteCacheFlushSequenceId. From its name you would not know what it is 
for.
bq. In fact, if you want to remove it it looks like you could since 
'TransactionalRegion' is a facility that no longer exists and going forward if 
you wanted to do this kinda thing, you'd do it via a coprocessor.
Removed.

bq. Is it right getting seqid before we advance memstore? We used to do it 
other way around.
Fixed.

bq. I wonder if the log of the roll should be outside of the lock...? Could be 
a bit sloppy but maybe it does not have to be too precise?
You mean rollWriter? The only reason it's inside the lock is to prevent two 
concurrent rolls. It can be done differently e.g. we could
lock around a boolean check and set, and bail duplicates. However current logic 
worked as having rollWriter-s queue up, not sure if anything relies on that.

I will submit another patch, then run some integration tests on real cluster.
                
> remove flush-related records from WAL and make locking more granular
> --------------------------------------------------------------------
>
>                 Key: HBASE-7329
>                 URL: https://issues.apache.org/jira/browse/HBASE-7329
>             Project: HBase
>          Issue Type: Improvement
>          Components: wal
>    Affects Versions: 0.96.0
>            Reporter: Sergey Shelukhin
>            Assignee: Sergey Shelukhin
>             Fix For: 0.96.0
>
>         Attachments: 7329-findbugs.diff, HBASE-7329-v0.patch, 
> HBASE-7329-v0.patch, HBASE-7329-v0-tmp.patch, HBASE-7329-v1.patch, 
> HBASE-7329-v1.patch, HBASE-7329-v2.patch, HBASE-7329-v3.patch, 
> HBASE-7329-v4.patch, HBASE-7329-v5.patch
>
>
> Comments from many people in HBASE-6466 and HBASE-6980 indicate that flush 
> records in WAL are not useful. If so, they should be removed.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to