HBase
  1. HBase
  2. HBASE-8753

Provide new delete flag which can delete all cells under a column-family which have designated timestamp

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.95.1
    • Fix Version/s: 0.98.0, 0.95.2
    • Component/s: Deletes, Scanners
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      In one of our production scenario (Xiaomi message search), multiple cells will be put in batch using a same timestamp with different column names under a specific column-family.

      And after some time these cells also need to be deleted in batch by given a specific timestamp. But the column names are parsed tokens which can be arbitrary words , so such batch delete is impossible without first retrieving all KVs from that CF and get the column name list which has KV with that given timestamp, and then issuing individual deleteColumn for each column in that column-list.

      Though it's possible to do such batch delete, its performance is poor, and customers also find their code is quite clumsy by first retrieving and populating the column list and then issuing a deleteColumn for each column in that column-list.

      This feature resolves this problem by introducing a new delete flag: DeleteFamilyVersion.

      1). When you need to delete all KVs under a column-family with a given timestamp, just call Delete.deleteFamilyVersion(cfName, timestamp); only a DeleteFamilyVersion type KV is put to HBase (like DeleteFamily / DeleteColumn / Delete) without read operation;

      2). Like other delete types, DeleteFamilyVersion takes effect in get/scan/flush/compact operations, the ScanDeleteTracker now parses out and uses DeleteFamilyVersion to prevent all KVs under the specific CF which has the same timestamp as the DeleteFamilyVersion KV to pop-up as part of a get/scan result (also in flush/compact).

      Our customers find this feature efficient, clean and easy-to-use since it does its work without knowing the exact column names list that needs to be deleted.

      This feature has been running smoothly for a couple of months in our production clusters.

      1. HBASE-8753-trunk-V3.patch
        14 kB
        Honghua Feng
      2. HBASE-8753-trunk-V1.patch
        15 kB
        Liang Xie
      3. HBASE-8753-trunk-V0.patch
        15 kB
        Honghua Feng
      4. HBASE-8753-0.94-V1.patch
        14 kB
        Honghua Feng
      5. HBASE-8753-0.94-V0.patch
        14 kB
        Honghua Feng
      6. 8753-trunk-v4.txt
        26 kB
        Ted Yu
      7. 8753-trunk-V2.patch
        27 kB
        Ted Yu

        Activity

        Hide
        stack added a comment -

        Integrated into 0.95 and trunk a few days ago.

        Show
        stack added a comment - Integrated into 0.95 and trunk a few days ago.
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #604 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/604/)
        HBASE-8753 Provide new delete flag which can delete all cells under a column-family which have designated timestamp (Honghua) (Revision 1500780)

        Result = FAILURE
        tedyu :
        Files :

        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java
        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
        • /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
        • /hbase/trunk/hbase-protocol/src/main/protobuf/Client.proto
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #604 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/604/ ) HBASE-8753 Provide new delete flag which can delete all cells under a column-family which have designated timestamp (Honghua) (Revision 1500780) Result = FAILURE tedyu : Files : /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java /hbase/trunk/hbase-protocol/src/main/protobuf/Client.proto /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
        Hide
        Hudson added a comment -

        Integrated in hbase-0.95-on-hadoop2 #170 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/170/)
        HBASE-8753 Provide new delete flag which can delete all cells under a column-family which have designated timestamp (Honghua) (Revision 1500779)

        Result = FAILURE
        tedyu :
        Files :

        • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java
        • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        • /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
        • /hbase/branches/0.95/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
        • /hbase/branches/0.95/hbase-protocol/src/main/protobuf/Client.proto
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
        • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
        • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
        Show
        Hudson added a comment - Integrated in hbase-0.95-on-hadoop2 #170 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/170/ ) HBASE-8753 Provide new delete flag which can delete all cells under a column-family which have designated timestamp (Honghua) (Revision 1500779) Result = FAILURE tedyu : Files : /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/branches/0.95/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java /hbase/branches/0.95/hbase-protocol/src/main/protobuf/Client.proto /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #4223 (See https://builds.apache.org/job/HBase-TRUNK/4223/)
        HBASE-8753 Provide new delete flag which can delete all cells under a column-family which have designated timestamp (Honghua) (Revision 1500780)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java
        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
        • /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
        • /hbase/trunk/hbase-protocol/src/main/protobuf/Client.proto
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #4223 (See https://builds.apache.org/job/HBase-TRUNK/4223/ ) HBASE-8753 Provide new delete flag which can delete all cells under a column-family which have designated timestamp (Honghua) (Revision 1500780) Result = SUCCESS tedyu : Files : /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java /hbase/trunk/hbase-protocol/src/main/protobuf/Client.proto /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
        Hide
        Hudson added a comment -

        Integrated in hbase-0.95 #296 (See https://builds.apache.org/job/hbase-0.95/296/)
        HBASE-8753 Provide new delete flag which can delete all cells under a column-family which have designated timestamp (Honghua) (Revision 1500779)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java
        • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        • /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
        • /hbase/branches/0.95/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
        • /hbase/branches/0.95/hbase-protocol/src/main/protobuf/Client.proto
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
        • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
        • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
        • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
        Show
        Hudson added a comment - Integrated in hbase-0.95 #296 (See https://builds.apache.org/job/hbase-0.95/296/ ) HBASE-8753 Provide new delete flag which can delete all cells under a column-family which have designated timestamp (Honghua) (Revision 1500779) Result = SUCCESS tedyu : Files : /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/branches/0.95/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java /hbase/branches/0.95/hbase-protocol/src/main/protobuf/Client.proto /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
        Hide
        Ted Yu added a comment -

        Integrated to 0.95 and trunk.

        Thanks for the patch, Honghua.

        Thanks for the review, Lars.

        Show
        Ted Yu added a comment - Integrated to 0.95 and trunk. Thanks for the patch, Honghua. Thanks for the review, Lars.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12591210/8753-trunk-v4.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 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

        -1 lineLengths. The patch introduces lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

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

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12591210/8753-trunk-v4.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 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6238//console This message is automatically generated.
        Hide
        Honghua Feng added a comment -

        Thanks Ted Yu and Lars Hofhansl

        Show
        Honghua Feng added a comment - Thanks Ted Yu and Lars Hofhansl
        Hide
        Ted Yu added a comment -

        Patch v4 restores changes to Client.proto

        Show
        Ted Yu added a comment - Patch v4 restores changes to Client.proto
        Hide
        Lars Hofhansl added a comment -

        Honghua Feng

        Your comment #2 is covered by this line of code: if (!hasFamilyStamp || timestamp > familyStamp) {

        Yes, you are right.

        +1 on patch. The perf/memory issue only comes into play when family version markers are actually used.

        Since we're introducing a new byte code for the family version markers there should be not client/server compatibility issues unless this feature is actually used with an old server. So if you want this in 0.94, I'd be OK with that as well.

        Show
        Lars Hofhansl added a comment - Honghua Feng Your comment #2 is covered by this line of code: if (!hasFamilyStamp || timestamp > familyStamp) { Yes, you are right. +1 on patch. The perf/memory issue only comes into play when family version markers are actually used. Since we're introducing a new byte code for the family version markers there should be not client/server compatibility issues unless this feature is actually used with an old server. So if you want this in 0.94, I'd be OK with that as well.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12591188/HBASE-8753-trunk-V3.patch
        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 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

        +1 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

        -1 core tests. The patch failed these unit tests:

        -1 core zombie tests. There are 2 zombie test(s): at org.apache.hadoop.hbase.client.TestFromClientSide.testDeleteFamilyVersion(TestFromClientSide.java:1746)
        at org.apache.hadoop.hbase.client.TestFromClientSide.testDeleteFamilyVersion(TestFromClientSide.java:1746)

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12591188/HBASE-8753-trunk-V3.patch 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 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: -1 core zombie tests . There are 2 zombie test(s): at org.apache.hadoop.hbase.client.TestFromClientSide.testDeleteFamilyVersion(TestFromClientSide.java:1746) at org.apache.hadoop.hbase.client.TestFromClientSide.testDeleteFamilyVersion(TestFromClientSide.java:1746) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6236//console This message is automatically generated.
        Hide
        Honghua Feng added a comment -

        Lars Hofhansl Your comment #2 is covered by this line of code:

        if (!hasFamilyStamp || timestamp > familyStamp) {

        Show
        Honghua Feng added a comment - Lars Hofhansl Your comment #2 is covered by this line of code: if (!hasFamilyStamp || timestamp > familyStamp) {
        Hide
        Honghua Feng added a comment -

        Lars Hofhansl: Your #1/#2 comments are good.

        For #3, your concern is correct. And since the ScanDeleteTracker will be reset/cleared (all delete markers) after a row is done, the number of family version marker that need to track at any time during scan is all family version markers that are put to a specific row, not for all rows and not accumulated throughout the whole scan process. Though in theory it's possible that client put arbitrarily number of family version markers to a specific row/cf, but in practice this number is expected to be equal to or less than the number of different versions(timestamps) of all cells a specific row/cf contains.

        Show
        Honghua Feng added a comment - Lars Hofhansl : Your #1/#2 comments are good. For #3, your concern is correct. And since the ScanDeleteTracker will be reset/cleared (all delete markers) after a row is done, the number of family version marker that need to track at any time during scan is all family version markers that are put to a specific row, not for all rows and not accumulated throughout the whole scan process. Though in theory it's possible that client put arbitrarily number of family version markers to a specific row/cf, but in practice this number is expected to be equal to or less than the number of different versions(timestamps) of all cells a specific row/cf contains.
        Hide
        Lars Hofhansl added a comment -

        Looked at v2. Three comments:
        1.

        +      } else if (type == KeyValue.Type.DeleteFamilyVersion.getCode()) {
        +        if (familyVersionStamps.isEmpty()) {
        +          familyVersionStamps.add(timestamp);
        +        } else {
        +          long minTimeStamp= familyVersionStamps.first();
        +          assert timestamp <= minTimeStamp : "deleteFamilyStamp " + minTimeStamp+
        +            " followed by a bigger one " + timestamp;
        +
        +          // remove duplication(ignore deleteFamilyVersion with same timestamp)
        +          if (timestamp < minTimeStamp) {
        +            familyVersionStamps.add(timestamp);
        +          }
        +        }
        +        return;
        

        This all seems overkill just to check the correct sort order. Can just do

        +      } else if (type == KeyValue.Type.DeleteFamilyVersion.getCode()) {
        +        familyVersionStamps.add(timestamp);
        +        return;
        

        2.
        Also, if there is a normal family delete marker with a timestamp newer than the family version marker, we do not need to store the version delete marker at all (as the row is already targeted for delete).

        3.
        Lastly, this is the only delete marker type for which multiple ones need to be kept in memory during a scan... There can never be more than one family, column, or version delete marker that need to be tracked, but for the family version marker we need to potentially track arbitrarily many. That is a concern.

        Show
        Lars Hofhansl added a comment - Looked at v2. Three comments: 1. + } else if (type == KeyValue.Type.DeleteFamilyVersion.getCode()) { + if (familyVersionStamps.isEmpty()) { + familyVersionStamps.add(timestamp); + } else { + long minTimeStamp= familyVersionStamps.first(); + assert timestamp <= minTimeStamp : "deleteFamilyStamp " + minTimeStamp+ + " followed by a bigger one " + timestamp; + + // remove duplication(ignore deleteFamilyVersion with same timestamp) + if (timestamp < minTimeStamp) { + familyVersionStamps.add(timestamp); + } + } + return ; This all seems overkill just to check the correct sort order. Can just do + } else if (type == KeyValue.Type.DeleteFamilyVersion.getCode()) { + familyVersionStamps.add(timestamp); + return ; 2. Also, if there is a normal family delete marker with a timestamp newer than the family version marker, we do not need to store the version delete marker at all (as the row is already targeted for delete). 3. Lastly, this is the only delete marker type for which multiple ones need to be kept in memory during a scan... There can never be more than one family, column, or version delete marker that need to be tracked, but for the family version marker we need to potentially track arbitrarily many. That is a concern.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12591012/8753-trunk-V2.patch
        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 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

        -1 lineLengths. The patch introduces lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

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

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12591012/8753-trunk-V2.patch 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 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6223//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        TestFromClientSide hung because there was missing protobuf translation in ProtobufUtil

        TestFromClientSide passes for patch v2.

        Show
        Ted Yu added a comment - TestFromClientSide hung because there was missing protobuf translation in ProtobufUtil TestFromClientSide passes for patch v2.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12590771/HBASE-8753-trunk-V1.patch
        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 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

        +1 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat

        -1 core zombie tests. There are 2 zombie test(s): at org.apache.hadoop.hbase.client.TestFromClientSide.testDeleteFamilyVersion(TestFromClientSide.java:1745)
        at org.apache.hadoop.hbase.client.TestFromClientSide.testDeleteFamilyVersion(TestFromClientSide.java:1745)

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12590771/HBASE-8753-trunk-V1.patch 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 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat -1 core zombie tests . There are 2 zombie test(s): at org.apache.hadoop.hbase.client.TestFromClientSide.testDeleteFamilyVersion(TestFromClientSide.java:1745) at org.apache.hadoop.hbase.client.TestFromClientSide.testDeleteFamilyVersion(TestFromClientSide.java:1745) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6211//console This message is automatically generated.
        Hide
        Liang Xie added a comment -

        I tried to assign this jira to my colleague Honghua Feng, but failed with: "User fenghh cannot be assigned issues.", could any guys help me? thanks

        Show
        Liang Xie added a comment - I tried to assign this jira to my colleague Honghua Feng , but failed with: "User fenghh cannot be assigned issues.", could any guys help me? thanks
        Hide
        Liang Xie added a comment -

        Attached v1 changes the new familyVersionStamps from list to set, since we observed it'll degrade perf sometimes, especially minor compaction. After this change, on one of our real product clusters, both the small compaction queue size and numStoreFiles reduced to normal.

        Show
        Liang Xie added a comment - Attached v1 changes the new familyVersionStamps from list to set, since we observed it'll degrade perf sometimes, especially minor compaction. After this change, on one of our real product clusters, both the small compaction queue size and numStoreFiles reduced to normal.
        Hide
        Honghua Feng added a comment -

        By design, DeleteFamilyVersion KV's qualifier is null, hence its comparison to any qualifier must be "== 0" or "< 0", so execution can never enter the branch of "throw new IllegalStateException("isDelete failed: deleteBuffer=" when an old RS handles a DeleteFamilyVersion. (DeleteFamilyVersion will be mis-explained as Delete type with qualifier = null)

        And I'll do the backwards-compatibility test as you suggest.

        Show
        Honghua Feng added a comment - By design, DeleteFamilyVersion KV's qualifier is null, hence its comparison to any qualifier must be "== 0" or "< 0", so execution can never enter the branch of "throw new IllegalStateException("isDelete failed: deleteBuffer=" when an old RS handles a DeleteFamilyVersion. (DeleteFamilyVersion will be mis-explained as Delete type with qualifier = null) And I'll do the backwards-compatibility test as you suggest.
        Hide
        Lars Hofhansl added a comment -

        May you not run into this case in isDeleted:

        int ret = Bytes.compareTo(deleteBuffer, deleteOffset, deleteLength,
             buffer, qualifierOffset, qualifierLength);
        if (ret == 0) {
          ...
          } else if(ret < 0){
            ...
          } else {
            throw new IllegalStateException("isDelete failed: deleteBuffer="
          ...
        

        In any case, we should just test it: Write an HFile with a new RS, start an old RS, scan that file, check that it works fine.

        Show
        Lars Hofhansl added a comment - May you not run into this case in isDeleted: int ret = Bytes.compareTo(deleteBuffer, deleteOffset, deleteLength, buffer, qualifierOffset, qualifierLength); if (ret == 0) { ... } else if (ret < 0){ ... } else { throw new IllegalStateException( "isDelete failed: deleteBuffer=" ... In any case, we should just test it: Write an HFile with a new RS, start an old RS, scan that file, check that it works fine.
        Hide
        Honghua Feng added a comment -

        Lars Hofhansl For the backwards-compatibility, when old RS processes the DeleteFamilyVersion type kv (either written from new client, or the two scenarios you mentioned regarding rolling restart), the DeleteFamilyVersion can enter ScanDeleteTracker, and its only effect it has is when no DeleteColumn for null column with the same timestamp as this DeleteFamilyVersion, this DeleteFamilyVersion can delete the KV (column=null) with the same timestamp (a bit like the Delete(DeleteVersion) with the same timestamp), and no other side-effect.

        In summary: DeleteFamilyVersion masks all the versions with a given timestamp under a CF, and when an old RS receives it(written from new client, or the two scenarios mentioned regarding rolling restart), the old RS treats it like it's a Delete(DeleteVersion) for null column. Nothing else.

        I think this side-effect is acceptable. Your opinion?

        Show
        Honghua Feng added a comment - Lars Hofhansl For the backwards-compatibility, when old RS processes the DeleteFamilyVersion type kv (either written from new client, or the two scenarios you mentioned regarding rolling restart), the DeleteFamilyVersion can enter ScanDeleteTracker, and its only effect it has is when no DeleteColumn for null column with the same timestamp as this DeleteFamilyVersion, this DeleteFamilyVersion can delete the KV (column=null) with the same timestamp (a bit like the Delete(DeleteVersion) with the same timestamp), and no other side-effect. In summary: DeleteFamilyVersion masks all the versions with a given timestamp under a CF, and when an old RS receives it(written from new client, or the two scenarios mentioned regarding rolling restart), the old RS treats it like it's a Delete(DeleteVersion) for null column. Nothing else. I think this side-effect is acceptable. Your opinion?
        Hide
        Lars Hofhansl added a comment -

        OK... Need to make sure there are no rolling restart or other compatibility issues. Can you do that, Honghua Feng?

        Show
        Lars Hofhansl added a comment - OK... Need to make sure there are no rolling restart or other compatibility issues. Can you do that, Honghua Feng ?
        Hide
        Honghua Feng added a comment -

        Lars Hofhansl It would be better to have it in 0.94. Thanks.

        Show
        Honghua Feng added a comment - Lars Hofhansl It would be better to have it in 0.94. Thanks.
        Hide
        Lars Hofhansl added a comment -

        You want this in 0.94, Honghua Feng? Or is 0.95+ good enough?

        Show
        Lars Hofhansl added a comment - You want this in 0.94, Honghua Feng ? Or is 0.95+ good enough?
        Hide
        Lars Hofhansl added a comment -

        Will this break backwards-compatibility during rolling restarts? (because of the new KV type)
        ===> Yes, old RS bits will ignore DeleteFamilyVersion type KV written by new client.

        This will only be an issue in two scenarios:

        1. An old RS replaying logs from a new RS that crashed during a rolling upgrade.
        2. A new HFile loaded by an old RS. (rebalance should be disabled during rolling restart, so that should also only happen when a RS crashes during rolling restart).

        It seems this is acceptable.
        It is not immediately clear that won't be other issues during a rolling restart. Comments?

        Otherwise +1 on patch.

        Thanks for digging in on this, Feng.

        Show
        Lars Hofhansl added a comment - Will this break backwards-compatibility during rolling restarts? (because of the new KV type) ===> Yes, old RS bits will ignore DeleteFamilyVersion type KV written by new client. This will only be an issue in two scenarios: An old RS replaying logs from a new RS that crashed during a rolling upgrade. A new HFile loaded by an old RS. (rebalance should be disabled during rolling restart, so that should also only happen when a RS crashes during rolling restart). It seems this is acceptable. It is not immediately clear that won't be other issues during a rolling restart. Comments? Otherwise +1 on patch. Thanks for digging in on this, Feng.
        Hide
        Honghua Feng added a comment -

        Answer some questions as below:

        [chunhui] Could reuse the tag of VERSION_DELETED (other than introducing a new FAMILY_VERSION_DELETED)?

        ===> Per my understanding, the result tag is indicating the kv is masked by which type of delete. Introducing a new tag here makes sense since it distinguishes itself from VERSION_DELETED which means 'masked by a DeleteColumn'.

        [chunhui] Maybe we need a better name instead of DeleteFamilyVersion

        ===> DeleteFamilyVersion is the best name I can come up with till now. We can keep it until you recommend a better one.

        [stack] Are there changes to KV missing?

        ===> No. Let me know if you feel something missing/wrong with KV. Thanks

        [Lars Hofhansl] :
        Do you want to only delete columns of a specific version, or columns older than a specific version?
        ===> What this patch does is the former: only delete columns of a specific version (without providing their column-names)

        In your use-case, do you ever want to keep version X but target target X+1 for delete?
        ===> Yes, this is exactly the effect this patch aims for

        Will this break backwards-compatibility during rolling restarts? (because of the new KV type)
        ===> Yes, old RS bits will ignore DeleteFamilyVersion type KV written by new client.

        Show
        Honghua Feng added a comment - Answer some questions as below: [chunhui] Could reuse the tag of VERSION_DELETED (other than introducing a new FAMILY_VERSION_DELETED)? ===> Per my understanding, the result tag is indicating the kv is masked by which type of delete. Introducing a new tag here makes sense since it distinguishes itself from VERSION_DELETED which means 'masked by a DeleteColumn'. [chunhui] Maybe we need a better name instead of DeleteFamilyVersion ===> DeleteFamilyVersion is the best name I can come up with till now. We can keep it until you recommend a better one. [stack] Are there changes to KV missing? ===> No. Let me know if you feel something missing/wrong with KV. Thanks [ Lars Hofhansl ] : Do you want to only delete columns of a specific version, or columns older than a specific version? ===> What this patch does is the former: only delete columns of a specific version (without providing their column-names) In your use-case, do you ever want to keep version X but target target X+1 for delete? ===> Yes, this is exactly the effect this patch aims for Will this break backwards-compatibility during rolling restarts? (because of the new KV type) ===> Yes, old RS bits will ignore DeleteFamilyVersion type KV written by new client.
        Hide
        Honghua Feng added a comment -

        chunhui shen/stack patch for trunk attached, thanks in advance for the review

        Show
        Honghua Feng added a comment - chunhui shen / stack patch for trunk attached, thanks in advance for the review
        Hide
        Lars Hofhansl added a comment -

        I should mention that in the dichotomy above it does make sense to add a version specific family delete marker.

        Show
        Lars Hofhansl added a comment - I should mention that in the dichotomy above it does make sense to add a version specific family delete marker.
        Hide
        Lars Hofhansl added a comment -

        Do you want to only delete columns of a specific version, or columns older than a specific version? The latter is currently possible by a family delete with a timestamp.
        In your use-case, do you ever want to keep version X but target target X+1 for delete?

        Currently we have:

        1. version delete: target a specific version of a specific column for delete
        2. column delete: target all version (optionally older than a ts) of a column for delete
        3. family delete: target all version (optionally older than a ts) of all columns of a family for delete

        Will this break backwards-compatibility during rolling restarts? (because of the new KV type)

        Show
        Lars Hofhansl added a comment - Do you want to only delete columns of a specific version, or columns older than a specific version? The latter is currently possible by a family delete with a timestamp. In your use-case, do you ever want to keep version X but target target X+1 for delete? Currently we have: version delete: target a specific version of a specific column for delete column delete: target all version (optionally older than a ts) of a column for delete family delete: target all version (optionally older than a ts) of all columns of a family for delete Will this break backwards-compatibility during rolling restarts? (because of the new KV type)
        Hide
        stack added a comment -

        Will wait on trunk patch to review. Nice feature. Are there changes to KV missing? Name is fine by me but try to make chunhui shen happy if you can.

        Show
        stack added a comment - Will wait on trunk patch to review. Nice feature. Are there changes to KV missing? Name is fine by me but try to make chunhui shen happy if you can.
        Hide
        Honghua Feng added a comment -

        OK. thanks Ted Yu for the code review.

        "Was deleteFamilyStamp the previous name for this feature ? "

        ==> No. can't come up with a better name for this log then. deleteFamilyVersion has no 'timestamp' meaning, and deleteFamilyVersionStamp is too long, so just remove the 'Version' and remain 'Stamp' to use it to indicate 'timestamp'...

        Show
        Honghua Feng added a comment - OK. thanks Ted Yu for the code review. "Was deleteFamilyStamp the previous name for this feature ? " ==> No. can't come up with a better name for this log then. deleteFamilyVersion has no 'timestamp' meaning, and deleteFamilyVersionStamp is too long, so just remove the 'Version' and remain 'Stamp' to use it to indicate 'timestamp'...
        Hide
        Ted Yu added a comment -

        Please move the new test to TestFromClientSide3.java - TestFromClientSide is getting too big:

        -rw-r--r--  1 tyu  staff  177212 May 23 09:49 src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
        -rw-r--r--  1 tyu  staff    9819 Mar 29 12:46 src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java
        

        At the end of each test, please close ht and admin.

        +          assert timestamp <= lastStamp : "deleteFamilyStamp " + lastStamp +
        

        Was deleteFamilyStamp the previous name for this feature ?

        Show
        Ted Yu added a comment - Please move the new test to TestFromClientSide3.java - TestFromClientSide is getting too big: -rw-r--r-- 1 tyu staff 177212 May 23 09:49 src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java -rw-r--r-- 1 tyu staff 9819 Mar 29 12:46 src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java At the end of each test, please close ht and admin. + assert timestamp <= lastStamp : "deleteFamilyStamp " + lastStamp + Was deleteFamilyStamp the previous name for this feature ?
        Hide
        Honghua Feng added a comment -

        Thanks chunhui for the review, I'll make a patch for trunk soon

        For the name, do you have a better alternative?

        Show
        Honghua Feng added a comment - Thanks chunhui for the review, I'll make a patch for trunk soon For the name, do you have a better alternative?
        Hide
        chunhui shen added a comment -

        Patch makes sense.

                 case VERSION_DELETED:
        +        case FAMILY_VERSION_DELETED:
        

        Could reuse the tag of VERSION_DELETED?

           /**
        +   * @return True if this KV is a delete family type.
        +   */
        +  public boolean isDeleteFamilyVersion() {
        +    return getType() == Type.DeleteFamilyVersion.getCode();
        +  }
        

        The annotation is not matched.

        Maybe we need a better name instead of DeleteFamilyVersion.

        Could you make a patch for trunk to run HadoopQA?

        Show
        chunhui shen added a comment - Patch makes sense. case VERSION_DELETED: + case FAMILY_VERSION_DELETED: Could reuse the tag of VERSION_DELETED? /** + * @ return True if this KV is a delete family type. + */ + public boolean isDeleteFamilyVersion() { + return getType() == Type.DeleteFamilyVersion.getCode(); + } The annotation is not matched. Maybe we need a better name instead of DeleteFamilyVersion. Could you make a patch for trunk to run HadoopQA?
        Hide
        Honghua Feng added a comment -
        Show
        Honghua Feng added a comment - patch HBASE-8753 -0.94-V0.patch is based on http://svn.apache.org/repos/asf/hbase/branches/0.94

          People

          • Assignee:
            Honghua Feng
            Reporter:
            Honghua Feng
          • Votes:
            1 Vote for this issue
            Watchers:
            11 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development