Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-10885

Support visibility expressions on Deletes

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.98.1
    • Fix Version/s: 0.99.0, 0.98.4, 2.0.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      Deletes can be specified with Cell Visibility as done for puts.
      Cells covered by the delete is found by doing pattern matching.
      A deleteFamily issued for row1, f1 with Cell Visibility (A & B) would delete only those cells of row1 under family f1 which has cell visibility A&B or B&A. A delete without any cell visibility would only delete those cells that does not have any cell visibility.
      In case of delete specific column with latest version only the latest cell with the specified cell visibility will be covered by the delete marker. In case there is no such cell with the specified cell visibility then no cell gets deleted.
      Show
      Deletes can be specified with Cell Visibility as done for puts. Cells covered by the delete is found by doing pattern matching. A deleteFamily issued for row1, f1 with Cell Visibility (A & B) would delete only those cells of row1 under family f1 which has cell visibility A&B or B&A. A delete without any cell visibility would only delete those cells that does not have any cell visibility. In case of delete specific column with latest version only the latest cell with the specified cell visibility will be covered by the delete marker. In case there is no such cell with the specified cell visibility then no cell gets deleted.

      Description

      Accumulo can specify visibility expressions for delete markers. During compaction the cells covered by the tombstone are determined in part by matching the visibility expression. This is useful for the use case of data set coalescing, where entries from multiple data sets carrying different labels are combined into one common large table. Later, a subset of entries can be conveniently removed using visibility expressions.

      Currently doing the same in HBase would only be possible with a custom coprocessor. Otherwise, a Delete will affect all cells covered by the tombstone regardless of any visibility expression scoping. This is correct behavior in that no data spill is possible, but certainly could be surprising, and is only meant to be transitional. We decided not to support visibility expressions on Deletes to control the complexity of the initial implementation.

      1. 10885-org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes-output.txt
        3.16 MB
        Ted Yu
      2. HBASE-10885_0.98_1.patch
        171 kB
        ramkrishna.s.vasudevan
      3. HBASE-10885_1.patch
        89 kB
        ramkrishna.s.vasudevan
      4. HBASE-10885_2.patch
        99 kB
        ramkrishna.s.vasudevan
      5. HBASE-10885_branch_1.patch
        171 kB
        ramkrishna.s.vasudevan
      6. HBASE-10885_new_tag_type_1.patch
        112 kB
        ramkrishna.s.vasudevan
      7. HBASE-10885_new_tag_type_2.patch
        112 kB
        ramkrishna.s.vasudevan
      8. HBASE-10885_v1.patch
        141 kB
        ramkrishna.s.vasudevan
      9. HBASE-10885_v12.patch
        148 kB
        ramkrishna.s.vasudevan
      10. HBASE-10885_v12.patch
        148 kB
        ramkrishna.s.vasudevan
      11. HBASE-10885_v13.patch
        153 kB
        ramkrishna.s.vasudevan
      12. HBASE-10885_v15.patch
        159 kB
        ramkrishna.s.vasudevan
      13. HBASE-10885_v17.patch
        171 kB
        Anoop Sam John
      14. HBASE-10885_v17.patch
        171 kB
        ramkrishna.s.vasudevan
      15. HBASE-10885_v2.patch
        113 kB
        stack
      16. HBASE-10885_v2.patch
        113 kB
        ramkrishna.s.vasudevan
      17. HBASE-10885_v2.patch
        113 kB
        ramkrishna.s.vasudevan
      18. HBASE-10885_v3.patch
        114 kB
        ramkrishna.s.vasudevan
      19. HBASE-10885_v4.patch
        130 kB
        ramkrishna.s.vasudevan
      20. HBASE-10885_v5.patch
        130 kB
        ramkrishna.s.vasudevan
      21. HBASE-10885_v7.patch
        128 kB
        ramkrishna.s.vasudevan
      22. HBASE-10885_v8.patch
        129 kB
        ramkrishna.s.vasudevan
      23. HBASE-10885_v9.patch
        138 kB
        ramkrishna.s.vasudevan

        Issue Links

          Activity

          Hide
          apurtell Andrew Purtell added a comment -

          How we handle deletes in the AccessController is to only allow the delete if it has covering permission. By that I mean CF ACLs and any ACLs in cells that would be covered by the tombstone not already covered by a tombstone must grant the subject access. If any do not, the delete is denied. We run an internal RegionScanner to enumerate the cells that would be affected by the operation.

          One way to do this for the VisibilityController is we could similarly run an internal RegionScanner with the parameters of the submitted Delete op, filter cells by effective visibility, and issue deletes scoped only to those cells visible to the subject. This would be easier than trying to hook compaction scanners and evaluating visibility expression tags there, because we will have the effective label set for the user in the RPC context, not at compaction time.

          Show
          apurtell Andrew Purtell added a comment - How we handle deletes in the AccessController is to only allow the delete if it has covering permission. By that I mean CF ACLs and any ACLs in cells that would be covered by the tombstone not already covered by a tombstone must grant the subject access. If any do not, the delete is denied. We run an internal RegionScanner to enumerate the cells that would be affected by the operation. One way to do this for the VisibilityController is we could similarly run an internal RegionScanner with the parameters of the submitted Delete op, filter cells by effective visibility, and issue deletes scoped only to those cells visible to the subject. This would be easier than trying to hook compaction scanners and evaluating visibility expression tags there, because we will have the effective label set for the user in the RPC context, not at compaction time.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Delete.setCellVisibility() should be supported now. And these labels passed here will be only a list of labels and not visibility expressions like A|B!C?
          Which means if an existing put had A&B now the delete should pass A&B or it could be A,B like how we pass for Authorization?

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Delete.setCellVisibility() should be supported now. And these labels passed here will be only a list of labels and not visibility expressions like A|B!C? Which means if an existing put had A&B now the delete should pass A&B or it could be A,B like how we pass for Authorization?
          Hide
          apurtell Andrew Purtell added a comment - - edited

          Delete.setCellVisibility() should be supported now.

          Yes.

          And these labels passed here will be only a list of labels and not visibility expressions like A|B!C?

          No. Deletes should support visibility expressions just like Put, etc. The supplied visibility expression is then associated with the delete marker(s).

          Actually, scratch what I said above in the first comment. We can store the delete marker as a cell with a visibility expression tag and do the work later, by hooking the compaction scanner. We would check for visibility expressions in tags on delete markers at compaction time. If we find one, then we have to filter only the cells covered by the tombstone that have a matching expression.

          If we are not storing visibility expression terminals (LeafExpressionNodes) in sorted order by ordinal we probably should consider it. (I don't think we are.) Because e.g. A|B == B|A. It would be most efficient if we can simply do byte comparison of serialized visibility expressions on the delete marker and any found while enumerating cells covered by it.

          If a delete marker has a visibility expression, then we only apply it to cells with matching visibility expressions. If a cell has no visibility tag then it does not match. (A|B != nil)

          Should we check that the supplied expression does not exceed the maximal authorization set for the user submitting the Delete in the preDelete hook? In other words, should we we allow a user only granted authorization A to submit a delete with visibility expression A|B? We should not, in my opinion. Recommend we answer this question for other op types on another JIRA, should there be any.

          Show
          apurtell Andrew Purtell added a comment - - edited Delete.setCellVisibility() should be supported now. Yes. And these labels passed here will be only a list of labels and not visibility expressions like A|B!C? No. Deletes should support visibility expressions just like Put, etc. The supplied visibility expression is then associated with the delete marker(s). Actually, scratch what I said above in the first comment. We can store the delete marker as a cell with a visibility expression tag and do the work later, by hooking the compaction scanner. We would check for visibility expressions in tags on delete markers at compaction time. If we find one, then we have to filter only the cells covered by the tombstone that have a matching expression. If we are not storing visibility expression terminals (LeafExpressionNodes) in sorted order by ordinal we probably should consider it. (I don't think we are.) Because e.g. A|B == B|A. It would be most efficient if we can simply do byte comparison of serialized visibility expressions on the delete marker and any found while enumerating cells covered by it. If a delete marker has a visibility expression, then we only apply it to cells with matching visibility expressions. If a cell has no visibility tag then it does not match. (A|B != nil) Should we check that the supplied expression does not exceed the maximal authorization set for the user submitting the Delete in the preDelete hook? In other words, should we we allow a user only granted authorization A to submit a delete with visibility expression A|B? We should not, in my opinion. Recommend we answer this question for other op types on another JIRA, should there be any.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Doing like what ACL does may be easier because we could see which subject issues the delete. If a super user/admin that makes the put does the delete then we can just allow the delete to happen.
          So assume we have user A with labels CONFIDENTIAL associated with him.
          And there is a Put already done with row1, cf1, q1, CONFIDENTIAL and another version
          row1,cf1,q1, TOPSECRET&CONFIDENTIAL
          If suppose user A tries to issue a delete ( not using Cellvisibility) - by using the labels associated with User A we could find that we will not be able to delete this row. Because one of the versions has additional labels associated with it.
          But allowing the delete to be added and then verifying the visibility expression during compaction time - then we may have to handle the logic of deletes and corresponding puts in the compaction scanner hook that we create. Also what about the scans that happens? May be there is a matching delete for a put with exactly same visibility expression, ideally the scan should not return that KV.
          Apart from this with the ACL delete handling case, some doubts regarding the handling of the deleteColumn() - which deletes only the latest version. But with the current implementation even though the current version allows the delete with valid permissions for the user, because there is an older version with lesser permission we deny the delete. Is that valid? same applies with deleteFamily() also.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Doing like what ACL does may be easier because we could see which subject issues the delete. If a super user/admin that makes the put does the delete then we can just allow the delete to happen. So assume we have user A with labels CONFIDENTIAL associated with him. And there is a Put already done with row1, cf1, q1, CONFIDENTIAL and another version row1,cf1,q1, TOPSECRET&CONFIDENTIAL If suppose user A tries to issue a delete ( not using Cellvisibility) - by using the labels associated with User A we could find that we will not be able to delete this row. Because one of the versions has additional labels associated with it. But allowing the delete to be added and then verifying the visibility expression during compaction time - then we may have to handle the logic of deletes and corresponding puts in the compaction scanner hook that we create. Also what about the scans that happens? May be there is a matching delete for a put with exactly same visibility expression, ideally the scan should not return that KV. Apart from this with the ACL delete handling case, some doubts regarding the handling of the deleteColumn() - which deletes only the latest version. But with the current implementation even though the current version allows the delete with valid permissions for the user, because there is an older version with lesser permission we deny the delete. Is that valid? same applies with deleteFamily() also.
          Hide
          apurtell Andrew Purtell added a comment - - edited

          Doing like what ACL does may be easier because we could see which subject issues the delete. If a super user/admin that makes the put does the delete then we can just allow the delete to happen.

          Above I suggest splitting the authorization check and the actual delete handling. Do the authorization check in the preDelete hook because we have the user's effective label set in the RPC context. Do the delete handling in compaction because for the deleteColumn or deleteFamily cases if we convert that delete request to a set of per-cell deletes, this could produce an explosion of tombstones.

          Apart from this with the ACL delete handling case, some doubts regarding the handling of the deleteColumn() - which deletes only the latest version. But with the current implementation even though the current version allows the delete with valid permissions for the user, because there is an older version with lesser permission we deny the delete. Is that valid? same applies with deleteFamily() also.

          Yes, the rule is all visible cells with an ACL must allow the delete, or the delete will be denied. However, we should respect the MAX_VERSIONS of column families as defined in the schema when determining the scope of visibility and so changes are needed for that (HBASE-10899).

          Show
          apurtell Andrew Purtell added a comment - - edited Doing like what ACL does may be easier because we could see which subject issues the delete. If a super user/admin that makes the put does the delete then we can just allow the delete to happen. Above I suggest splitting the authorization check and the actual delete handling. Do the authorization check in the preDelete hook because we have the user's effective label set in the RPC context. Do the delete handling in compaction because for the deleteColumn or deleteFamily cases if we convert that delete request to a set of per-cell deletes, this could produce an explosion of tombstones. Apart from this with the ACL delete handling case, some doubts regarding the handling of the deleteColumn() - which deletes only the latest version. But with the current implementation even though the current version allows the delete with valid permissions for the user, because there is an older version with lesser permission we deny the delete. Is that valid? same applies with deleteFamily() also. Yes, the rule is all visible cells with an ACL must allow the delete, or the delete will be denied. However, we should respect the MAX_VERSIONS of column families as defined in the schema when determining the scope of visibility and so changes are needed for that ( HBASE-10899 ).
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Do the delete handling in compaction because for the deleteColumn or deleteFamily cases if we convert that delete request to a set of per-cell deletes, this could produce an explosion of tombstones.

          +1

          If we are not storing visibility expression terminals (LeafExpressionNodes) in sorted order by ordinal we probably should consider it. (I don't think we are.) Because e.g. A|B == B|A. It would be most efficient if we can simply do byte comparison of serialized visibility expressions on the delete marker and any found while enumerating cells covered by it.

          Yes +1.

          So when we decide at the compaction time by direct byte compare of expressions in Delete kv and other kvs, what abt the old HFile data written using 98.0 or 98.1 versions? This will be like not sorted.
          Also it wont one simple bytes compare. A&B will be stored as one Tag. In case of '|' it will be stored as 2 tags. So we need to consider all these tags and compare all. Just a note before starting the impl.

          Show
          anoop.hbase Anoop Sam John added a comment - Do the delete handling in compaction because for the deleteColumn or deleteFamily cases if we convert that delete request to a set of per-cell deletes, this could produce an explosion of tombstones. +1 If we are not storing visibility expression terminals (LeafExpressionNodes) in sorted order by ordinal we probably should consider it. (I don't think we are.) Because e.g. A|B == B|A. It would be most efficient if we can simply do byte comparison of serialized visibility expressions on the delete marker and any found while enumerating cells covered by it. Yes +1. So when we decide at the compaction time by direct byte compare of expressions in Delete kv and other kvs, what abt the old HFile data written using 98.0 or 98.1 versions? This will be like not sorted. Also it wont one simple bytes compare. A&B will be stored as one Tag. In case of '|' it will be stored as 2 tags. So we need to consider all these tags and compare all. Just a note before starting the impl.
          Hide
          apurtell Andrew Purtell added a comment -

          Also it wont one simple bytes compare. A&B will be stored as one Tag. In case of '|' it will be stored as 2 tags. So we need to consider all these tags and compare all. Just a note before starting the impl.

          Ok thanks Anoop.

          Show
          apurtell Andrew Purtell added a comment - Also it wont one simple bytes compare. A&B will be stored as one Tag. In case of '|' it will be stored as 2 tags. So we need to consider all these tags and compare all. Just a note before starting the impl. Ok thanks Anoop.
          Hide
          apurtell Andrew Purtell added a comment -

          Critical for 0.98.2

          Show
          apurtell Andrew Purtell added a comment - Critical for 0.98.2
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          what abt the old HFile data written using 98.0 or 98.1 versions?

          This is a very much valid concern. Any suggestions?

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - what abt the old HFile data written using 98.0 or 98.1 versions? This is a very much valid concern. Any suggestions?
          Hide
          apurtell Andrew Purtell added a comment -

          what abt the old HFile data written using 98.0 or 98.1 versions?

          This is a very much valid concern

          We don't consider sorting expression terminals by ordinal after all. Right?

          Show
          apurtell Andrew Purtell added a comment - what abt the old HFile data written using 98.0 or 98.1 versions? This is a very much valid concern We don't consider sorting expression terminals by ordinal after all. Right?
          Hide
          apurtell Andrew Purtell added a comment - - edited

          On sorting of terminals or not, a discussion that Ram, Anoop, and I had included this topic and it seems reasonable to change the serialization. I think we should start by splitting out the ad hoc visibility tag serialization in VisibilityController to a separate file. We could put magic bytes in front and test for those, falling back to an expensive comparison of we don't find the magic, otherwise use one optimized for sorted representation. While we are at it we could use protobuf for the new serialization and so the magic preamble would be 'PBUF' I suppose.

          Show
          apurtell Andrew Purtell added a comment - - edited On sorting of terminals or not, a discussion that Ram, Anoop, and I had included this topic and it seems reasonable to change the serialization. I think we should start by splitting out the ad hoc visibility tag serialization in VisibilityController to a separate file. We could put magic bytes in front and test for those, falling back to an expensive comparison of we don't find the magic, otherwise use one optimized for sorted representation. While we are at it we could use protobuf for the new serialization and so the magic preamble would be 'PBUF' I suppose.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          First version for review. Once shell support is committed in the sub-task this patch could be easily tested from shell also.
          Tried out the option of using VisiblityDeleteFilters also, but it has some code duplication required and all the scan should be of type raw (incase of user scan and compaction). So we may have to lose the information if the scan has already been attached with the families and qualifiers that needs to be retrieved.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - First version for review. Once shell support is committed in the sub-task this patch could be easily tested from shell also. Tried out the option of using VisiblityDeleteFilters also, but it has some code duplication required and all the scan should be of type raw (incase of user scan and compaction). So we may have to lose the information if the scan has already been attached with the families and qualifiers that needs to be retrieved.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Found a bug while testing with shells. Will fix and update the patch soon.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Found a bug while testing with shells. Will fix and update the patch soon.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Fixes the bug, when two deleteFamilies are issued , first with a lower timestamp and next with a higher timestamp. The delete tombstone of the higher one should be applied and the visibility expression of the higher should be considered and mask based on that rather than considering the visibility expression of the delete family with lower timestamp.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Fixes the bug, when two deleteFamilies are issued , first with a lower timestamp and next with a higher timestamp. The delete tombstone of the higher one should be applied and the visibility expression of the higher should be considered and mask based on that rather than considering the visibility expression of the delete family with lower timestamp.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -
          +   * Add the specified KeyValue to this Delete operation.  Operation assumes that
          +   * the passed Cell is immutable and its backing array will not be modified
          

          KeyValue appears in first line and Cell in second. Better make the terms consistent.

          +    if (!KeyValue.isDelete(kv.getTypeByte())) {
          +      throw new WrongRowIOException("The cell " + kv.toString() + "is not of Delete type");
          

          Would IllegalArgumentException be more appropriate above ?

          +  public static boolean isDeleteColumn(final Cell cell) {
          +    return cell.getTypeByte() == Type.Delete.getCode();
          +  }
          

          In KeyValue, the above comparison is in a method called isDeleteType().

          In VisibilityController#preDelete():

          +    for (Map.Entry<byte[], List<Cell>> e : delete.getFamilyCellMap().entrySet()) {
          +      byte[] family = e.getKey();
          +      List<Cell> cells = e.getValue();
          +      Map<byte[], Integer> kvCount = new TreeMap<byte[], Integer>(Bytes.BYTES_COMPARATOR);
          

          The tree map can be reused across iterations, right ?

          +      if (!matchFound && isDeleteLatest) {
          +/*        throw new RuntimeException("No matching cell with given visibility expression found "
          +            + delete);*/
          

          What's your plan for the above comment ?

          +public class VisibilityScanDeleteTracker extends ScanDeleteTracker {
          

          Add annotation for audience.

          Putting patch on review board would be nice.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - + * Add the specified KeyValue to this Delete operation. Operation assumes that + * the passed Cell is immutable and its backing array will not be modified KeyValue appears in first line and Cell in second. Better make the terms consistent. + if (!KeyValue.isDelete(kv.getTypeByte())) { + throw new WrongRowIOException( "The cell " + kv.toString() + "is not of Delete type" ); Would IllegalArgumentException be more appropriate above ? + public static boolean isDeleteColumn( final Cell cell) { + return cell.getTypeByte() == Type.Delete.getCode(); + } In KeyValue, the above comparison is in a method called isDeleteType(). In VisibilityController#preDelete(): + for (Map.Entry< byte [], List<Cell>> e : delete.getFamilyCellMap().entrySet()) { + byte [] family = e.getKey(); + List<Cell> cells = e.getValue(); + Map< byte [], Integer > kvCount = new TreeMap< byte [], Integer >(Bytes.BYTES_COMPARATOR); The tree map can be reused across iterations, right ? + if (!matchFound && isDeleteLatest) { +/* throw new RuntimeException( "No matching cell with given visibility expression found " + + delete);*/ What's your plan for the above comment ? + public class VisibilityScanDeleteTracker extends ScanDeleteTracker { Add annotation for audience. Putting patch on review board would be nice.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12642441/HBASE-10885_2.patch
          against trunk revision .
          ATTACHMENT ID: 12642441

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

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

          +1 javadoc. The javadoc tool did not generate any 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.security.visibility.TestVisibilityLabels
          org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsOpWithDifferentUsersNoACL
          org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDistributedLogReplay

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12642441/HBASE-10885_2.patch against trunk revision . ATTACHMENT ID: 12642441 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javadoc . The javadoc tool did not generate any 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.security.visibility.TestVisibilityLabels org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsOpWithDifferentUsersNoACL org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDistributedLogReplay Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9426//console This message is automatically generated.
          Hide
          apurtell Andrew Purtell added a comment -

          Will review patch after we have a clean run of o.a.h.h.security.visibility.* tests.

          Show
          apurtell Andrew Purtell added a comment - Will review patch after we have a clean run of o.a.h.h.security.visibility.* tests.
          Hide
          apurtell Andrew Purtell added a comment -

          Rolling the 0.98.2 RC0 tommorrow. This looks almost ready but not quite. No problem, making a blocker for 0.98.3, we can get in in there for sure. The integration test for deletes-with-visibility is slotted for 0.98.3 anyhow.

          Show
          apurtell Andrew Purtell added a comment - Rolling the 0.98.2 RC0 tommorrow. This looks almost ready but not quite. No problem, making a blocker for 0.98.3, we can get in in there for sure. The integration test for deletes-with-visibility is slotted for 0.98.3 anyhow.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Corrected the patch and now all cases passes.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Corrected the patch and now all cases passes.
          Show
          ram_krish ramkrishna.s.vasudevan added a comment - https://reviews.apache.org/r/20887/
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          What should be the behaviour in this case

          put 't1','r1','f1:c1','100',101,{VISIBILITY=>'PRIVATE|SECRET'}
          put 't1','r1','f1:c1','100',102,{VISIBILITY=>'PRIVATE'}
          put 't1','r1','f1:c1','100',103,{VISIBILITY=>'(SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL)'}
          put 't1','r1','f1:c1','100',104,{VISIBILITY=>'(SECRET&TOPSECRET)'}
          put 't1','r1','f1:c1','100',105,{VISIBILITY=>'(SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL)'}
          

          Now if we issue a deleteColumn with specific timestamp for ts = 104 and visibility=(SECRET&TOPSECRET). Then cell with ts=104 gets deleted.
          Now if there is a deleteFamily issued with ts = 103 and visibility = (SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL) now should we delete cells from 103 to 101 and also the cell at 104 caused by the first deleteColumn?
          In case of cells without visibility the behaviour is finally only the cell with 105 is available and others are deleted.
          In such cases should we need to track the visibility expression per ts. It is possible but in case of cases where the number of versions are more then this tracking would be a bit of an overhead. Thoughts? All these cases happens when things are in memstore and no flushes/compactions.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - What should be the behaviour in this case put 't1','r1','f1:c1','100',101,{VISIBILITY=>'PRIVATE|SECRET'} put 't1','r1','f1:c1','100',102,{VISIBILITY=>'PRIVATE'} put 't1','r1','f1:c1','100',103,{VISIBILITY=>'(SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL)'} put 't1','r1','f1:c1','100',104,{VISIBILITY=>'(SECRET&TOPSECRET)'} put 't1','r1','f1:c1','100',105,{VISIBILITY=>'(SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL)'} Now if we issue a deleteColumn with specific timestamp for ts = 104 and visibility=(SECRET&TOPSECRET). Then cell with ts=104 gets deleted. Now if there is a deleteFamily issued with ts = 103 and visibility = (SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL) now should we delete cells from 103 to 101 and also the cell at 104 caused by the first deleteColumn? In case of cells without visibility the behaviour is finally only the cell with 105 is available and others are deleted. In such cases should we need to track the visibility expression per ts. It is possible but in case of cases where the number of versions are more then this tracking would be a bit of an overhead. Thoughts? All these cases happens when things are in memstore and no flushes/compactions.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Yes as u said , without vis exp the delete will remove all cells except ts=105
          With vis exp in delete, the exact version delete should delete cell ts=104 as the vis exps are matching. For deleteFamily case all cells with ts <=103 are candidates for delete but u can see only one cell with ts=103 is having matching vis exp withh that in the delete cell. Cells with ts 102 and 101 are not having matching vis exps and so should not be getting deleted.

          Show
          anoop.hbase Anoop Sam John added a comment - Yes as u said , without vis exp the delete will remove all cells except ts=105 With vis exp in delete, the exact version delete should delete cell ts=104 as the vis exps are matching. For deleteFamily case all cells with ts <=103 are candidates for delete but u can see only one cell with ts=103 is having matching vis exp withh that in the delete cell. Cells with ts 102 and 101 are not having matching vis exps and so should not be getting deleted.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          In the current case only one gets applied. So need to track the vis expression based on ts.
          One more thing to note here is

          For deleteFamily case all cells with ts <=103 are candidates for delete

          Here because deleteFamily with ts = 103 satisfies the condition I think from that cell onwards all the cells less than 103 should also be covered because it is deleteFamily. If it is deleteFamilyVersion then we need to check for exact match only. But here I think once we find the cell with matching visibility expression the ones below that also should get deleted.
          Which means if a deleteFamily says ts = 103 and assume in this case the ts =101 only matches the vis expression then from 101 on wards all cells less than 101 should be masked.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - In the current case only one gets applied. So need to track the vis expression based on ts. One more thing to note here is For deleteFamily case all cells with ts <=103 are candidates for delete Here because deleteFamily with ts = 103 satisfies the condition I think from that cell onwards all the cells less than 103 should also be covered because it is deleteFamily. If it is deleteFamilyVersion then we need to check for exact match only. But here I think once we find the cell with matching visibility expression the ones below that also should get deleted. Which means if a deleteFamily says ts = 103 and assume in this case the ts =101 only matches the vis expression then from 101 on wards all cells less than 101 should be masked.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Let me change the code to track the visibility expression per ts.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Let me change the code to track the visibility expression per ts.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12644102/HBASE-10885_new_tag_type_1.patch
          against trunk revision .
          ATTACHMENT ID: 12644102

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 findbugs. The patch appears to introduce 2 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 the following lines longer than 100:
          + // Latest_timestamp. In such cases the ts in the delete marker and the masking put will not be same.
          + // and also ensure that the combination of different type of deletes with diff ts would also work fine
          + d.setCellVisibility(new CellVisibility("(" + CONFIDENTIAL + "&" + PRIVATE + ")|(" + TOPSECRET
          + public void testDeleteColumnAndDeleteFamilylSpecificTimeStampWithMulipleVersion() throws Exception {

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.client.TestMultiParallel
          org.apache.hadoop.hbase.regionserver.TestEncryptionKeyRotation

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12644102/HBASE-10885_new_tag_type_1.patch against trunk revision . ATTACHMENT ID: 12644102 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 findbugs . The patch appears to introduce 2 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 the following lines longer than 100: + // Latest_timestamp. In such cases the ts in the delete marker and the masking put will not be same. + // and also ensure that the combination of different type of deletes with diff ts would also work fine + d.setCellVisibility(new CellVisibility("(" + CONFIDENTIAL + "&" + PRIVATE + ")|(" + TOPSECRET + public void testDeleteColumnAndDeleteFamilylSpecificTimeStampWithMulipleVersion() throws Exception { +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.client.TestMultiParallel org.apache.hadoop.hbase.regionserver.TestEncryptionKeyRotation Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9493//console This message is automatically generated.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Left some comments in RB. Review is not over. Will complete it soon.

          Show
          anoop.hbase Anoop Sam John added a comment - Left some comments in RB. Review is not over. Will complete it soon.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12644400/HBASE-10885_new_tag_type_2.patch
          against trunk revision .
          ATTACHMENT ID: 12644400

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

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

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

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

          -1 findbugs. The patch appears to introduce 2 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.master.TestAssignmentManager

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12644400/HBASE-10885_new_tag_type_2.patch against trunk revision . ATTACHMENT ID: 12644400 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 58 warning messages. -1 findbugs . The patch appears to introduce 2 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.master.TestAssignmentManager Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9516//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -
          58 warnings
          [WARNING] Javadoc Warnings
          [WARNING] javadoc: warning - Multiple sources of package comments found for package "org.apache.hadoop.hbase.io.hfile"
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:120: warning - Tag @link: reference not found: BUCKET_CACHE_COMBINED_KEY
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:88: warning - Tag @link: reference not found: BUCKET_CACHE_SIZE_KEY
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:99: warning - Tag @link: reference not found: BUCKET_CACHE_COMBINED_KEY
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:99: warning - Tag @link: reference not found: BUCKET_CACHE_COMBINED_PERCENTAGE_KEY
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:120: warning - Tag @link: reference not found: BUCKET_CACHE_COMBINED_KEY
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java:40: warning - Missing closing '}' character for inline tag: "{@link #getBlock(BlockCacheKey, boolean, boolean) reads
          

          This is not part of the patch.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - 58 warnings [WARNING] Javadoc Warnings [WARNING] javadoc: warning - Multiple sources of package comments found for package "org.apache.hadoop.hbase.io.hfile" [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:120: warning - Tag @link: reference not found: BUCKET_CACHE_COMBINED_KEY [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:88: warning - Tag @link: reference not found: BUCKET_CACHE_SIZE_KEY [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:99: warning - Tag @link: reference not found: BUCKET_CACHE_COMBINED_KEY [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:99: warning - Tag @link: reference not found: BUCKET_CACHE_COMBINED_PERCENTAGE_KEY [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:120: warning - Tag @link: reference not found: BUCKET_CACHE_COMBINED_KEY [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CombinedBlockCache.java:40: warning - Missing closing '}' character for inline tag: "{@link #getBlock(BlockCacheKey, boolean , boolean ) reads This is not part of the patch.
          Hide
          apurtell Andrew Purtell added a comment -

          Move to 0.98.4. This is about ready but there is shell support and integration test related issues that should go in at the same time. Unlikely all will make the train for .3 but likely all will make it for .4.

          Show
          apurtell Andrew Purtell added a comment - Move to 0.98.4. This is about ready but there is shell support and integration test related issues that should go in at the same time. Unlikely all will make the train for .3 but likely all will make it for .4.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Patch based on HBASE-11126. Can rebase once again after it gets committed.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Patch based on HBASE-11126 . Can rebase once again after it gets committed.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Updated patch after HBASE-11126 went in. Tested the patch with an IT case that am working on internally. Will also add the IT patch once it is tested fully.
          Will add it to the RB also.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Updated patch after HBASE-11126 went in. Tested the patch with an IT case that am working on internally. Will also add the IT patch once it is tested fully. Will add it to the RB also.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12648430/HBASE-10885_v2.patch
          against trunk revision .
          ATTACHMENT ID: 12648430

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +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.TestImportTSVWithVisibilityLabels

          -1 core zombie tests. There are 1 zombie test(s):

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12648430/HBASE-10885_v2.patch against trunk revision . ATTACHMENT ID: 12648430 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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.TestImportTSVWithVisibilityLabels -1 core zombie tests . There are 1 zombie test(s): Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9696//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          org.apache.hadoop.hbase.mapreduce.TestImportTSVWithVisibilityLabels

          I tried running this on 0.98 it passed. Let me check with trunk.
          https://builds.apache.org/job/PreCommit-HBASE-Build/9696//testReport/ - does not have the test reports like we used to have.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - org.apache.hadoop.hbase.mapreduce.TestImportTSVWithVisibilityLabels I tried running this on 0.98 it passed. Let me check with trunk. https://builds.apache.org/job/PreCommit-HBASE-Build/9696//testReport/ - does not have the test reports like we used to have.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          The TestImportTSVWithVisibilityLabels seems to pass even in trunk. wil resubmit again to see if it is an actual issue.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - The TestImportTSVWithVisibilityLabels seems to pass even in trunk. wil resubmit again to see if it is an actual issue.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12648616/HBASE-10885_v2.patch
          against trunk revision .
          ATTACHMENT ID: 12648616

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +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 1 zombie test(s): at org.apache.hadoop.hbase.mapreduce.TestTableMapReduceBase.testMultiRegionTable(TestTableMapReduceBase.java:96)

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12648616/HBASE-10885_v2.patch against trunk revision . ATTACHMENT ID: 12648616 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 1 zombie test(s): at org.apache.hadoop.hbase.mapreduce.TestTableMapReduceBase.testMultiRegionTable(TestTableMapReduceBase.java:96) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9704//console This message is automatically generated.
          Hide
          stack stack added a comment -

          I've not seen TestTableMapReduceBase.testMultiRegionTable( failing recently.... Let me retry the patch.

          Show
          stack stack added a comment - I've not seen TestTableMapReduceBase.testMultiRegionTable( failing recently.... Let me retry the patch.
          Hide
          stack stack added a comment -

          Retry

          Show
          stack stack added a comment - Retry
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12648643/HBASE-10885_v2.patch
          against trunk revision .
          ATTACHMENT ID: 12648643

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +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 passed unit tests in .

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12648643/HBASE-10885_v2.patch against trunk revision . ATTACHMENT ID: 12648643 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9705//console This message is automatically generated.
          Hide
          apurtell Andrew Purtell added a comment -

          Test results look good.

          Minor stuff.

          diff --git hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java
          index 6c43c78..6e79038 100644
          --- hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java
          +++ hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java
          @@ -27,4 +27,5 @@ public final class TagType {
             public static final byte ACL_TAG_TYPE = (byte) 1;
             public static final byte VISIBILITY_TAG_TYPE = (byte) 2;
             public static final byte LOG_REPLAY_TAG_TYPE = (byte) 3;
          +  public static final byte VISBILITY_EXP_SERIALIZATION_FORMAT = (byte)4;
          

          Misspelled. Also, can we use a better name for this? Everything else is named foo_TAG_TYPE.

          When you write that new tag e.g. in VisibilityController#createVisibilityTags you are doing this:

          +    // Add new tag type
          +    tags.add(new Tag(VisibilityUtils.VISBILITY_EXP_SERIALIZATION_FORMAT,
          +        HConstants.EMPTY_BYTE_ARRAY));
          

          The tag is empty (length 0) - doesn't seem right. Later checks for whether the serialization format has sorted ordinals or not use the presence of this tag as definitive, e.g.

          +      if(tag.getType() == VisibilityUtils.VISBILITY_EXP_SERIALIZATION_FORMAT) {
          

          That doesn't seem right either. The intent is versioning the serialization format. Therefore let's version it, with an integer value, and test the value.

          Comments in VisibilityScanDeleteTracker are good.

          Many unit tests, representing the bulk of the patch, good.

          Tested the patch with an IT case that am working on internally. Will also add the IT patch once it is tested fully.

          But not in the v2 patch, right? So that will be in v3?

          Show
          apurtell Andrew Purtell added a comment - Test results look good. Minor stuff. diff --git hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java index 6c43c78..6e79038 100644 --- hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java +++ hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java @@ -27,4 +27,5 @@ public final class TagType { public static final byte ACL_TAG_TYPE = ( byte ) 1; public static final byte VISIBILITY_TAG_TYPE = ( byte ) 2; public static final byte LOG_REPLAY_TAG_TYPE = ( byte ) 3; + public static final byte VISBILITY_EXP_SERIALIZATION_FORMAT = ( byte )4; Misspelled. Also, can we use a better name for this? Everything else is named foo_TAG_TYPE. When you write that new tag e.g. in VisibilityController#createVisibilityTags you are doing this: + // Add new tag type + tags.add( new Tag(VisibilityUtils.VISBILITY_EXP_SERIALIZATION_FORMAT, + HConstants.EMPTY_BYTE_ARRAY)); The tag is empty (length 0) - doesn't seem right. Later checks for whether the serialization format has sorted ordinals or not use the presence of this tag as definitive, e.g. + if (tag.getType() == VisibilityUtils.VISBILITY_EXP_SERIALIZATION_FORMAT) { That doesn't seem right either. The intent is versioning the serialization format. Therefore let's version it, with an integer value, and test the value. Comments in VisibilityScanDeleteTracker are good. Many unit tests, representing the bulk of the patch, good. Tested the patch with an IT case that am working on internally. Will also add the IT patch once it is tested fully. But not in the v2 patch, right? So that will be in v3?
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Thanks for the reviews. Updated patch addressing the comments.

          But not in the v2 patch, right? So that will be in v3?

          I meant that once my IT is ready will attach it in the relevant JIRA. Not along with this patch.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Thanks for the reviews. Updated patch addressing the comments. But not in the v2 patch, right? So that will be in v3? I meant that once my IT is ready will attach it in the relevant JIRA. Not along with this patch.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12648921/HBASE-10885_v3.patch
          against trunk revision .
          ATTACHMENT ID: 12648921

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

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

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

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

          +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.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3499)
          at org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3500)

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12648921/HBASE-10885_v3.patch against trunk revision . ATTACHMENT ID: 12648921 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +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.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3499) at org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3500) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9723//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Any more comments on this patch?

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Any more comments on this patch?
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Can you put the latest patch in RB?

          Show
          anoop.hbase Anoop Sam John added a comment - Can you put the latest patch in RB?
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Copying the discussion from RB to here because it will get better attention

          If suppose there are no tags in the delete cell, but the Cell that is getting covered by the delete has a Tag, then we should not delete the cell right? If that cell also does not have the tag then it is ok to consider it deleted as per the case.

          No. IMO no need to go this way. visibility tags might be there with put cells. This was done to control the visibility across different users. But delete can be like normal delete as today. A delete is issued and it is not having vis exp in it. Means all the cells covered by that tombstone must be deleted (irrespective of whether the put cells having tags or not). Issuing deletes with vis exp in it is special kind of deletion. There only do the matching of the exps and only the exp matching put cells can be deleted.

          So in the isDeleted() check the existing logic can still be good here in your new DeleteTracker impl. Under every case, have extra check for vis exp matching also. (When the delete tombstone under consideration was having a vis exp in it.)

          Correct me if I am wrong Andrew Purtell

          Show
          anoop.hbase Anoop Sam John added a comment - Copying the discussion from RB to here because it will get better attention If suppose there are no tags in the delete cell, but the Cell that is getting covered by the delete has a Tag, then we should not delete the cell right? If that cell also does not have the tag then it is ok to consider it deleted as per the case. No. IMO no need to go this way. visibility tags might be there with put cells. This was done to control the visibility across different users. But delete can be like normal delete as today. A delete is issued and it is not having vis exp in it. Means all the cells covered by that tombstone must be deleted (irrespective of whether the put cells having tags or not). Issuing deletes with vis exp in it is special kind of deletion. There only do the matching of the exps and only the exp matching put cells can be deleted. So in the isDeleted() check the existing logic can still be good here in your new DeleteTracker impl. Under every case, have extra check for vis exp matching also. (When the delete tombstone under consideration was having a vis exp in it.) Correct me if I am wrong Andrew Purtell
          Hide
          anoop.hbase Anoop Sam John added a comment -

          In 98.3 we allow the puts to have the vis exp in it. When a user issue delete, it will delete all covered cells (irrespective of whether the put cells having tags in it or not). This way it works till 98.3 Now in 98.4 we will change this behavior. Is that okey?

          Show
          anoop.hbase Anoop Sam John added a comment - In 98.3 we allow the puts to have the vis exp in it. When a user issue delete, it will delete all covered cells (irrespective of whether the put cells having tags in it or not). This way it works till 98.3 Now in 98.4 we will change this behavior. Is that okey?
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          We have this special case of visibility expression matching just to ensure that we try to delete only the matching visibility tag, if again we do the normal way of deleting also then may be we are not giving accumulo type of behaviour?
          AnyPut is considered to be tagged with a visibility and so only a matching delete should be used a tombstone I feel. BC break, in my perspective here is that may be we are adding this functionality to define Delete behaviour correctly in terms of Visibility exp.
          Then for delete family also if once a visibiity expression matches.. from there we could delete all the cells less that that particular version. That also defines the right behaviour in terms of delete family combined with visibility exp. In the current patch we only delete matching cells that are less than the given ts.

          This way it works till 98.3 Now in 98.4 we will change this behavior. Is that okey?

          If we just give the old behaviour also then there is no point in all these changes and this code should go only in major release only.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - We have this special case of visibility expression matching just to ensure that we try to delete only the matching visibility tag, if again we do the normal way of deleting also then may be we are not giving accumulo type of behaviour? AnyPut is considered to be tagged with a visibility and so only a matching delete should be used a tombstone I feel. BC break, in my perspective here is that may be we are adding this functionality to define Delete behaviour correctly in terms of Visibility exp. Then for delete family also if once a visibiity expression matches.. from there we could delete all the cells less that that particular version. That also defines the right behaviour in terms of delete family combined with visibility exp. In the current patch we only delete matching cells that are less than the given ts. This way it works till 98.3 Now in 98.4 we will change this behavior. Is that okey? If we just give the old behaviour also then there is no point in all these changes and this code should go only in major release only.
          Hide
          apurtell Andrew Purtell added a comment -

          In 98.3 we allow the puts to have the vis exp in it. When a user issue delete, it will delete all covered cells (irrespective of whether the put cells having tags in it or not). This way it works till 98.3 Now in 98.4 we will change this behavior.

          Yes.

          What we should accomplish with this patch are two objectives:
          1) Support deletes with user specified visibility expressions (if they have the authorizations)
          2) Do not allow users without visibility from covering cells with tombstones, effectively.

          #2 is the semantic change from earlier versions.

          From 0.98.0 we have been communicating at every opportunity that these features are experimental until 1.0. If there are still concerns or objections for this kind of change, we can guard the new behavior with a configuration toggle as a bridge from earlier 0.98 releases to 1.0.

          If suppose there are no tags in the delete cell, but the Cell that is getting covered by the delete has a Tag, then we should not delete the cell right? If that cell also does not have the tag then it is ok to consider it deleted as per the case.

          This isn't quite right though. If there are no tags in the delete cell, but the cell that is covered by the delete has a visibility expression tag, then we need to check if the effective auths of the user who issued the delete grant visibility to the covered cell. Only if true do we delete it. If the covered cell has no visibility expression tag it is ok to delete (cells without visibility expressions evaluate is visible to everyone)

          What do you think Anoop Sam John?

          Show
          apurtell Andrew Purtell added a comment - In 98.3 we allow the puts to have the vis exp in it. When a user issue delete, it will delete all covered cells (irrespective of whether the put cells having tags in it or not). This way it works till 98.3 Now in 98.4 we will change this behavior. Yes. What we should accomplish with this patch are two objectives: 1) Support deletes with user specified visibility expressions (if they have the authorizations) 2) Do not allow users without visibility from covering cells with tombstones, effectively. #2 is the semantic change from earlier versions. From 0.98.0 we have been communicating at every opportunity that these features are experimental until 1.0. If there are still concerns or objections for this kind of change, we can guard the new behavior with a configuration toggle as a bridge from earlier 0.98 releases to 1.0. If suppose there are no tags in the delete cell, but the Cell that is getting covered by the delete has a Tag, then we should not delete the cell right? If that cell also does not have the tag then it is ok to consider it deleted as per the case. This isn't quite right though. If there are no tags in the delete cell, but the cell that is covered by the delete has a visibility expression tag, then we need to check if the effective auths of the user who issued the delete grant visibility to the covered cell. Only if true do we delete it. If the covered cell has no visibility expression tag it is ok to delete (cells without visibility expressions evaluate is visible to everyone) What do you think Anoop Sam John ?
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          then we need to check if the effective auths of the user who issued the delete grant visibility to the covered cell. Only if true do we delete it.

          Oh.. Then this becomes more complex. So not only the cell visibility associated with the delete needs to be considered, in cases that is not present then go with the Auths defined in the labels table? In that sense I feel passing Cell visibility itself is not needed right? Apply deletes only based on the Auths associated with the user issuing the delete. Also if this behaviour we need not only we need to match the exact labels but also do as in Scan and find out if the labels in the cell is a subset of the auths and then decide the delete behaviour.

          #2 is the semantic change from earlier versions.

          I agree that we can change the semantics without which we are not allowing the deletes to function properly.
          I was testing more scenarios today and ended up in more tricky situations

          put 't1','r1','f1:c1','100',101,{VISIBILITY=>'PRIVATE|SECRET'}
          put 't1','r1','f1:c1','100',102,{VISIBILITY=>'PRIVATE'}
          put 't1','r1','f1:c1','100',103,{VISIBILITY=>'(SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL)'}
          put 't1','r1','f1:c1','100',104,{VISIBILITY=>'(SECRET&TOPSECRET)'}
          put 't1','r1','f1:c1','100',105,{VISIBILITY=>'(SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL)'}
          

          Now issue these two deletes

          delete_column_timeStamp 't1','r1','f1:c1',104,'SECRET&TOPSECRET'
          delete_familyversion 't1','r1','f1:c1',103,'(CONFIDENTIAL&PRIVATE)|(TOPSECRET&SECRET)'
          

          Now when we do

          delete_family 't1','r1','f1','PRIVATE|SECRET'
          

          Then we are bound to get back all the previously deleted cells also as per the current patch because DELETE_FAMILY being a higher precedence we don check for lower types of deletes and only match the visibility labels for those cells with PRIVATE|SECRET and this makes already deleted cells to reappear.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - then we need to check if the effective auths of the user who issued the delete grant visibility to the covered cell. Only if true do we delete it. Oh.. Then this becomes more complex. So not only the cell visibility associated with the delete needs to be considered, in cases that is not present then go with the Auths defined in the labels table? In that sense I feel passing Cell visibility itself is not needed right? Apply deletes only based on the Auths associated with the user issuing the delete. Also if this behaviour we need not only we need to match the exact labels but also do as in Scan and find out if the labels in the cell is a subset of the auths and then decide the delete behaviour. #2 is the semantic change from earlier versions. I agree that we can change the semantics without which we are not allowing the deletes to function properly. I was testing more scenarios today and ended up in more tricky situations put 't1','r1','f1:c1','100',101,{VISIBILITY=>'PRIVATE|SECRET'} put 't1','r1','f1:c1','100',102,{VISIBILITY=>'PRIVATE'} put 't1','r1','f1:c1','100',103,{VISIBILITY=>'(SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL)'} put 't1','r1','f1:c1','100',104,{VISIBILITY=>'(SECRET&TOPSECRET)'} put 't1','r1','f1:c1','100',105,{VISIBILITY=>'(SECRET&TOPSECRET)|(PRIVATE&CONFIDENTIAL)'} Now issue these two deletes delete_column_timeStamp 't1','r1','f1:c1',104,'SECRET&TOPSECRET' delete_familyversion 't1','r1','f1:c1',103,'(CONFIDENTIAL&PRIVATE)|(TOPSECRET&SECRET)' Now when we do delete_family 't1','r1','f1','PRIVATE|SECRET' Then we are bound to get back all the previously deleted cells also as per the current patch because DELETE_FAMILY being a higher precedence we don check for lower types of deletes and only match the visibility labels for those cells with PRIVATE|SECRET and this makes already deleted cells to reappear.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Latest patch updating the comments. The delete family related issue is solved.
          The delete cells with and without Visibility tags is not yet handled. I think we could discuss and then proceed on that.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Latest patch updating the comments. The delete family related issue is solved. The delete cells with and without Visibility tags is not yet handled. I think we could discuss and then proceed on that.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12650293/HBASE-10885_v4.patch
          against trunk revision .
          ATTACHMENT ID: 12650293

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +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.client.TestMultiParallel
          org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes

          -1 core zombie tests. There are 1 zombie test(s): at org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3499)

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12650293/HBASE-10885_v4.patch against trunk revision . ATTACHMENT ID: 12650293 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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.client.TestMultiParallel org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes -1 core zombie tests . There are 1 zombie test(s): at org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3499) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9766//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Not sure why this
          org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes
          failed. Seems to pass always. Not able to see the reports over there. Let me resubmit once again to know if I uploaded a wrong patch.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Not sure why this org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes failed. Seems to pass always. Not able to see the reports over there. Let me resubmit once again to know if I uploaded a wrong patch.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12650472/HBASE-10885_v5.patch
          against trunk revision .
          ATTACHMENT ID: 12650472

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 findbugs. The patch appears to introduce 1 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.security.visibility.TestVisibilityLabelsWithDeletes

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12650472/HBASE-10885_v5.patch against trunk revision . ATTACHMENT ID: 12650472 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 findbugs . The patch appears to introduce 1 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.security.visibility.TestVisibilityLabelsWithDeletes Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9775//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          The reason for the test failure is due to the presence of an assert stmt in VisisbilityScanDeleteTracker - the scenario is possible now. Now the test case passes though the same was passing in my IDE as there was no assertion enabled.
          Will upload a new patch after some internal discussions are updated in the patch (after finalising).

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - The reason for the test failure is due to the presence of an assert stmt in VisisbilityScanDeleteTracker - the scenario is possible now. Now the test case passes though the same was passing in my IDE as there was no assertion enabled. Will upload a new patch after some internal discussions are updated in the patch (after finalising).
          Hide
          anoop.hbase Anoop Sam John added a comment -

          We can't see the test failure report in the link. Actually it gives 404 error..
          https://builds.apache.org/job/PreCommit-HBASE-Build/9775//testReport/

          It got broken recently. Any one having some idea?

          Show
          anoop.hbase Anoop Sam John added a comment - We can't see the test failure report in the link. Actually it gives 404 error.. https://builds.apache.org/job/PreCommit-HBASE-Build/9775//testReport/ It got broken recently. Any one having some idea?
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          I can access above URL:

          Tests in error: 
            testVisibilityExpressionWithNotEqualORCondition(org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes): Failed after retry of OutOfOrderScannerNextException: was there a rpc timeout?
            testDeleteColumnSpecificTimeStampWithMulipleVersionsDoneTwice2(org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes): Failed after retry of OutOfOrderScannerNextException: was there a rpc timeout?
            testDeleteColumnWithSpecificTimeStampUsingMultipleVersionsUnMatchingVisExpression(org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes): Failed after retry of OutOfOrderScannerNextException: was there a rpc timeout?
          
          Show
          yuzhihong@gmail.com Ted Yu added a comment - I can access above URL: Tests in error: testVisibilityExpressionWithNotEqualORCondition(org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes): Failed after retry of OutOfOrderScannerNextException: was there a rpc timeout? testDeleteColumnSpecificTimeStampWithMulipleVersionsDoneTwice2(org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes): Failed after retry of OutOfOrderScannerNextException: was there a rpc timeout? testDeleteColumnWithSpecificTimeStampUsingMultipleVersionsUnMatchingVisExpression(org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDeletes): Failed after retry of OutOfOrderScannerNextException: was there a rpc timeout?
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Test output from QA run.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Test output from QA run.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          I already saw that in my local machine and corrected the patch. That was due to an assertion that was added in the code. I have corrected it but as I said we are working on some more things. Once it is finalised I will update the patch. Any way after removing the assertion the failed cases passes.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - I already saw that in my local machine and corrected the patch. That was due to an assertion that was added in the code. I have corrected it but as I said we are working on some more things. Once it is finalised I will update the patch. Any way after removing the assertion the failed cases passes.
          Hide
          apurtell Andrew Purtell added a comment -

          This isn't quite right though. If there are no tags in the delete cell, but the cell that is covered by the delete has a visibility expression tag, then we need to check if the effective auths of the user who issued the delete grant visibility to the covered cell.

          Oh.. Then this becomes more complex. So not only the cell visibility associated with the delete needs to be considered, in cases that is not present then go with the Auths defined in the labels table?

          That's a good point. We may be getting ahead of ourselves here.

          Let us separate mechanism and policy considerations.

          For mechanism, we have a goal like: IF a delete operation has a visibility expression associated with it, then the tombstone we lay down should be tagged with that visibility expression for later evaluation during delete tracking. A visibility expression defines the auths needed to see the mutation. A delete mutation is the negation of a put. Therefore, any cells with visibility expressions that match or are a subset of the expression on the tombstone should be removed by the delete tracker.

          For policy, we have considerations like whether or not to validate if the visibility expression supplied by a user contains only labels from their effective auth set.

          So why not separate the two for sake of discussion and completing this issue?

          If we separate mechanism from policy, then we can complete this issue by implementing visibility expression based delete tracking but not the other stuff I started talking about like checking if the effective auths of the user who issued the delete allow them to use all of the labels in the expression. It also means that visibility expression based filtering during deletes can be bypassed by a user just issuing a delete with no expression. However, that's fine if the goal is providing mechanisms for higher layers to use. It only becomes a problem if, after finishing this issue, we then debate whether we should check on all mutations if the labels supplied are in the auth set of the user....

          Show
          apurtell Andrew Purtell added a comment - This isn't quite right though. If there are no tags in the delete cell, but the cell that is covered by the delete has a visibility expression tag, then we need to check if the effective auths of the user who issued the delete grant visibility to the covered cell. Oh.. Then this becomes more complex. So not only the cell visibility associated with the delete needs to be considered, in cases that is not present then go with the Auths defined in the labels table? That's a good point. We may be getting ahead of ourselves here. Let us separate mechanism and policy considerations. For mechanism, we have a goal like: IF a delete operation has a visibility expression associated with it, then the tombstone we lay down should be tagged with that visibility expression for later evaluation during delete tracking. A visibility expression defines the auths needed to see the mutation. A delete mutation is the negation of a put. Therefore, any cells with visibility expressions that match or are a subset of the expression on the tombstone should be removed by the delete tracker. For policy, we have considerations like whether or not to validate if the visibility expression supplied by a user contains only labels from their effective auth set. So why not separate the two for sake of discussion and completing this issue? If we separate mechanism from policy, then we can complete this issue by implementing visibility expression based delete tracking but not the other stuff I started talking about like checking if the effective auths of the user who issued the delete allow them to use all of the labels in the expression. It also means that visibility expression based filtering during deletes can be bypassed by a user just issuing a delete with no expression. However, that's fine if the goal is providing mechanisms for higher layers to use. It only becomes a problem if, after finishing this issue, we then debate whether we should check on all mutations if the labels supplied are in the auth set of the user....
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Updated patch. Addressing the review comments.
          The idea of checking with the user's auth for a given mutation before applying the mutation and if the deletes does not have a cell visibility checking for the user's auth issuing the delete and based on that decide the covering cells will be done in a follow up issue.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Updated patch. Addressing the review comments. The idea of checking with the user's auth for a given mutation before applying the mutation and if the deletes does not have a cell visibility checking for the user's auth issuing the delete and based on that decide the covering cells will be done in a follow up issue.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12650978/HBASE-10885_v7.patch
          against trunk revision .
          ATTACHMENT ID: 12650978

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 findbugs. The patch appears to introduce 1 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.master.TestMasterNoCluster
          org.apache.hadoop.hbase.regionserver.TestHRegion
          org.apache.hadoop.hbase.snapshot.TestFlushSnapshotFromClient

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12650978/HBASE-10885_v7.patch against trunk revision . ATTACHMENT ID: 12650978 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 findbugs . The patch appears to introduce 1 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.master.TestMasterNoCluster org.apache.hadoop.hbase.regionserver.TestHRegion org.apache.hadoop.hbase.snapshot.TestFlushSnapshotFromClient Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9788//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Ping!!

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Ping!!
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Will review today Ram. Sorry for the delay.

          Show
          anoop.hbase Anoop Sam John added a comment - Will review today Ram. Sorry for the delay.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Done. Added some comments in RB.

          Show
          anoop.hbase Anoop Sam John added a comment - Done. Added some comments in RB.
          Hide
          apurtell Andrew Purtell added a comment -

          Sorry Ram.

          Anoop added good comments. I added a couple complimentary ones.

          Show
          apurtell Andrew Purtell added a comment - Sorry Ram. Anoop added good comments. I added a couple complimentary ones.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Updated patch addressing all the comments in the RB.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Updated patch addressing all the comments in the RB.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Latest patch. There is one change in the Visibility Delete Tracker where multiple family delete are handled by tracking those time stamps and also multiple delete columns are also handled. I think this covers all the cases and the cases have been verified running the IT test case that will be attached in HBASE-11039.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Latest patch. There is one change in the Visibility Delete Tracker where multiple family delete are handled by tracking those time stamps and also multiple delete columns are also handled. I think this covers all the cases and the cases have been verified running the IT test case that will be attached in HBASE-11039 .
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          One round of review on the VisisbilityDeleteTracker would be great before committing this.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - One round of review on the VisisbilityDeleteTracker would be great before committing this.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          One round of review on the VisisbilityDeleteTracker would be great before committing this.

          Good testing Ram. U found the missing scenario we missed in reviews also..

          Looking at this VisisbilityDeleteTracker I think still need changes.
          I think for every delete type u have to track TS vs vis exp.
          In normal case, for family delete, the latest TS only we need to track and delete all under that TS.
          Now with vis exp also in place, the combination of TS + vis exp us becoming something based on which the decision of whether cell is deleted or not is calculated.
          So in this tracking if same family comes and vis exp is different you have to track that TS vs vis exp. And if again same family comes wih same as previous vis exp, just ignore (As the second one TS will be lower. We need consider the higher TS value)

          Similar way for all types we will need track.

          Things are bit complex

          Show
          anoop.hbase Anoop Sam John added a comment - One round of review on the VisisbilityDeleteTracker would be great before committing this. Good testing Ram. U found the missing scenario we missed in reviews also.. Looking at this VisisbilityDeleteTracker I think still need changes. I think for every delete type u have to track TS vs vis exp. In normal case, for family delete, the latest TS only we need to track and delete all under that TS. Now with vis exp also in place, the combination of TS + vis exp us becoming something based on which the decision of whether cell is deleted or not is calculated. So in this tracking if same family comes and vis exp is different you have to track that TS vs vis exp. And if again same family comes wih same as previous vis exp, just ignore (As the second one TS will be lower. We need consider the higher TS value) Similar way for all types we will need track. Things are bit complex
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Adds cases where a put reappears with already deleted vis exp. Thanks Anoop for the review.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Adds cases where a put reappears with already deleted vis exp. Thanks Anoop for the review.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          REsubmitting

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - REsubmitting
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Final review before commit pls?

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Final review before commit pls?
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Ping!!!

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Ping!!!
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Give me a day time Ram. Busy with another patch.

          Show
          anoop.hbase Anoop Sam John added a comment - Give me a day time Ram. Busy with another patch.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          One more quick comment
          Check for the new tag type (ie. the Serialization format tag type) also in the VC reserved tags check.

          Will check new ScanDeleteTracker code once again.

          Show
          anoop.hbase Anoop Sam John added a comment - One more quick comment Check for the new tag type (ie. the Serialization format tag type) also in the VC reserved tags check. Will check new ScanDeleteTracker code once again.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Updated the comment regarding the REserved tags. And to be on the safer side using Map<Long,List<Tag>> for the delete column specific version.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Updated the comment regarding the REserved tags. And to be on the safer side using Map<Long,List<Tag>> for the delete column specific version.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          If i get +1s - I will commit later today/tomorrow.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - If i get +1s - I will commit later today/tomorrow.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12653152/HBASE-10885_v13.patch
          against trunk revision .
          ATTACHMENT ID: 12653152

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

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.

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

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + // TODO : See if HRegion.updateDeleteLatestVersionTimeStamp() could be called only if the hook is
          + // We are creating a new type of delete tracker here which is able to track the timestamps and also the
          + // visibility tags per cell. The covering cells are determined not only based on the delete type and ts
          + public void testVisibilityLabelsWithDeleteColumnsWithNoMatchVisExpWithMultipleVersionsNoTimestamp()
          + public void testVisibilityLabelsWithDeleteFamilyWithNoMatchingVisExpWithMultipleVersionsNoTimestamp()

          -1 site. The patch appears to cause mvn site goal to fail.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.http.TestSSLHttpServer

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9891//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9891//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12653152/HBASE-10885_v13.patch against trunk revision . ATTACHMENT ID: 12653152 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 findbugs . The patch appears to cause Findbugs (version 1.3.9) to fail. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + // TODO : See if HRegion.updateDeleteLatestVersionTimeStamp() could be called only if the hook is + // We are creating a new type of delete tracker here which is able to track the timestamps and also the + // visibility tags per cell. The covering cells are determined not only based on the delete type and ts + public void testVisibilityLabelsWithDeleteColumnsWithNoMatchVisExpWithMultipleVersionsNoTimestamp() + public void testVisibilityLabelsWithDeleteFamilyWithNoMatchingVisExpWithMultipleVersionsNoTimestamp() -1 site . The patch appears to cause mvn site goal to fail. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.http.TestSSLHttpServer Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9891//testReport/ Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9891//console This message is automatically generated.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          VisibilityScanDeleteTracker#add()
          For the deleteColumns case and delete column versions, We keep on extracting tags and add to a List. This List is cleared only on reset() which is called only when a row changes..
          But we need to reset these 2 Lists when the column (cf:q) changes.

          Show
          anoop.hbase Anoop Sam John added a comment - VisibilityScanDeleteTracker#add() For the deleteColumns case and delete column versions, We keep on extracting tags and add to a List. This List is cleared only on reset() which is called only when a row changes.. But we need to reset these 2 Lists when the column (cf:q) changes.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -
          +        deleteBuffer = null;
          +        visibilityTagsDeleteColumns = null;
          +        visiblityTagsDeleteColumnVersion = null;
          

          It is reset here also when the column qualifier changes.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - + deleteBuffer = null ; + visibilityTagsDeleteColumns = null ; + visiblityTagsDeleteColumnVersion = null ; It is reset here also when the column qualifier changes.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12653366/HBASE-10885_v15.patch
          against trunk revision .
          ATTACHMENT ID: 12653366

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

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 findbugs. The patch appears to introduce 13 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.hdfs.server.namenode.TestCheckpoint.testSecondaryFailsWithErrorBeforeSettingHeaders(TestCheckpoint.java:570)
          at org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3500)

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12653366/HBASE-10885_v15.patch against trunk revision . ATTACHMENT ID: 12653366 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 findbugs . The patch appears to introduce 13 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.hdfs.server.namenode.TestCheckpoint.testSecondaryFailsWithErrorBeforeSettingHeaders(TestCheckpoint.java:570) at org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3500) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9920//console This message is automatically generated.
          Hide
          apurtell Andrew Purtell added a comment -

          This is a blocker for 0.98 but should not go in here before commit to 1.0. Please ping Enis Soztutar when ready.

          Show
          apurtell Andrew Purtell added a comment - This is a blocker for 0.98 but should not go in here before commit to 1.0. Please ping Enis Soztutar when ready.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Sure. Its in the final stages of review. We got lot of new scenarios while reviewing and also while fixing one we got new types of combinations.
          Some small minor logical changes were suggested by Anoop on the final review. Just discussing them if the changes (if we make it or not) are fine then we can commit it later today EOD my time. Also Anoop is on travel now so once he finalises the patch we can be ready for commit.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Sure. Its in the final stages of review. We got lot of new scenarios while reviewing and also while fixing one we got new types of combinations. Some small minor logical changes were suggested by Anoop on the final review. Just discussing them if the changes (if we make it or not) are fine then we can commit it later today EOD my time. Also Anoop is on travel now so once he finalises the patch we can be ready for commit.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          +1 for the latest patch in RB
          nit : Pls add private Interface audience to VisibilityScanDeleteTracker. Can fix on commit.

          Thanks for the efforts and great work Ram.

          Show
          anoop.hbase Anoop Sam John added a comment - +1 for the latest patch in RB nit : Pls add private Interface audience to VisibilityScanDeleteTracker. Can fix on commit. Thanks for the efforts and great work Ram.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Enis Soztutar
          Can this be committed to branch 1.0 also? Waiting for your nod. Only later I will commit to 0.98.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Enis Soztutar Can this be committed to branch 1.0 also? Waiting for your nod. Only later I will commit to 0.98.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Latest patch addressing all the comments. Added new test case for the ImportTSV case so that after loading thro MR we can verify the deletes also. This is what I will be committing.
          In the previous QA run it shows 13 find bugs. I am not able to see that link. I ran mvn package findbugs:findbugs and saw the XML but inferring from them is bit difficult becuase it shows line that was not added by this patch also. For eg,

          instanceOccurrenceNum='0' priority='2' abbrev='IS' type='IS2_INCONSISTENT_SYNC' instanceOccurrenceMax='0'><ShortMessage>Inconsistent synchronization</ShortMessage><LongMessage>Inconsistent synchronization of org.apache.hadoop.hbase.security.visibility.VisibilityController.regionEnv; locked 62% of time</LongMessage><Class classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' primary='true'><SourceLine start='132' classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' sourcepath='org/apache/hadoop/hbase/security/visibility/VisibilityController.java' sourcefile='VisibilityController.java' end='1501'><Message>At VisibilityController.java:[lines 132-1501]</Message></SourceLine><Message>In class org.apache.hadoop.hbase.security.visibility.VisibilityController</Message></Class><Field isStatic='false' classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' name='regionEnv' primary='true' signature='Lorg/apache/hadoop/hbase/coprocessor/RegionCoprocessorEnvironment;'><SourceLine classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' sourcepath='org/apache/hadoop/hbase/security/visibility/VisibilityController.java' sourcefile='VisibilityController.java'><Message>In VisibilityController.java</Message></SourceLine><Message>Field org.apache.hadoop.hbase.security.visibility.VisibilityController.regionEnv</Message></Field><Int value='62' role='INT_SYNC_PERCENT'><Message>Synchronized 62% of the time</Message></Int><SourceLine endBytecode='9' startBytecode='9' start='1320' classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' primary='true' sourcepath='org/apache/hadoop/hbase/security/visibility/VisibilityController.java' role='SOURCE_LINE_UNSYNC_ACCESS' sourcefile='VisibilityController.java' end='1320'><Message>Unsynchronized access at VisibilityController.java:[line 1320]</Message></SourceLine><SourceLine endBytecode='72' startBytecode='72' start='192' classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' sourcepath='org/apache/hadoop/hbase/security/visibility/VisibilityController.java' role='SOURCE_LINE_UNSYNC_ACCESS' sourcefile='VisibilityController.java' end='192'>
          

          So if there any any real find bug issue will fix it as a followup?

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Latest patch addressing all the comments. Added new test case for the ImportTSV case so that after loading thro MR we can verify the deletes also. This is what I will be committing. In the previous QA run it shows 13 find bugs. I am not able to see that link. I ran mvn package findbugs:findbugs and saw the XML but inferring from them is bit difficult becuase it shows line that was not added by this patch also. For eg, instanceOccurrenceNum='0' priority='2' abbrev='IS' type='IS2_INCONSISTENT_SYNC' instanceOccurrenceMax='0'><ShortMessage>Inconsistent synchronization</ShortMessage><LongMessage>Inconsistent synchronization of org.apache.hadoop.hbase.security.visibility.VisibilityController.regionEnv; locked 62% of time</LongMessage>< Class classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' primary=' true '><SourceLine start='132' classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' sourcepath='org/apache/hadoop/hbase/security/visibility/VisibilityController.java' sourcefile='VisibilityController.java' end='1501'><Message>At VisibilityController.java:[lines 132-1501]</Message></SourceLine><Message>In class org.apache.hadoop.hbase.security.visibility.VisibilityController</Message></ Class ><Field isStatic=' false ' classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' name='regionEnv' primary=' true ' signature='Lorg/apache/hadoop/hbase/coprocessor/RegionCoprocessorEnvironment;'><SourceLine classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' sourcepath='org/apache/hadoop/hbase/security/visibility/VisibilityController.java' sourcefile='VisibilityController.java'><Message>In VisibilityController.java</Message></SourceLine><Message>Field org.apache.hadoop.hbase.security.visibility.VisibilityController.regionEnv</Message></Field><Int value='62' role='INT_SYNC_PERCENT'><Message>Synchronized 62% of the time</Message></Int><SourceLine endBytecode='9' startBytecode='9' start='1320' classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' primary=' true ' sourcepath='org/apache/hadoop/hbase/security/visibility/VisibilityController.java' role='SOURCE_LINE_UNSYNC_ACCESS' sourcefile='VisibilityController.java' end='1320'><Message>Unsynchronized access at VisibilityController.java:[line 1320]</Message></SourceLine><SourceLine endBytecode='72' startBytecode='72' start='192' classname='org.apache.hadoop.hbase.security.visibility.VisibilityController' sourcepath='org/apache/hadoop/hbase/security/visibility/VisibilityController.java' role='SOURCE_LINE_UNSYNC_ACCESS' sourcefile='VisibilityController.java' end='192'> So if there any any real find bug issue will fix it as a followup?
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          0.98 version's patch.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - 0.98 version's patch.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Branch-1 patch. I think already there is some compilation issue with the branch-1 and master.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Branch-1 patch. I think already there is some compilation issue with the branch-1 and master.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Verified in cluster with a table with puts before the patch (so that they are not sorted). The applied the patch and issued deletes. The patch works fine.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Verified in cluster with a table with puts before the patch (so that they are not sorted). The applied the patch and issued deletes. The patch works fine.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment - - edited

          Enis Soztutar
          Ping !! Can i commit this to branch-1?

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - - edited Enis Soztutar Ping !! Can i commit this to branch-1?
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Reattaching V17 patch for trunk for QA run.

          Show
          anoop.hbase Anoop Sam John added a comment - Reattaching V17 patch for trunk for QA run.
          Hide
          enis Enis Soztutar added a comment -

          +1 for branch-1, because this will help complete the story for cell level visibility.

          Show
          enis Enis Soztutar added a comment - +1 for branch-1, because this will help complete the story for cell level visibility.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12654057/HBASE-10885_v17.patch
          against trunk revision .
          ATTACHMENT ID: 12654057

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

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 findbugs. The patch appears to introduce 4 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 passed unit tests in .

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12654057/HBASE-10885_v17.patch against trunk revision . ATTACHMENT ID: 12654057 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 findbugs . The patch appears to introduce 4 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 passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9968//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -
          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Comparing the reports of https://builds.apache.org/job/PreCommit-HBASE-Build/9968//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html and https://builds.apache.org/job/PreCommit-HBASE-Build/9967//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html both seems similar. So I think this patch is not introducing any findbugs. Should be a residue from some previous patch.
          Hide
          apurtell Andrew Purtell added a comment -

          +1 for 0.98

          Show
          apurtell Andrew Purtell added a comment - +1 for 0.98
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Committed to 0.98, branch-1 and trunk.
          Thanks for excellent reviews to Anoop. It was very helpful to identify some missing cases.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Committed to 0.98, branch-1 and trunk. Thanks for excellent reviews to Anoop. It was very helpful to identify some missing cases.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.0 #13 (See https://builds.apache.org/job/HBase-1.0/13/)
          HBASE-10885 Support visibility expressions on Deletes (Ram) (ramkrishna: rev c7efcd0fb32e90dc2a3c2aeeeb8f6eb43758e39c)

          • hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LabelExpander.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.0 #13 (See https://builds.apache.org/job/HBase-1.0/13/ ) HBASE-10885 Support visibility expressions on Deletes (Ram) (ramkrishna: rev c7efcd0fb32e90dc2a3c2aeeeb8f6eb43758e39c) hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LabelExpander.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK #5273 (See https://builds.apache.org/job/HBase-TRUNK/5273/)
          HBASE-10885 Support visibility expressions on Deletes (Ram) (ramkrishna: rev 62c048660bd3d4062e56646ae7c17b9a2ef15614)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LabelExpander.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #5273 (See https://builds.apache.org/job/HBase-TRUNK/5273/ ) HBASE-10885 Support visibility expressions on Deletes (Ram) (ramkrishna: rev 62c048660bd3d4062e56646ae7c17b9a2ef15614) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LabelExpander.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-0.98 #376 (See https://builds.apache.org/job/HBase-0.98/376/)
          HBASE-10885 Support visibility expressions on Deletes (Ram) (ramkrishna: rev 4ad9bb39aef8c98bb6608223d77c7e147ae6d946)

          • hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LabelExpander.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #376 (See https://builds.apache.org/job/HBase-0.98/376/ ) HBASE-10885 Support visibility expressions on Deletes (Ram) (ramkrishna: rev 4ad9bb39aef8c98bb6608223d77c7e147ae6d946) hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LabelExpander.java hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #356 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/356/)
          HBASE-10885 Support visibility expressions on Deletes (Ram) (ramkrishna: rev 4ad9bb39aef8c98bb6608223d77c7e147ae6d946)

          • hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LabelExpander.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #356 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/356/ ) HBASE-10885 Support visibility expressions on Deletes (Ram) (ramkrishna: rev 4ad9bb39aef8c98bb6608223d77c7e147ae6d946) hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DeleteTracker.java hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanDeleteTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LabelExpander.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityUtils.java hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java hbase-common/src/main/java/org/apache/hadoop/hbase/TagType.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Just to add on
          The deletes happen based on pattern matching. If suppose a Cell has SECRET&TOPSECRET as the visibility expression then any delete coming with the same expression is matched for, but TOPSECRET&SECRET is also considered to be same
          A&B<=>B&A
          A|B<=>B|A.
          This is the difference between what Accumulo does in terms of delete where only the exact match is looked out for.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Just to add on The deletes happen based on pattern matching. If suppose a Cell has SECRET&TOPSECRET as the visibility expression then any delete coming with the same expression is matched for, but TOPSECRET&SECRET is also considered to be same A&B<=>B&A A|B<=>B|A. This is the difference between what Accumulo does in terms of delete where only the exact match is looked out for.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Ram, can you add release notes pls

          Show
          anoop.hbase Anoop Sam John added a comment - Ram, can you add release notes pls
          Hide
          enis Enis Soztutar added a comment -

          Closing this issue after 0.99.0 release.

          Show
          enis Enis Soztutar added a comment - Closing this issue after 0.99.0 release.

            People

            • Assignee:
              ram_krish ramkrishna.s.vasudevan
              Reporter:
              apurtell Andrew Purtell
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development