[jira] [Commented] (HBASE-5523) Fix Delete Timerange logic for KEEP_DELETED_CELLS

["Hadoop QA (Commented) (JIRA)" <ji...@apache.org>]

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

Hadoop QA commented on HBASE-5523:
----------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12517170/5523.txt
  against trunk revision .

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 6 new or modified tests.

    -1 javadoc.  The javadoc tool appears to have generated -129 warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    -1 findbugs.  The patch appears to introduce 154 new Findbugs (version 1.3.9) warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    +1 core tests.  The patch passed unit tests in .

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1105//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1105//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1105//console

This message is automatically generated.
                
> Fix Delete Timerange logic for KEEP_DELETED_CELLS
> -------------------------------------------------
>
>                 Key: HBASE-5523
>                 URL: https://issues.apache.org/jira/browse/HBASE-5523
>             Project: HBase
>          Issue Type: Sub-task
>          Components: regionserver
>            Reporter: Lars Hofhansl
>            Assignee: Lars Hofhansl
>            Priority: Minor
>             Fix For: 0.94.0, 0.96.0
>
>         Attachments: 5523.txt
>
>
> A Delete at time T marks a Put at time T as deleted.
> In parent I invented special logic that insert a virtual millisecond into the tr if the encountered KV is a delete marker.
> This was so that there is a way to specify a timerange that would allow to see the put but not the delete:
> {code}
> if (kv.isDelete()) {
>   if (!keepDeletedCells) {
>     // first ignore delete markers if the scanner can do so, and the
>     // range does not include the marker
>     boolean includeDeleteMarker = seePastDeleteMarkers ?
>     // +1, to allow a range between a delete and put of same TS
>     tr.withinTimeRange(timestamp+1) :
>     tr.withinOrAfterTimeRange(timestamp);
> {code}
> Discussed this today with a coworker and he convinced me that this is very confusing and also not needed.
> When we have a Delete and Put at the same time T, there *is* not timerange that can include the Put but not the Delete.
> So I will change the code to this (and fix the tests):
> {code}
> if (kv.isDelete()) {
>   if (!keepDeletedCells) {
>     // first ignore delete markers if the scanner can do so, and the
>     // range does not include the marker
>     boolean includeDeleteMarker = seePastDeleteMarkers ?
>     tr.withinTimeRange(timestamp) :
>     tr.withinOrAfterTimeRange(timestamp);
> {code}
> It's easier to understand, and does not lead to strange scenarios when the TS is used as a controlled counter.
> Needs to be done before 0.94 goes out.

--
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