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

stack commented on HBASE-5689:
------------------------------

Looking at the patch, it looks good to me.

On the Math.abs, its probably not needed any more given current file formats 
but its not doing any harm.  I tried to go back through src history to see how 
we used name the recovered edits files -- they used be called stuff like 
oldlogfile.log, etc -- and they have been named for first seqid in the file 
since 0.90 so its unlikely this will ever go negative or that this code will 
encouter a file named negatively.

I also thought that perhaps this patch only for trunk because what about the 
case where we have existing wal files that are still named by the first seqid 
in the file?  But this should be fine.  We'll over-replay rather than 
under-replay edits in this case so it should be fine.

+1 on commit adding back the Math.abs on commit (though reasoning would have it 
we don't need, I think we should add it back just-in-case).
                
> Skipping RecoveredEdits may cause data loss
> -------------------------------------------
>
>                 Key: HBASE-5689
>                 URL: https://issues.apache.org/jira/browse/HBASE-5689
>             Project: HBase
>          Issue Type: Bug
>          Components: regionserver
>    Affects Versions: 0.94.0
>            Reporter: chunhui shen
>            Assignee: chunhui shen
>            Priority: Critical
>             Fix For: 0.94.0
>
>         Attachments: 5689-testcase.patch, 5689-v4.txt, HBASE-5689.patch, 
> HBASE-5689.patch, HBASE-5689v2.patch, HBASE-5689v3.patch
>
>
> Let's see the following scenario:
> 1.Region is on the server A
> 2.put KV(r1->v1) to the region
> 3.move region from server A to server B
> 4.put KV(r2->v2) to the region
> 5.move region from server B to server A
> 6.put KV(r3->v3) to the region
> 7.kill -9 server B and start it
> 8.kill -9 server A and start it 
> 9.scan the region, we could only get two KV(r1->v1,r2->v2), the third 
> KV(r3->v3) is lost.
> Let's analyse the upper scenario from the code:
> 1.the edit logs of KV(r1->v1) and KV(r3->v3) are both recorded in the same 
> hlog file on server A.
> 2.when we split server B's hlog file in the process of ServerShutdownHandler, 
> we create one RecoveredEdits file f1 for the region.
> 2.when we split server A's hlog file in the process of ServerShutdownHandler, 
> we create another RecoveredEdits file f2 for the region.
> 3.however, RecoveredEdits file f2 will be skiped when initializing region
> HRegion#replayRecoveredEditsIfAny
> {code}
>  for (Path edits: files) {
>       if (edits == null || !this.fs.exists(edits)) {
>         LOG.warn("Null or non-existent edits file: " + edits);
>         continue;
>       }
>       if (isZeroLengthThenDelete(this.fs, edits)) continue;
>       if (checkSafeToSkip) {
>         Path higher = files.higher(edits);
>         long maxSeqId = Long.MAX_VALUE;
>         if (higher != null) {
>           // Edit file name pattern, HLog.EDITFILES_NAME_PATTERN: "-?[0-9]+"
>           String fileName = higher.getName();
>           maxSeqId = Math.abs(Long.parseLong(fileName));
>         }
>         if (maxSeqId <= minSeqId) {
>           String msg = "Maximum possible sequenceid for this log is " + 
> maxSeqId
>               + ", skipped the whole file, path=" + edits;
>           LOG.debug(msg);
>           continue;
>         } else {
>           checkSafeToSkip = false;
>         }
>       }
> {code}
>  

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to