Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Date: Wed, 25 Jan 2017 07:36:28 +0000 (UTC)
From: "Yu Li (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: <JIRA.13035030.1484496873000.196975.1485329788273@Atlassian.JIRA>
In-Reply-To: <JIRA.13035030.1484496873000@Atlassian.JIRA>
References: <JIRA.13035030.1484496873000@Atlassian.JIRA> <JIRA.13035030.1484496873149@jira-lw-us.apache.org>
Subject: [jira] [Comment Edited] (HBASE-17471) Region Seqid will be out of
 order in WAL if using mvccPreAssign
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Wed, 25 Jan 2017 07:36:35 -0000


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

Yu Li edited comment on HBASE-17471 at 1/25/17 7:36 AM:
--------------------------------------------------------

bq. As we tested this patch in our custom HBase-1.1.2, there is no regression either
Great, good to know.

bq. The order of steps in doMiniBatchMutation will not influence the mvcc assign.
The order matters w/o mvcc preassign.
However, it's true that here we only need to compare the old and new design for preassign, although you also listed the performance for the "Preassign off" case for master branch.

bq. But still, if I have time, I will post data on branch-1
Since you've checked with your custom 1.1.2, I guess no surprise for branch-1. But yes, better to check if time allows.

bq. we actually don't need to stamp mvcc/seqid to cells in the wal endits. We only need to stamp them to cells in the memstore
I don't think so. We need to stamp mvcc/seqenceId before WAL sync, and keep cells in WAL edits and memstore exactly the same, or else there might be disorder during WAL replay.

Regarding the UT improvement, I think we need to differentiate the below two cases:
1. The WAL-only test cases
    - For this case, I think the change Duo made (create WALKey w/o MVCC) is enough

2. The test cases involving both mvcc and WAL, like those in {{TestHRegion}}
    - For this case, obviously our test coverage is not enough and missed some cases, and we need to complete them.

I took it for granted when [~allan163] mentioned "they are working just because they don't wait on mvcc" he meant case #2. Could you clarify here [~allan163]? Or in another word, does the newly added UT cases in current patch enough to cover all known issues from your observation in product env? Thanks.


was (Author: carp84):
bq. As we tested this patch in our custom HBase-1.1.2, there is no regression either
Great, good to know.

bq. The order of steps in doMiniBatchMutation will not influence the mvcc assign.
The order matters w/o mvcc preassign.
However, it's true that here we only need to compare the old and new design for preassign, although you also listed the performance for the "Preassign off" case for master branch.

bq. But still, if I have time, I will post data on branch-1
Since you've checked with your custom 1.1.2, I guess no surprise for branch-1. But yes, better to check if time allows.

bq. we actually don't need to stamp mvcc/seqid to cells in the wal endits. We only need to stamp them to cells in the memstore
I don't think so. We need to stamp mvcc/seqenceId before WAL sync, and keep cells in WAL edits and memstore exactly the same, or else there might be disorder during WAL replay.

Regarding the UT improvement, I think we need to differentiate the below two cases:
1. The WAL-only test cases
    - For this case, I think the change Duo made (create WALKey w/o MVCC) is enough

2. The test cases involving both mvcc and WAL, like those in {{TestHRegion}}
    - For this case, obviously our test coverage is not enough and missed some cases, and we need to complete them.

I took it for granted when [~allan163] mentioned "they are working just because they don't wait on mvcc" he meant case #2. Could you clarify here [~allan163]? Or in another word, does the newly added UT cases in current patch enough to cover all known issues from your point of view? Thanks.

> Region Seqid will be out of order in WAL if using mvccPreAssign
> ---------------------------------------------------------------
>
>                 Key: HBASE-17471
>                 URL: https://issues.apache.org/jira/browse/HBASE-17471
>             Project: HBase
>          Issue Type: Bug
>          Components: wal
>    Affects Versions: 2.0.0, 1.4.0
>            Reporter: Allan Yang
>            Assignee: Allan Yang
>            Priority: Critical
>         Attachments: HBASE-17471-duo.patch, HBASE-17471-duo-v1.patch, HBASE-17471-duo-v2.patch, HBASE-17471.patch, HBASE-17471.tmp, HBASE-17471.v2.patch, HBASE-17471.v3.patch, HBASE-17471.v4.patch, HBASE-17471.v5.patch, HBASE-17471.v6.patch
>
>
>  mvccPreAssign was brought by HBASE-16698, which truly improved the performance of writing, especially in ASYNC_WAL scenario. But mvccPreAssign was only used in {{doMiniBatchMutate}}, not in Increment/Append path. If Increment/Append and batch put are using against the same region in parallel, then seqid of the same region may not monotonically increasing in the WAL. Since one write path acquires mvcc/seqid before append, and the other acquires in the append/sync consume thread.
> The out of order situation can easily reproduced by a simple UT, which was attached in the attachment. I modified the code to assert on the disorder: 
> {code}
>     if(this.highestSequenceIds.containsKey(encodedRegionName)) {
>       assert highestSequenceIds.get(encodedRegionName) < sequenceid;
>     }
> {code}
> I'd like to say, If we allow disorder in WALs, then this is not a issue. 
> But as far as I know, if {{highestSequenceIds}} is not properly set, some WALs may not archive to oldWALs correctly.
> which I haven't figure out yet is that, will disorder in WAL cause data loss when recovering from disaster? If so, then it is a big problem need to be fixed.
> I have fix this problem in our costom1.1.x branch, my solution is using mvccPreAssign everywhere, making it un-configurable. Since mvccPreAssign it is indeed a better way than assign seqid in the ringbuffer thread while keeping handlers waiting for it.
> If anyone think it is doable, then I will port it to branch-1 and master branch and upload it. 
>  


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)