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

stack commented on HBASE-8763:
------------------------------

Patch looks great.

On commit, remove this comment.  Doesn't seem appropriate to the statement that 
follows:

+        // Record the mvcc for all transactions in progress.

You need to do this?

-
+      mvccNum.setValue(this.sequenceId.incrementAndGet());

I'm trying to keep it so all sequenceid increments happen inside HLog only.

You do it in a few places.  Does it have to increment?  Could you just get 
current?  Would that work?

This should almost be a method because it happens more than a few times:

+          if(walKey == null){
+            // Append a faked WALEdit in order for SKIP_WAL updates to get 
mvccNum assigned
+            walKey = this.appendNoSyncNoAppend(this.log, mvccNum);
+          }

Rename SequenceNumberAssignor as SequenceNumber and method as getSequenceNumber?

If no one else reviews in next few days, I'll give it another go (having 
trouble concentrating on this because I've looked at a few versions of this 
patch....).  Overall I think this is excellent.

Maybe we should just commit and then tune up in new issues?

Not sure we can get our speedup back looking at this patch at the mo.

> [BRAINSTORM] Combine MVCC and SeqId
> -----------------------------------
>
>                 Key: HBASE-8763
>                 URL: https://issues.apache.org/jira/browse/HBASE-8763
>             Project: HBase
>          Issue Type: Improvement
>          Components: regionserver
>            Reporter: Enis Soztutar
>            Assignee: Jeffrey Zhong
>            Priority: Critical
>         Attachments: HBase MVCC & LogSeqId Combined.pdf, 
> hbase-8736-poc.patch, hbase-8763-poc-v1.patch, hbase-8763-v1.patch, 
> hbase-8763-v2.patch, hbase-8763-v3.patch, hbase-8763-v4.patch, 
> hbase-8763_wip1.patch
>
>
> HBASE-8701 and a lot of recent issues include good discussions about mvcc + 
> seqId semantics. It seems that having mvcc and the seqId complicates the 
> comparator semantics a lot in regards to flush + WAL replay + compactions + 
> delete markers and out of order puts. 
> Thinking more about it I don't think we need a MVCC write number which is 
> different than the seqId. We can keep the MVCC semantics, read point and 
> smallest read points intact, but combine mvcc write number and seqId. This 
> will allow cleaner semantics + implementation + smaller data files. 
> We can do some brainstorming for 0.98. We still have to verify that this 
> would be semantically correct, it should be so by my current understanding.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to