HBase
  1. HBase
  2. HBASE-10322

Strip tags from KV while sending back to client on reads

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.98.0
    • Fix Version/s: 0.98.0, 0.99.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      Added a new config hbase.replication.rpc.codec, using which one can specify the codec to be used by the replication. This config don't have a default value. When it is not specified, the codec specified by hbase.client.rpc.codec config will be used for replication.(For which org.apache.hadoop.hbase.codec.KeyValueCodec is the default value)

      Note : When using tags directly or indirectly (ie. Usage of cell level visibility labels or per cell ACL) make sure to configure hbase.replication.rpc.codec with a Codec which supports shipping of tags. We have org.apache.hadoop.hbase.codec.KeyValueCodecWithTags and org.apache.hadoop.hbase.codec.CellCodecWithTags, which can handle tags.
      Show
      Added a new config hbase.replication.rpc.codec, using which one can specify the codec to be used by the replication. This config don't have a default value. When it is not specified, the codec specified by hbase.client.rpc.codec config will be used for replication.(For which org.apache.hadoop.hbase.codec.KeyValueCodec is the default value) Note : When using tags directly or indirectly (ie. Usage of cell level visibility labels or per cell ACL) make sure to configure hbase.replication.rpc.codec with a Codec which supports shipping of tags. We have org.apache.hadoop.hbase.codec.KeyValueCodecWithTags and org.apache.hadoop.hbase.codec.CellCodecWithTags, which can handle tags.

      Description

      Right now we have some inconsistency wrt sending back tags on read. We do this in scan when using Java client(Codec based cell block encoding). But during a Get operation or when a pure PB based Scan comes we are not sending back the tags. So any of the below fix we have to do
      1. Send back tags in missing cases also. But sending back visibility expression/ cell ACL is not correct.
      2. Don't send back tags in any case. This will a problem when a tool like ExportTool use the scan to export the table data. We will miss exporting the cell visibility/ACL.
      3. Send back tags based on some condition. It has to be per scan basis. Simplest way is pass some kind of attribute in Scan which says whether to send back tags or not. But believing some thing what scan specifies might not be correct IMO. Then comes the way of checking the user who is doing the scan. When a HBase super user doing the scan then only send back tags. So when a case comes like Export Tool's the execution should happen from a super user.

      So IMO we should go with #3.
      Patch coming soon.

      1. HBASE-10322_codec.patch
        3 kB
        ramkrishna.s.vasudevan
      2. HBASE-10322_V2.patch
        32 kB
        Anoop Sam John
      3. HBASE-10322_V3.patch
        59 kB
        Anoop Sam John
      4. HBASE-10322_V4.patch
        70 kB
        Anoop Sam John
      5. HBASE-10322_V5.patch
        71 kB
        Anoop Sam John
      6. HBASE-10322_V6.patch
        61 kB
        Anoop Sam John
      7. HBASE-10322.patch
        73 kB
        Anoop Sam John

        Issue Links

          Activity

          Show
          Anoop Sam John added a comment - https://reviews.apache.org/r/16810/
          Hide
          Hadoop QA added a comment -

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

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

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

          +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

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

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

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

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

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + assertEquals(3L, Bytes.toLong(kv.getValueArray(), kv.getValueOffset(), kv.getValueLength()));
          + increment.add(new KeyValue(row1, f, q, 1234L, v, new Tag[]

          { new Tag((byte) 1, "tag2") }));
          + assertEquals(5L, Bytes.toLong(kv.getValueArray(), kv.getValueOffset(), kv.getValueLength()));
          + increment.add(new KeyValue(row2, f, q, 1234L, v, new Tag[] { new Tag((byte) 1, "tag2") }

          ));
          + assertEquals(4L, Bytes.toLong(kv.getValueArray(), kv.getValueOffset(), kv.getValueLength()));

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

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12622534/HBASE-10322.patch against trunk revision . ATTACHMENT ID: 12622534 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 24 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + assertEquals(3L, Bytes.toLong(kv.getValueArray(), kv.getValueOffset(), kv.getValueLength())); + increment.add(new KeyValue(row1, f, q, 1234L, v, new Tag[] { new Tag((byte) 1, "tag2") })); + assertEquals(5L, Bytes.toLong(kv.getValueArray(), kv.getValueOffset(), kv.getValueLength())); + increment.add(new KeyValue(row2, f, q, 1234L, v, new Tag[] { new Tag((byte) 1, "tag2") } )); + assertEquals(4L, Bytes.toLong(kv.getValueArray(), kv.getValueOffset(), kv.getValueLength())); -1 site . The patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8392//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

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

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

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

          +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

          +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 patch appears to cause mvn site goal to fail.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12622539/HBASE-10322.patch against trunk revision . ATTACHMENT ID: 12622539 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 24 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +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 patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8393//console This message is automatically generated.
          Hide
          Andrew Purtell added a comment -

          +1

          Show
          Andrew Purtell added a comment - +1
          Hide
          stack added a comment -
          • final CellScanner cellScanner)
          • throws IOException {
            + final CellScanner cellScanner, final boolean isClientContext) throws IOException {

          The above is from a class named IPCUtil and the method is buildCellBlock. Passing a flag named isClientContext seems odd to me. Why does IPCUtil or buildCellBlock care whether client or server context? Or rather, it should not have to care. Why not pass in an encoder that does client encoding rather than do this passing of a flag.

          isClientContext is the wrong name for such a flag. It should be clientContext. isClientContext is the name of the method that would check the clientContext data member setting.

          bbias

          Show
          stack added a comment - final CellScanner cellScanner) throws IOException { + final CellScanner cellScanner, final boolean isClientContext) throws IOException { The above is from a class named IPCUtil and the method is buildCellBlock. Passing a flag named isClientContext seems odd to me. Why does IPCUtil or buildCellBlock care whether client or server context? Or rather, it should not have to care. Why not pass in an encoder that does client encoding rather than do this passing of a flag. isClientContext is the wrong name for such a flag. It should be clientContext. isClientContext is the name of the method that would check the clientContext data member setting. bbias
          Hide
          Anoop Sam John added a comment -

          Thanks Stack.

          Or rather, it should not have to care. Why not pass in an encoder that does client encoding rather than do this passing of a flag.

          That seems very much valid for me too.. Why not done is we need OutputStream to be passed for the creation of the Encoder. (Compressed or uncompressed). All these common logic is moved inside the IPCUtil now.. Passing in Encoder will make us to do this stuff out in RpcClient and RpcServer. So we can pass in boolean only? Will change the variable name (and in other places also)

          Show
          Anoop Sam John added a comment - Thanks Stack. Or rather, it should not have to care. Why not pass in an encoder that does client encoding rather than do this passing of a flag. That seems very much valid for me too.. Why not done is we need OutputStream to be passed for the creation of the Encoder. (Compressed or uncompressed). All these common logic is moved inside the IPCUtil now.. Passing in Encoder will make us to do this stuff out in RpcClient and RpcServer. So we can pass in boolean only? Will change the variable name (and in other places also)
          Hide
          stack added a comment -

          Let me continue the review Anoop (sorry took a while to get back here):

          -        ByteBuffer cellBlock = ipcUtil.buildCellBlock(this.codec, this.compressor, call.cells);
          +        ByteBuffer cellBlock = ipcUtil
          +            .buildCellBlock(this.codec, this.compressor, call.cells, true);
          

          The above is in RpcClient. Can the Codec be a client codec rather than pass a 'true' for client context? (Just asking....)

          This looks good:

          +    if (RequestContext.isSuperUserRequest()) {
          +      kvbuilder.setTags(ZeroCopyLiteralByteString.wrap(kv.getTagsArray(), kv.getTagsOffset(),
          +          kv.getTagsLength()));
          +    }
          

          .... as long as that call to isSuperUserRequest is cheap!!!!

          Is this call inexpensive?

          + if (cell.hasTags()) {

          What is this?

          + public static long oswrite(final KeyValue kv, final OutputStream out, final boolean withTags)

          You pass a flag if you want tags written? This saves a KV copy I'd imagine? If so, that is good.

          Now we never write tags in CellCodec?

          • // Write tags
          • write(cell.getTagsArray(), cell.getTagsOffset(), cell.getTagsLength());
            + // TODO writing tags can be implemented once we do connection negotiation work
            // MvccVersion

          How are we going to do the following in a backward compatible way?

          + // TODO writing tags can be implemented once we do connection negotiation work

          Are we going to up the rpc version number?

          It is odd that CellCodec Interface knows about 'client':

          + public Decoder getClientDecoder(InputStream is) {

          It shouldn't have to.

          Ditto for getServerDecoder.

          Codec should know nothing about client and server.

          The below seems wrong to me:

          • Decoder getDecoder(InputStream is);
          • Encoder getEncoder(OutputStream os);
            + Decoder getClientDecoder(InputStream is);
            + Encoder getClientEncoder(OutputStream os);
            + Decoder getServerDecoder(InputStream is);
            + Encoder getServerEncoder(OutputStream os);

          This also seems 'off':

          • public KeyValueEncoder(final OutputStream out) {
            + private final boolean isClientContext;

          Fix the below text:

          + * will be <code>null</code>. The CallRunner class before it a call and then on

          It is unorthodox in the hbase codebase having data members midway down the class:

          + private User user;
          + private boolean isSuperUser = false;
          + private InetAddress remoteAddress;
          + // indicates we're within a RPC request invocation
          + private boolean inRequest;

          On the below:

          • CallRunner(final RpcServerInterface rpcServer, final Call call, UserProvider userProvider) {
            + CallRunner(final RpcServerInterface rpcServer, final Call call, User user, boolean isSuperUser) {

          ...can we not pass in some object that contains User info and whether or not it is super user rather than pass User and super user separately? (It should be called superUser, not isSuperUser).

          RequestContext no longer takes service (see below)?

          • RequestContext.set(userProvider.create(call.connection.user), RpcServer.getRemoteIp(),
          • call.connection.service);
            + InetAddress remoteAddress = RpcServer.getRemoteIp();
            + RequestContext.set(user, isSuperUser, remoteAddress);

          Should this stuff below be in a finally?

          + RequestContext.clear();

          Yeah, this seems wrong Anoop:

          • ipcUtil.buildCellBlock(this.connection.codec, this.connection.compressionCodec, cells);
            + ByteBuffer cellBlock = ipcUtil.buildCellBlock(this.connection.codec,
            + this.connection.compressionCodec, cells, false);

          The bit where it is passing flag if server or client. Can this not be encapsulated inside in the codec?

          Pardon me Anoop but I don't like the way this is done. This may the only way to implement it but I'd like to hear argument why so and why we can't encapsulate the encode/decode in the codec implementation rather than leak client/server context beyond codec'ing.

          Good stuff Anoop.

          Show
          stack added a comment - Let me continue the review Anoop (sorry took a while to get back here): - ByteBuffer cellBlock = ipcUtil.buildCellBlock( this .codec, this .compressor, call.cells); + ByteBuffer cellBlock = ipcUtil + .buildCellBlock( this .codec, this .compressor, call.cells, true ); The above is in RpcClient. Can the Codec be a client codec rather than pass a 'true' for client context? (Just asking....) This looks good: + if (RequestContext.isSuperUserRequest()) { + kvbuilder.setTags(ZeroCopyLiteralByteString.wrap(kv.getTagsArray(), kv.getTagsOffset(), + kv.getTagsLength())); + } .... as long as that call to isSuperUserRequest is cheap!!!! Is this call inexpensive? + if (cell.hasTags()) { What is this? + public static long oswrite(final KeyValue kv, final OutputStream out, final boolean withTags) You pass a flag if you want tags written? This saves a KV copy I'd imagine? If so, that is good. Now we never write tags in CellCodec? // Write tags write(cell.getTagsArray(), cell.getTagsOffset(), cell.getTagsLength()); + // TODO writing tags can be implemented once we do connection negotiation work // MvccVersion How are we going to do the following in a backward compatible way? + // TODO writing tags can be implemented once we do connection negotiation work Are we going to up the rpc version number? It is odd that CellCodec Interface knows about 'client': + public Decoder getClientDecoder(InputStream is) { It shouldn't have to. Ditto for getServerDecoder. Codec should know nothing about client and server. The below seems wrong to me: Decoder getDecoder(InputStream is); Encoder getEncoder(OutputStream os); + Decoder getClientDecoder(InputStream is); + Encoder getClientEncoder(OutputStream os); + Decoder getServerDecoder(InputStream is); + Encoder getServerEncoder(OutputStream os); This also seems 'off': public KeyValueEncoder(final OutputStream out) { + private final boolean isClientContext; Fix the below text: + * will be <code>null</code>. The CallRunner class before it a call and then on It is unorthodox in the hbase codebase having data members midway down the class: + private User user; + private boolean isSuperUser = false; + private InetAddress remoteAddress; + // indicates we're within a RPC request invocation + private boolean inRequest; On the below: CallRunner(final RpcServerInterface rpcServer, final Call call, UserProvider userProvider) { + CallRunner(final RpcServerInterface rpcServer, final Call call, User user, boolean isSuperUser) { ...can we not pass in some object that contains User info and whether or not it is super user rather than pass User and super user separately? (It should be called superUser, not isSuperUser). RequestContext no longer takes service (see below)? RequestContext.set(userProvider.create(call.connection.user), RpcServer.getRemoteIp(), call.connection.service); + InetAddress remoteAddress = RpcServer.getRemoteIp(); + RequestContext.set(user, isSuperUser, remoteAddress); Should this stuff below be in a finally? + RequestContext.clear(); Yeah, this seems wrong Anoop: ipcUtil.buildCellBlock(this.connection.codec, this.connection.compressionCodec, cells); + ByteBuffer cellBlock = ipcUtil.buildCellBlock(this.connection.codec, + this.connection.compressionCodec, cells, false); The bit where it is passing flag if server or client. Can this not be encapsulated inside in the codec? Pardon me Anoop but I don't like the way this is done. This may the only way to implement it but I'd like to hear argument why so and why we can't encapsulate the encode/decode in the codec implementation rather than leak client/server context beyond codec'ing. Good stuff Anoop.
          Hide
          Andrew Purtell added a comment - - edited

          Pardon me Anoop but I don't like the way this is done. This may the only way to implement it but I'd like to hear argument why so and why we can't encapsulate the encode/decode in the codec implementation rather than leak client/server context beyond codec'ing.

          I'm fine with holding off the RC until you guys are both good here. Edit: Just in case the tone doesn't come through the text, I mean there's enough time as needed to work this into shape as far as I am concerned.

          Show
          Andrew Purtell added a comment - - edited Pardon me Anoop but I don't like the way this is done. This may the only way to implement it but I'd like to hear argument why so and why we can't encapsulate the encode/decode in the codec implementation rather than leak client/server context beyond codec'ing. I'm fine with holding off the RC until you guys are both good here. Edit: Just in case the tone doesn't come through the text, I mean there's enough time as needed to work this into shape as far as I am concerned.
          Hide
          stack added a comment -

          1. Send back tags in missing cases also. But sending back visibility expression/ cell ACL is not correct.

          This is tough. The Visibility tags are managed by CPs. When they are not present, you'd like to not return them? Are tags grouped? Don't send back system tags?

          2. Don't send back tags in any case. This will a problem when a tool like ExportTool use the scan to export the table data. We will miss exporting the cell visibility/ACL.

          Can we check perms of the client doing the export? If they have access to 'system' tags, export them? We'd have a ACLCheckingCodec?

          3. Send back tags based on some condition. It has to be per scan basis. Simplest way is pass some kind of attribute in Scan which says whether to send back tags or not. But believing some thing what scan specifies might not be correct IMO. Then comes the way of checking the user who is doing the scan. When a HBase super user doing the scan then only send back tags. So when a case comes like Export Tool's the execution should happen from a super user.

          Should be super user or some super user-like group if they want tags; else they don't get them?

          Show
          stack added a comment - 1. Send back tags in missing cases also. But sending back visibility expression/ cell ACL is not correct. This is tough. The Visibility tags are managed by CPs. When they are not present, you'd like to not return them? Are tags grouped? Don't send back system tags? 2. Don't send back tags in any case. This will a problem when a tool like ExportTool use the scan to export the table data. We will miss exporting the cell visibility/ACL. Can we check perms of the client doing the export? If they have access to 'system' tags, export them? We'd have a ACLCheckingCodec? 3. Send back tags based on some condition. It has to be per scan basis. Simplest way is pass some kind of attribute in Scan which says whether to send back tags or not. But believing some thing what scan specifies might not be correct IMO. Then comes the way of checking the user who is doing the scan. When a HBase super user doing the scan then only send back tags. So when a case comes like Export Tool's the execution should happen from a super user. Should be super user or some super user-like group if they want tags; else they don't get them?
          Hide
          Andrew Purtell added a comment -

          Our bottom line, in my opinion, is that tags don't end up in the hands of those who shouldn't see them.

          The rock bottom simplest way to do this is to just not support tags in RPC codecs. Maybe we can have a separate class that keeps them for the Export tool specifically? Import is no problem if the user, presumably privileged, is building HFiles and therefore the cells within them directly. Accumulo has the same approach to whole file imports - no checking done, YMMV.

          Show
          Andrew Purtell added a comment - Our bottom line, in my opinion, is that tags don't end up in the hands of those who shouldn't see them. The rock bottom simplest way to do this is to just not support tags in RPC codecs. Maybe we can have a separate class that keeps them for the Export tool specifically? Import is no problem if the user, presumably privileged, is building HFiles and therefore the cells within them directly. Accumulo has the same approach to whole file imports - no checking done, YMMV.
          Hide
          ramkrishna.s.vasudevan added a comment -

          The rock bottom simplest way to do this is to just not support tags in RPC codecs

          But from client to server it should be supported and in the WAL part it should be supported both ways. for export tool alone how to identify that the client is doing an export? We ended up discussing all this and came up with a patch.
          Another suggestion atleast to avoid changes to the codec part is to have an init() in the Codec.java. So once the codec is instantiated we could set this flag as true or false based on client or server.
          So for server if the flag says false then the tags are not sent back but for client it is always written. This involves changes to the Codec.java, introduces an init() method and decision is taken based on what is set on this init method. We have a patch for this, but again it does not do completely what Stack wants. Only a part of what Stack wants is solved by that.
          Anyway the User related things are just same as in the exisitng patch. This whole stripping of tags is really tricky.

          Show
          ramkrishna.s.vasudevan added a comment - The rock bottom simplest way to do this is to just not support tags in RPC codecs But from client to server it should be supported and in the WAL part it should be supported both ways. for export tool alone how to identify that the client is doing an export? We ended up discussing all this and came up with a patch. Another suggestion atleast to avoid changes to the codec part is to have an init() in the Codec.java. So once the codec is instantiated we could set this flag as true or false based on client or server. So for server if the flag says false then the tags are not sent back but for client it is always written. This involves changes to the Codec.java, introduces an init() method and decision is taken based on what is set on this init method. We have a patch for this, but again it does not do completely what Stack wants. Only a part of what Stack wants is solved by that. Anyway the User related things are just same as in the exisitng patch. This whole stripping of tags is really tricky.
          Hide
          Anoop Sam John added a comment - - edited

          Selectively sending back tags is one problem.. But that is second...
          The 1st problem is making codec to send tags when its Encoder encodes data from client to server. The same Codec Encoder, when working in server side should not send back the tags. This is where we were needing the context information. Also pls note one more thing. We use a WALCellCodec whose Encoder uses the KVCodec for writing to the WAL. When writing to the WAL, even if it is inside a server, it must write tags.. We have to solve this problem.. Selective sending based on user is second and it might be simpler that 1st IMO.

          Show
          Anoop Sam John added a comment - - edited Selectively sending back tags is one problem.. But that is second... The 1st problem is making codec to send tags when its Encoder encodes data from client to server. The same Codec Encoder, when working in server side should not send back the tags. This is where we were needing the context information. Also pls note one more thing. We use a WALCellCodec whose Encoder uses the KVCodec for writing to the WAL. When writing to the WAL, even if it is inside a server, it must write tags.. We have to solve this problem.. Selective sending based on user is second and it might be simpler that 1st IMO.
          Hide
          Anoop Sam John added a comment -

          BTW, the stripping of tags can be achieved by removing the tag from KV by the CP/Filter that it handles. This will allow system tags being blocked from sending back and other user tags getting back to the client. (The decision can be taken by the CP/Filter which handles the tags) This was from the begin of our discussions internally here. Just saying. The major concern with that was we will have to recreate KVs (In filter/cp) and byte array copying. The perf penalty is a major concern

          Show
          Anoop Sam John added a comment - BTW, the stripping of tags can be achieved by removing the tag from KV by the CP/Filter that it handles. This will allow system tags being blocked from sending back and other user tags getting back to the client. (The decision can be taken by the CP/Filter which handles the tags) This was from the begin of our discussions internally here. Just saying. The major concern with that was we will have to recreate KVs (In filter/cp) and byte array copying. The perf penalty is a major concern
          Hide
          ramkrishna.s.vasudevan added a comment -

          As Anoop says, even in Codec negotiation HBASE-9681, the problem is same.. in the sense any codec we write should behave differently when it works from client side and from the server side. Atleast in terms of tags. So we should have a mechanism to decide whether the codec is instantiated is on the client or on the server to induce this behaviour.
          Stripping tags is the simplest of the options, but performance was a major concern. Infact in the tags patch there was a proposal to attach tags as in memory object in KV rather than byte array. That would mean stripping tags would have been easier.

          Show
          ramkrishna.s.vasudevan added a comment - As Anoop says, even in Codec negotiation HBASE-9681 , the problem is same.. in the sense any codec we write should behave differently when it works from client side and from the server side. Atleast in terms of tags. So we should have a mechanism to decide whether the codec is instantiated is on the client or on the server to induce this behaviour. Stripping tags is the simplest of the options, but performance was a major concern. Infact in the tags patch there was a proposal to attach tags as in memory object in KV rather than byte array. That would mean stripping tags would have been easier.
          Hide
          stack added a comment -

          Another suggestion atleast to avoid changes to the codec part is to have an init() in the Codec.java. So once the codec is instantiated we could set this flag as true or false based on client or server.

          Anoop fed me the above off line. It just seems wrong that codec need know if 'server' or 'client'. Why can't it be TagsCodec and StripTagsCodec and then at the various junctions (client sending, server receiving, WAL writing, etc.) they read configuration what Codec to use or what code to use Decoding a particular Encoder; e.g. on server, we'd write back to the client using NoTagsKVCodec.

          Pardon me if I am making suggestion you fellas have already said won't work.

          The major concern with that was we will have to recreate KVs (In filter/cp) and byte array copying. The perf penalty is a major concern

          Are we writing new KVs or creating a cell block? If the latter, then it'll be no more expensive copying a KV with or without the Tags?

          To get Andrew his RC the sooner, will life be easier if no tags from server to client? In a later HBase we can add codec negotiation, etc?

          Good stuff lads.

          Show
          stack added a comment - Another suggestion atleast to avoid changes to the codec part is to have an init() in the Codec.java. So once the codec is instantiated we could set this flag as true or false based on client or server. Anoop fed me the above off line. It just seems wrong that codec need know if 'server' or 'client'. Why can't it be TagsCodec and StripTagsCodec and then at the various junctions (client sending, server receiving, WAL writing, etc.) they read configuration what Codec to use or what code to use Decoding a particular Encoder; e.g. on server, we'd write back to the client using NoTagsKVCodec. Pardon me if I am making suggestion you fellas have already said won't work. The major concern with that was we will have to recreate KVs (In filter/cp) and byte array copying. The perf penalty is a major concern Are we writing new KVs or creating a cell block? If the latter, then it'll be no more expensive copying a KV with or without the Tags? To get Andrew his RC the sooner, will life be easier if no tags from server to client? In a later HBase we can add codec negotiation, etc? Good stuff lads.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Anoop fed me the above off line. It just seems wrong that codec need know if 'server' or 'client'. Why can't it be TagsCodec and StripTagsCodec and then at the various junctions (client sending, server receiving, WAL writing, etc.) they read configuration what Codec to use or what code to use Decoding a particular Encoder; e.g. on server, we'd write back to the client using NoTagsKVCodec.

          True.. But this I think has to happen with codec negotiation. Only then the server and the client will know about each other what the other is using. TagsCodec and StripTagsCodec has to be specified in a configuration (which I call it as a mapping - Anoop does not like that ) and use that on either side.
          May be we can see how costly is stripping out the tags from every kv in the CPs.. we can benchmark it once?

          Show
          ramkrishna.s.vasudevan added a comment - Anoop fed me the above off line. It just seems wrong that codec need know if 'server' or 'client'. Why can't it be TagsCodec and StripTagsCodec and then at the various junctions (client sending, server receiving, WAL writing, etc.) they read configuration what Codec to use or what code to use Decoding a particular Encoder; e.g. on server, we'd write back to the client using NoTagsKVCodec. True.. But this I think has to happen with codec negotiation. Only then the server and the client will know about each other what the other is using. TagsCodec and StripTagsCodec has to be specified in a configuration (which I call it as a mapping - Anoop does not like that ) and use that on either side. May be we can see how costly is stripping out the tags from every kv in the CPs.. we can benchmark it once?
          Hide
          stack added a comment -

          Well, for 0.98, we could even hardcode it given we have one Codec only at this time?

          Show
          stack added a comment - Well, for 0.98, we could even hardcode it given we have one Codec only at this time?
          Hide
          ramkrishna.s.vasudevan added a comment -

          Ok.. So considering KVCodec as the default - we will create StripTagKVCodec and on the server side we would instantiate the StripTagKVCodec and keep using it and on client it would be KVCodec. So export tool will not be working with this. Correct?
          Anoop Sam John,Andrew Purtell
          Thoughts?

          Show
          ramkrishna.s.vasudevan added a comment - Ok.. So considering KVCodec as the default - we will create StripTagKVCodec and on the server side we would instantiate the StripTagKVCodec and keep using it and on client it would be KVCodec. So export tool will not be working with this. Correct? Anoop Sam John , Andrew Purtell Thoughts?
          Hide
          Andrew Purtell added a comment -

          So export tool will not be working with this. Correct?

          No, that is not what I said.

          I said: The rock bottom simplest way to do this is to just not support tags in RPC codecs. Maybe we can have a separate class that keeps them for the Export tool specifically? Import is no problem if the user, presumably privileged, is building HFiles and therefore the cells within them directly. Accumulo has the same approach to whole file imports - no checking done, YMMV.

          Show
          Andrew Purtell added a comment - So export tool will not be working with this. Correct? No, that is not what I said. I said: The rock bottom simplest way to do this is to just not support tags in RPC codecs. Maybe we can have a separate class that keeps them for the Export tool specifically? Import is no problem if the user, presumably privileged, is building HFiles and therefore the cells within them directly. Accumulo has the same approach to whole file imports - no checking done, YMMV.
          Hide
          Andrew Purtell added a comment -

          Are we writing new KVs or creating a cell block? If the latter, then it'll be no more expensive copying a KV with or without the Tags?

          This is my assumption too. Why is this wrong.

          Show
          Andrew Purtell added a comment - Are we writing new KVs or creating a cell block? If the latter, then it'll be no more expensive copying a KV with or without the Tags? This is my assumption too. Why is this wrong.
          Hide
          Andrew Purtell added a comment - - edited

          Finally, the reason I say "rock bottom simplest way" is there is too much discussion on this issue and it is holding up the RC essentially. Talk of negotiation is totally out of scope at this point. Let's move this over to reviewboard so we have code to get us all on the same page.

          Show
          Andrew Purtell added a comment - - edited Finally, the reason I say "rock bottom simplest way" is there is too much discussion on this issue and it is holding up the RC essentially. Talk of negotiation is totally out of scope at this point. Let's move this over to reviewboard so we have code to get us all on the same page.
          Hide
          Anoop Sam John added a comment -

          Let us try doing it by removing tags in cp/filter way. Patch coming soon.

          Show
          Anoop Sam John added a comment - Let us try doing it by removing tags in cp/filter way. Patch coming soon.
          Hide
          Anoop Sam John added a comment -

          V2 version is doing the visibility tag strip at VisibilityFilter level.
          Andrew Purtell If this looks fine same to be done for AccessController case also. Will be you able to help me with that pls?

          Show
          Anoop Sam John added a comment - V2 version is doing the visibility tag strip at VisibilityFilter level. Andrew Purtell If this looks fine same to be done for AccessController case also. Will be you able to help me with that pls?
          Hide
          Andrew Purtell added a comment - - edited

          So for every cell we have to make a decision and at least copy everything once? I see a lot of added .toByteArray()s in the patch. Is that a copy on each invocation? This feels like it will be too expensive. Have you benchmarked how much throughput this takes off a LoadTestTool run?

          How about: Make the decision once, with no additional copies. Subclass the codec class. Unconditionally strip tags for RPC. Don't for WAL and HFile. I guess we don't support tags with the Export tool until we can handle this better.

          Show
          Andrew Purtell added a comment - - edited So for every cell we have to make a decision and at least copy everything once? I see a lot of added .toByteArray()s in the patch. Is that a copy on each invocation? This feels like it will be too expensive. Have you benchmarked how much throughput this takes off a LoadTestTool run? How about: Make the decision once, with no additional copies. Subclass the codec class. Unconditionally strip tags for RPC. Don't for WAL and HFile. I guess we don't support tags with the Export tool until we can handle this better.
          Hide
          Hadoop QA added a comment -

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

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

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

          +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

          +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 patch appears to cause mvn site goal to fail.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12623185/HBASE-10322_V2.patch against trunk revision . ATTACHMENT ID: 12623185 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 9 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +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 patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8437//console This message is automatically generated.
          Hide
          Andrew Purtell added a comment -

          Sorry, I need to -1 the current patch out of concerns about the doubling (at least) of data copies. We need to go the route of different codecs, codecs which do not make context specific decisions, codecs which don't do additional copying. Then choose one codec for RPC which strips tags. Choose another codec for WAL and HFile which does not strip tags. I guess it's ok to not support the Export tool for now since we need to reach a conclusion here.

          Show
          Andrew Purtell added a comment - Sorry, I need to -1 the current patch out of concerns about the doubling (at least) of data copies. We need to go the route of different codecs, codecs which do not make context specific decisions, codecs which don't do additional copying. Then choose one codec for RPC which strips tags. Choose another codec for WAL and HFile which does not strip tags. I guess it's ok to not support the Export tool for now since we need to reach a conclusion here.
          Hide
          Anoop Sam John added a comment -

          Because of the perf penalty I am also not so in favor of this way..

          Show
          Anoop Sam John added a comment - Because of the perf penalty I am also not so in favor of this way..
          Hide
          Andrew Purtell added a comment -

          There are three contexts where we serialize cells: RPC, WAL, and HFile. Support tags in serialization for WAL and HFile. Don't for RPC. RPC is both client and server side, that's why that distinction is meaningless (to me).

          Show
          Andrew Purtell added a comment - There are three contexts where we serialize cells: RPC, WAL, and HFile. Support tags in serialization for WAL and HFile. Don't for RPC. RPC is both client and server side, that's why that distinction is meaningless (to me).
          Hide
          Anoop Sam John added a comment -

          After lot of discussions here btw me, Andy and Ram and trying diff approaches, here is what we think is a possible solution. (Considering we have to have the RC soon and this is a blocker for that)
          We will not write tags over RPC. * ie. in both client -> Server and server -> client.*
          So KVCodec will not write tags at all
          CelCodec and MessageCodec already not handling tags.
          HBASE-10321 added a new CellCodecV2 which is a CellCodec with Tags support. We will remove it
          Make sure WALCodec can write tags.

          So tags are becoming a server only thing as of now. Because of this we will remove the new APIs added to Put as part of Tags jira
          Put#add(byte[] family, byte [] qualifier, byte [] value, Tag[] tag)
          And some more related which handles tags. User can not pass tags directly from client. As of now the only way is like what Visibility labels or ACL do. Pass the tags or related info via op attr and make a CP to handle it and add as Tags at server side only.

          Later we can add support for passing tags directly via Put from client

          Thanks a lot Andrew Purtell ,ramkrishna.s.vasudevan ..
          Concerns, suggestions pls.

          Show
          Anoop Sam John added a comment - After lot of discussions here btw me, Andy and Ram and trying diff approaches, here is what we think is a possible solution. (Considering we have to have the RC soon and this is a blocker for that) We will not write tags over RPC. * ie. in both client -> Server and server -> client.* So KVCodec will not write tags at all CelCodec and MessageCodec already not handling tags. HBASE-10321 added a new CellCodecV2 which is a CellCodec with Tags support. We will remove it Make sure WALCodec can write tags. So tags are becoming a server only thing as of now. Because of this we will remove the new APIs added to Put as part of Tags jira Put#add(byte[] family, byte [] qualifier, byte [] value, Tag[] tag) And some more related which handles tags. User can not pass tags directly from client. As of now the only way is like what Visibility labels or ACL do. Pass the tags or related info via op attr and make a CP to handle it and add as Tags at server side only. Later we can add support for passing tags directly via Put from client Thanks a lot Andrew Purtell , ramkrishna.s.vasudevan .. Concerns, suggestions pls.
          Hide
          Andrew Purtell added a comment -

          Agree.

          The goal is to prevent sensitive tags from going to clients who are not supposed to see them. This is blocking the 0.98 RC.

          Checking on a per cell basis is going to hurt performance. Massaging cell data on a per cell basis e.g. copying, will kill performance. We need some global decision per connection.

          Earlier we explored per-connection negotiation ideas on another JIRA but didn't come to a satisfactory resolution.

          Now we want to do the simplest thing possible. There is no need to handle tags in cell serialization for RPC. (Except! Replication! - Thanks Anoop Sam John.) Cell ACLs and visibility expressions are shipped server side in operation attributes. Tag persistence with HFile v3 is all set. Tag persistence in the WAL uses "WAL codecs" which are only applied server side.

          We need an answer for replication though. My thinking is since we set up RPC for replication specially in the sink and source code, and replication is a server to server thing - or at least we can say replication is "privileged" - it should be ok to add a tag capable codec for replication, but have it not be the default. We can tell users that replication will be compatible between 0.96 and 0.98 as long as you don't use cell tags. If you do start using the 0.98 features which require cell tags, then your replication endpoints must all be upgrade to 0.98 first, and you must change a configuration setting.

          Show
          Andrew Purtell added a comment - Agree. The goal is to prevent sensitive tags from going to clients who are not supposed to see them. This is blocking the 0.98 RC. Checking on a per cell basis is going to hurt performance. Massaging cell data on a per cell basis e.g. copying, will kill performance. We need some global decision per connection. Earlier we explored per-connection negotiation ideas on another JIRA but didn't come to a satisfactory resolution. Now we want to do the simplest thing possible. There is no need to handle tags in cell serialization for RPC. (Except! Replication! - Thanks Anoop Sam John .) Cell ACLs and visibility expressions are shipped server side in operation attributes. Tag persistence with HFile v3 is all set. Tag persistence in the WAL uses "WAL codecs" which are only applied server side. We need an answer for replication though. My thinking is since we set up RPC for replication specially in the sink and source code, and replication is a server to server thing - or at least we can say replication is "privileged" - it should be ok to add a tag capable codec for replication, but have it not be the default. We can tell users that replication will be compatible between 0.96 and 0.98 as long as you don't use cell tags. If you do start using the 0.98 features which require cell tags, then your replication endpoints must all be upgrade to 0.98 first, and you must change a configuration setting.
          Hide
          Andrew Purtell added a comment -
          Show
          Andrew Purtell added a comment - Ping stack , Jean-Daniel Cryans , ramkrishna.s.vasudevan , see ^^^
          Hide
          stack added a comment -

          Sounds reasonable Andrew, what you say about replication (Jean-Daniel Cryans is not around – he is off in exotic locations.... again!).

          Is there a patch to review yet?

          Show
          stack added a comment - Sounds reasonable Andrew, what you say about replication ( Jean-Daniel Cryans is not around – he is off in exotic locations.... again!). Is there a patch to review yet?
          Hide
          Andrew Purtell added a comment -

          Is there a patch to review yet?

          There will be something shortly after we agree this is the way to go. Sounds like it. Soon, then.

          Show
          Andrew Purtell added a comment - Is there a patch to review yet? There will be something shortly after we agree this is the way to go. Sounds like it. Soon, then.
          Hide
          Anoop Sam John added a comment - - edited

          My patch is like almost ready except Replication!
          ReplicationSource uses AdminService#ReplicateWALEntry() and call HRS#replicateWALEntry() via the same path of client to HRS communication.. ie via RpcClient.. So if we have to introduce 2 configs some sort of if checks etc will come which tells abt the context etc..
          Another simple way with out any code change I am thinking now is as follows:

          RpcClient looks at "hbase.client.rpc.codec" for getting the Codec class name.
          Well HBase client side looks at this config at client end and uses that Codec to write to server and also writes the codec class name in connection header. Server looks at the connection header to see the class name of the Codec which is used by the client and it instantiate that and uses the same to communicate with that client.
          Here we can do one thing. We can ask the user to configure two class names (When some one uses tags) at client and server.
          Say at client use KVCodec and at server configure it as KVCodecWithTags. Let KVCodecWithTags serialize tags also.At the peer cluster HRS also this codec class should be there and there, in connection header, the class name will be KVCodecWithTags and will use that

          This will work well. So for existing users there is no need to do any changes. When users use the tags (or visibility labels or cell level acl) they must make this change and make sure the 98 upgrade is done in all the clusters.

          Opinions pls.

          Edit : ReplicationSource already refers to this config "hbase.client.rpc.codec" as it uses RpcClient to talk with peer.

          Show
          Anoop Sam John added a comment - - edited My patch is like almost ready except Replication! ReplicationSource uses AdminService#ReplicateWALEntry() and call HRS#replicateWALEntry() via the same path of client to HRS communication.. ie via RpcClient.. So if we have to introduce 2 configs some sort of if checks etc will come which tells abt the context etc.. Another simple way with out any code change I am thinking now is as follows: RpcClient looks at "hbase.client.rpc.codec" for getting the Codec class name. Well HBase client side looks at this config at client end and uses that Codec to write to server and also writes the codec class name in connection header. Server looks at the connection header to see the class name of the Codec which is used by the client and it instantiate that and uses the same to communicate with that client. Here we can do one thing. We can ask the user to configure two class names (When some one uses tags) at client and server. Say at client use KVCodec and at server configure it as KVCodecWithTags. Let KVCodecWithTags serialize tags also.At the peer cluster HRS also this codec class should be there and there, in connection header, the class name will be KVCodecWithTags and will use that This will work well. So for existing users there is no need to do any changes. When users use the tags (or visibility labels or cell level acl) they must make this change and make sure the 98 upgrade is done in all the clusters. Opinions pls. Edit : ReplicationSource already refers to this config "hbase.client.rpc.codec" as it uses RpcClient to talk with peer.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Just attaching a patch where after we support only server side tags, with the above change for the replication side things would work. May be some one more familiar with replication can take a look. Replication testcases passes with this.

          Show
          ramkrishna.s.vasudevan added a comment - Just attaching a patch where after we support only server side tags, with the above change for the replication side things would work. May be some one more familiar with replication can take a look. Replication testcases passes with this.
          Hide
          ramkrishna.s.vasudevan added a comment -

          >>If you do start using the 0.98 features which require cell tags, then your replication endpoints must all be upgrade to 0.98 first, and you must change a configuration setting.
          This would apply there.

          Show
          ramkrishna.s.vasudevan added a comment - >>If you do start using the 0.98 features which require cell tags, then your replication endpoints must all be upgrade to 0.98 first, and you must change a configuration setting. This would apply there.
          Hide
          Anoop Sam John added a comment -

          How about adding new config hbase.replication.rpc.codec or going with existing config only but asking to configure xxxWithTags codec to be used? Ping stack , Andrew Purtell.. I am fine with any of this.
          1st one needs some code change as Ram attached while the other don't need any code changes.

          Show
          Anoop Sam John added a comment - How about adding new config hbase.replication.rpc.codec or going with existing config only but asking to configure xxxWithTags codec to be used? Ping stack , Andrew Purtell .. I am fine with any of this. 1st one needs some code change as Ram attached while the other don't need any code changes.
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

          +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 introduces the following lines longer than 100:
          + this.conf.set("hbase.client.rpc.codec", this.conf.get("hbase.replication.rpc.codec","org.apache.hadoop.hbase.codec.CellCodecV2"));
          + this.conf.set("hbase.client.rpc.codec", this.conf.get("hbase.replication.rpc.codec","org.apache.hadoop.hbase.codec.CellCodecV2"));

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12623779/HBASE-10322_codec.patch against trunk revision . ATTACHMENT ID: 12623779 +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +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 introduces the following lines longer than 100: + this.conf.set("hbase.client.rpc.codec", this.conf.get("hbase.replication.rpc.codec","org.apache.hadoop.hbase.codec.CellCodecV2")); + this.conf.set("hbase.client.rpc.codec", this.conf.get("hbase.replication.rpc.codec","org.apache.hadoop.hbase.codec.CellCodecV2")); -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.TestAcidGuarantees Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8468//console This message is automatically generated.
          Hide
          stack added a comment -

          It is wrong that replication is in the admin Service and that is in the server Service. Can we make a Replication Service? Would that help?

          Show
          stack added a comment - It is wrong that replication is in the admin Service and that is in the server Service. Can we make a Replication Service? Would that help?
          Hide
          Anoop Sam John added a comment -

          That changes can be in another Jira stack?
          We need any way an RpcClient to talk with peer right?
          So we have 2 options.
          1.Go with current way without any code changes. The RpcClient used by ReplicationSource looks at the config "hbase.client.rpc.codec" to know the codec name and uses that. This defaults to KVCodec. As long as user don't deal with tags directly or indirectly (via usage of cell level ACL/ visibility labels) the current way works good. If tag case comes, user must
          a. Change this config value at HRS side to any of the codec with tags class. (We plan to give a KVCodecWithTag)
          b. Make sure upgrade the RSs in peer clusters also so that the new class added in 98 is available there also.
          2. Introduce a new config name as in latest patch and do change is ReplicationSource to decorate the conf. In the attached patch a new Codec ie. CellCodecV2 is used as default. But I think there should not be any default value for this codec because of the below reason. (Default value should be value of the old config with thats default as KVCodec)
          Suppose the src cluster user is upgrading to 98 (or later versions in future) But the peer is still in 96 .
          When the replication src write using new Codec class, the destination will need the codec class to be present in it also. So this make it necessary for the peer also should be upgraded. What abt rolling upgrade then?
          So even if the new config is there or not, the def codec should not change.

          Out of these 2 options which one you guys prefer? Can give a patch accordingly.

          Show
          Anoop Sam John added a comment - That changes can be in another Jira stack ? We need any way an RpcClient to talk with peer right? So we have 2 options. 1.Go with current way without any code changes. The RpcClient used by ReplicationSource looks at the config "hbase.client.rpc.codec" to know the codec name and uses that. This defaults to KVCodec. As long as user don't deal with tags directly or indirectly (via usage of cell level ACL/ visibility labels) the current way works good. If tag case comes, user must a. Change this config value at HRS side to any of the codec with tags class. (We plan to give a KVCodecWithTag) b. Make sure upgrade the RSs in peer clusters also so that the new class added in 98 is available there also. 2. Introduce a new config name as in latest patch and do change is ReplicationSource to decorate the conf. In the attached patch a new Codec ie. CellCodecV2 is used as default. But I think there should not be any default value for this codec because of the below reason. (Default value should be value of the old config with thats default as KVCodec) Suppose the src cluster user is upgrading to 98 (or later versions in future) But the peer is still in 96 . When the replication src write using new Codec class, the destination will need the codec class to be present in it also. So this make it necessary for the peer also should be upgraded. What abt rolling upgrade then? So even if the new config is there or not, the def codec should not change. Out of these 2 options which one you guys prefer? Can give a patch accordingly.
          Hide
          Lars Hofhansl added a comment -

          I am a bit late to this party. The visibility tags control what a client can see, right?

          Then what's a "client"? A client is outside of the HBase cluster outside of HBase's control. So HFile, HLog are not a client. Replication is also not a client. Export is a client, just like any other Java/Thrift/MR/etc client.
          As Andy points out the interesting part here are these real clients.

          Are the tags themselves (i.e. who sees what) more sensitive than the data that can be accessed.
          I.e. if I can see a certain KV, should I be able to see its visibility tags?

          • If the answer is "yes", this is an easy problem in principle and squarely in the hands of an HBase admin to setup access correctly. You just run Export/etc as a user with sufficient access and all problems just go away.
          • If the answer is "no" it gets murky quickly. Now all tools and access paths need to be considered individually.

          Maybe we can even have a tag that controls the visibility of the tags? Generally anything that we hardware assumes something about desired behavior that might not be the same at every institution.

          Show
          Lars Hofhansl added a comment - I am a bit late to this party. The visibility tags control what a client can see, right? Then what's a "client"? A client is outside of the HBase cluster outside of HBase's control. So HFile, HLog are not a client. Replication is also not a client. Export is a client, just like any other Java/Thrift/MR/etc client. As Andy points out the interesting part here are these real clients. Are the tags themselves (i.e. who sees what) more sensitive than the data that can be accessed. I.e. if I can see a certain KV, should I be able to see its visibility tags? If the answer is "yes", this is an easy problem in principle and squarely in the hands of an HBase admin to setup access correctly. You just run Export/etc as a user with sufficient access and all problems just go away. If the answer is "no" it gets murky quickly. Now all tools and access paths need to be considered individually. Maybe we can even have a tag that controls the visibility of the tags? Generally anything that we hardware assumes something about desired behavior that might not be the same at every institution.
          Hide
          ramkrishna.s.vasudevan added a comment -

          >>bq.CellCodecV2 is used as default. But I think there should not be any default value for this codec because of the below reason. (Default value should be value of the old config with thats default as KVCodec)
          Suppose the src cluster user is upgrading to 98 (or later versions in future) But the peer is still in 96 .
          I agree. the patch's intention was to show how we could do that config settings. +1
          Lars Hofhansl

          if I can see a certain KV, should I be able to see its visibility tags?

          What you say is right in the sense admin sets up proper access control say for User A and User A would be seeing only those KVs which has visibility labels that A is associated with. But sometimes the labels can be combination of visibility labels seperted by &,|,!. In that case the User A on reading the visibility labels would come to know about the existence of other labels. And added to that, the whole idea of associating the labels and users are done by admin with super user privileges. So allowing all users to view the labels in the KV would break it because reading the kv the User A would come to know what combination of labels he can pass to access the kvs to which he would not be authorised to.

          Show
          ramkrishna.s.vasudevan added a comment - >>bq.CellCodecV2 is used as default. But I think there should not be any default value for this codec because of the below reason. (Default value should be value of the old config with thats default as KVCodec) Suppose the src cluster user is upgrading to 98 (or later versions in future) But the peer is still in 96 . I agree. the patch's intention was to show how we could do that config settings. +1 Lars Hofhansl if I can see a certain KV, should I be able to see its visibility tags? What you say is right in the sense admin sets up proper access control say for User A and User A would be seeing only those KVs which has visibility labels that A is associated with. But sometimes the labels can be combination of visibility labels seperted by &,|,!. In that case the User A on reading the visibility labels would come to know about the existence of other labels. And added to that, the whole idea of associating the labels and users are done by admin with super user privileges. So allowing all users to view the labels in the KV would break it because reading the kv the User A would come to know what combination of labels he can pass to access the kvs to which he would not be authorised to.
          Hide
          stack added a comment -

          We need any way an RpcClient to talk with peer right?

          Yes, but I thought that if the Service is the explicit Replication Service, then you could identify the context as "replication" and slot in replication suited codecs that preserve tags on setup of the replication connection – if asked for (A codec for replication that is other than what we use for 'normal' client/server seems like something we'd want to have anyways).

          If we break out a replication Service, it will break being able to replicate from a 0.98 to a 0.96 whether or not you are forwarding tags or not. That ain't good. If we leave the service as is, It sounds like we can have a 0.98 replicate to a 0.96 when no tags in the mix. It is only when you enable tags will you have to update the sink cluster so it recognizes the tag-bearing codec.

          Of your 1., and 2., 1. is preferable. Pity it has to be a config. in the hbase-*xml. Can it be a replication config (I suppose this is what your 2. does in part)? Can ship 0.98.0RC as soon as we dump in a codec that can do tags (what happens when you pass a KV with tags to default KVCodec? It just dumps them?)

          I like how Lars Hofhansl is telling it. Does that help lads?

          Show
          stack added a comment - We need any way an RpcClient to talk with peer right? Yes, but I thought that if the Service is the explicit Replication Service, then you could identify the context as "replication" and slot in replication suited codecs that preserve tags on setup of the replication connection – if asked for (A codec for replication that is other than what we use for 'normal' client/server seems like something we'd want to have anyways). If we break out a replication Service, it will break being able to replicate from a 0.98 to a 0.96 whether or not you are forwarding tags or not. That ain't good. If we leave the service as is, It sounds like we can have a 0.98 replicate to a 0.96 when no tags in the mix. It is only when you enable tags will you have to update the sink cluster so it recognizes the tag-bearing codec. Of your 1., and 2., 1. is preferable. Pity it has to be a config. in the hbase-*xml. Can it be a replication config (I suppose this is what your 2. does in part)? Can ship 0.98.0RC as soon as we dump in a codec that can do tags (what happens when you pass a KV with tags to default KVCodec? It just dumps them?) I like how Lars Hofhansl is telling it. Does that help lads?
          Hide
          stack added a comment -

          Just to say that the 'codec ' problem would be true of any sink cluster no matter what the version; you couldn't do some fancy compression codec unless you first updated the sink cluster so it recognized it when the source cluster set up the connection.

          Show
          stack added a comment - Just to say that the 'codec ' problem would be true of any sink cluster no matter what the version; you couldn't do some fancy compression codec unless you first updated the sink cluster so it recognized it when the source cluster set up the connection.
          Hide
          ramkrishna.s.vasudevan added a comment -

          what happens when you pass a KV with tags to default KVCodec? It just dumps them

          No KVCodec by default will not dump tags but when it works with the WALCellCodec it would dump. so we would control it with a flag.
          The reason being kvcodec writes the entire length of the byte array.

          Show
          ramkrishna.s.vasudevan added a comment - what happens when you pass a KV with tags to default KVCodec? It just dumps them No KVCodec by default will not dump tags but when it works with the WALCellCodec it would dump. so we would control it with a flag. The reason being kvcodec writes the entire length of the byte array.
          Hide
          Anoop Sam John added a comment -

          I.e. if I can see a certain KV, should I be able to see its visibility tags?

          The answer to this is No. Ram explain the reason.

          Maybe we can even have a tag that controls the visibility of the tags?

          Who can see the tags also along with KVs, we thought let it be decided by the user. Only HBase super user will be able to get tags also along with KVs.

          So this is the overall idea. Yes tags control what a client can see.. But we would like to control normal clients from seeing the tags.

          The impl of this becomes very tricky as we use same Codec to write from client to server and back. We were giving options for user to add tags in Mutation KVs. As of now we are thinking of removing this APIs. Over RPC tags will not go (ie. client -> server or reverse)
          To write to WAL we use WALCellCodec and that will be able to write and read tags.
          Then the last problem came out was replication for which we propose 2 possible solutions. stack with out big changes like ReplicationServer can we do? I think those we can try addressing in another issue..

          Of your 1., and 2., 1. is preferable. Pity it has to be a config. in the hbase-*xml. Can it be a replication config (I suppose this is what your 2. does in part)?

          This config ("hbase.client.rpc.codec") is used by the RpcClient. RpcClient used by the ReplicationSource. Yes already it refers to the config. As long as user dont deal with tags (existing migrated to 98) no changes required at all.. When they have tags usage have to change this config at HRSs side to some codec with tags.. We will ship some new codecs which can handle tags also along with this issue. Yes option 2 adds a new config and as shown in the patch it just write the value of this new config as old config's value. (Decorating the conf object used by ReplicationSource)
          If you feel option 1 is fine, no extra code will be needed in Replication side.

          Show
          Anoop Sam John added a comment - I.e. if I can see a certain KV, should I be able to see its visibility tags? The answer to this is No. Ram explain the reason. Maybe we can even have a tag that controls the visibility of the tags? Who can see the tags also along with KVs, we thought let it be decided by the user. Only HBase super user will be able to get tags also along with KVs. So this is the overall idea. Yes tags control what a client can see.. But we would like to control normal clients from seeing the tags. The impl of this becomes very tricky as we use same Codec to write from client to server and back. We were giving options for user to add tags in Mutation KVs. As of now we are thinking of removing this APIs. Over RPC tags will not go (ie. client -> server or reverse) To write to WAL we use WALCellCodec and that will be able to write and read tags. Then the last problem came out was replication for which we propose 2 possible solutions. stack with out big changes like ReplicationServer can we do? I think those we can try addressing in another issue.. Of your 1., and 2., 1. is preferable. Pity it has to be a config. in the hbase-*xml. Can it be a replication config (I suppose this is what your 2. does in part)? This config ("hbase.client.rpc.codec") is used by the RpcClient. RpcClient used by the ReplicationSource. Yes already it refers to the config. As long as user dont deal with tags (existing migrated to 98) no changes required at all.. When they have tags usage have to change this config at HRSs side to some codec with tags.. We will ship some new codecs which can handle tags also along with this issue. Yes option 2 adds a new config and as shown in the patch it just write the value of this new config as old config's value. (Decorating the conf object used by ReplicationSource) If you feel option 1 is fine, no extra code will be needed in Replication side.
          Hide
          Lars Hofhansl added a comment -

          Thanks for explaining, Ram and Anoop.

          So tags are becoming a server only thing as of now

          Agree. Can tackle Export/Copytable/etc later, although I figure eventually these would have to be addressed if folks use them for backup.

          Only HBase super user will be able to get tags also along with KVs.

          This seems to contradict the earlier point.

          Show
          Lars Hofhansl added a comment - Thanks for explaining, Ram and Anoop. So tags are becoming a server only thing as of now Agree. Can tackle Export/Copytable/etc later, although I figure eventually these would have to be addressed if folks use them for backup. Only HBase super user will be able to get tags also along with KVs. This seems to contradict the earlier point.
          Hide
          Anoop Sam John added a comment - - edited

          Sorry for the confusion Lars
          The final idea is that only super user can view tags. But the impl raised some issues and we decided that we will not handle at this time. As of now for 0.98.0 we will make tags as serevr only thing. No user, even super user, will be able to retrieve tags to client side. Am I making it clear now?

          Can tackle Export/Copytable/etc later

          Yes for use cases of Export/Copytable we thought tags should be accessible for clients also. Then thougt might be this can be controlled via user and we can ask Export to be executed by a super user. Then only tags will get exported. Even all the KVs can be surely scanned back only when the super user is executing it. For some other users, some KVs for which he is not authorized with related labels, wont get read.

          Show
          Anoop Sam John added a comment - - edited Sorry for the confusion Lars The final idea is that only super user can view tags. But the impl raised some issues and we decided that we will not handle at this time. As of now for 0.98.0 we will make tags as serevr only thing. No user, even super user, will be able to retrieve tags to client side. Am I making it clear now? Can tackle Export/Copytable/etc later Yes for use cases of Export/Copytable we thought tags should be accessible for clients also. Then thougt might be this can be controlled via user and we can ask Export to be executed by a super user. Then only tags will get exported. Even all the KVs can be surely scanned back only when the super user is executing it. For some other users, some KVs for which he is not authorized with related labels, wont get read.
          Hide
          Andrew Purtell added a comment -

          Let's condense the dense walls of text above to a one line answer to the below question if it is possible:

          Can we have a tag-aware codec that can be configured by only ReplicationSource and ReplicationSink for the RPC they do server-to-server cross site?

          Show
          Andrew Purtell added a comment - Let's condense the dense walls of text above to a one line answer to the below question if it is possible: Can we have a tag-aware codec that can be configured by only ReplicationSource and ReplicationSink for the RPC they do server-to-server cross site?
          Hide
          Andrew Purtell added a comment -

          Lars Hofhansl, stack, Anoop Sam John, ramkrishna.s.vasudevan: Quick recap as I see it. Security tags can be more sensitive than the cell itself. Users will share the cells among each other. However, we don't want that sharing to also leak access rules for the cell. That would be at best a violation of "need to know". Also, 0.96 clients can't handle serializations that include tags. The easiest answer is: RPC does not handle cell tags. We can thus avoid: negotiation, per-cell access checks, per-cell rewrites (copies). However, that fails to address replication, which uses the RPC code but must be able to replicate tags from a 0.98 source to another 0.98 sink. For replication, we need to hand RPC a codec that is tag aware. Because 0.98 may be talking to 0.96, we can't do that by default, we need a configuration setting for replication that tells it what RPC codec to select when talking to the peer.

          Show
          Andrew Purtell added a comment - Lars Hofhansl , stack , Anoop Sam John , ramkrishna.s.vasudevan : Quick recap as I see it. Security tags can be more sensitive than the cell itself. Users will share the cells among each other. However, we don't want that sharing to also leak access rules for the cell. That would be at best a violation of "need to know". Also, 0.96 clients can't handle serializations that include tags. The easiest answer is: RPC does not handle cell tags. We can thus avoid: negotiation, per-cell access checks, per-cell rewrites (copies). However, that fails to address replication, which uses the RPC code but must be able to replicate tags from a 0.98 source to another 0.98 sink. For replication, we need to hand RPC a codec that is tag aware. Because 0.98 may be talking to 0.96, we can't do that by default, we need a configuration setting for replication that tells it what RPC codec to select when talking to the peer.
          Hide
          Andrew Purtell added a comment -

          Lars Hofhansl: Instead of Export, make a snapshot and DistCp the HFiles. Instead of Import, use the bulk import facility.

          Show
          Andrew Purtell added a comment - Lars Hofhansl : Instead of Export, make a snapshot and DistCp the HFiles. Instead of Import, use the bulk import facility.
          Hide
          Lars Hofhansl added a comment -

          Good summary. (And I'm really just a bystander here.)
          It's simplest to keep tag server-only unless there is a compelling argument to do differently.
          A compelling argument might eventually be that code outside of HBase needs to check/manipulate tags.

          Show
          Lars Hofhansl added a comment - Good summary. (And I'm really just a bystander here.) It's simplest to keep tag server-only unless there is a compelling argument to do differently. A compelling argument might eventually be that code outside of HBase needs to check/manipulate tags.
          Hide
          stack added a comment -

          Nice distillation. So, to 'solve' this issue for 0.98.0RC, we just need to figure a means of allowing a user insert a particular codec when the client is replicating. It does not even have to be 'on' in 0.98.0..... in fact it is better if is not 'on'. It just needs to be possible. Right?

          Show
          stack added a comment - Nice distillation. So, to 'solve' this issue for 0.98.0RC, we just need to figure a means of allowing a user insert a particular codec when the client is replicating. It does not even have to be 'on' in 0.98.0..... in fact it is better if is not 'on'. It just needs to be possible. Right?
          Hide
          Andrew Purtell added a comment -

          So, to 'solve' this issue for 0.98.0RC, we just need to figure a means of allowing a user insert a particular codec when the client is replicating. It does not even have to be 'on' in 0.98.0..... in fact it is better if is not 'on'. It just needs to be possible. Right?

          Yes.

          One more configuration variable (yeah...) named "hbase.replication.rpc.codec" or some such, so the tag-aware codec can be separately set up by the source and sink. Defaulting to the same codec we are using for RPC to be compatible with 0.96 clients.

          Show
          Andrew Purtell added a comment - So, to 'solve' this issue for 0.98.0RC, we just need to figure a means of allowing a user insert a particular codec when the client is replicating. It does not even have to be 'on' in 0.98.0..... in fact it is better if is not 'on'. It just needs to be possible. Right? Yes. One more configuration variable (yeah...) named "hbase.replication.rpc.codec" or some such, so the tag-aware codec can be separately set up by the source and sink. Defaulting to the same codec we are using for RPC to be compatible with 0.96 clients.
          Hide
          Andrew Purtell added a comment -

          A compelling argument might eventually be that code outside of HBase needs to check/manipulate tags.

          This will be possible after my proposed change on this issue. Such code can directly build HFiles (v3) containing tags, and submit them through the bulk import facility. Likewise, if you copy out HFiles (v3) from a snapshot, they will come over with tags included, which can be read by accessing the HFile directly using the low level scanners. The security story is acceptable. Accumulo has a similar hands-off approach to labels in bulk imported files, see http://accumulo.apache.org/1.5/accumulo_user_manual.html#_security: "This constraint is not applied to bulk imported data, if this a concern then disable the bulk import permission." Also we can trivially prevent unauthorized direct access to HFiles by enabling encryption (HBASE-7544).

          Show
          Andrew Purtell added a comment - A compelling argument might eventually be that code outside of HBase needs to check/manipulate tags. This will be possible after my proposed change on this issue. Such code can directly build HFiles (v3) containing tags, and submit them through the bulk import facility. Likewise, if you copy out HFiles (v3) from a snapshot, they will come over with tags included, which can be read by accessing the HFile directly using the low level scanners. The security story is acceptable. Accumulo has a similar hands-off approach to labels in bulk imported files, see http://accumulo.apache.org/1.5/accumulo_user_manual.html#_security: "This constraint is not applied to bulk imported data, if this a concern then disable the bulk import permission." Also we can trivially prevent unauthorized direct access to HFiles by enabling encryption ( HBASE-7544 ).
          Hide
          Anoop Sam John added a comment -

          It does not even have to be 'on' in 0.98.0..... in fact it is better if is not 'on'. It just needs to be possible.

          +1 for that. The interface Codec is marked InterfaceAudience.Private . Do we have to change this marking now?

          Show
          Anoop Sam John added a comment - It does not even have to be 'on' in 0.98.0..... in fact it is better if is not 'on'. It just needs to be possible. +1 for that. The interface Codec is marked InterfaceAudience.Private . Do we have to change this marking now?
          Hide
          ramkrishna.s.vasudevan added a comment -

          +1 on the entire summary.

          +1 for that. The interface Codec is marked InterfaceAudience.Private . Do we have to change this marking now?

          Anoop Sam John
          I think Stack meant it only in the replication context? May be am not getting the intent correctly.

          Show
          ramkrishna.s.vasudevan added a comment - +1 on the entire summary. +1 for that. The interface Codec is marked InterfaceAudience.Private . Do we have to change this marking now? Anoop Sam John I think Stack meant it only in the replication context? May be am not getting the intent correctly.
          Hide
          stack added a comment -

          The interface Codec is marked InterfaceAudience.Private . Do we have to change this marking now?

          I dont follow you Anoop. The Codec remains as is. You just need to be able to configure the replication source to use a codec that ships tags (do you have one?) rather than the default no tags (and the default for replication should be no tags so it can replicate from 0.98 to 0.96).

          Show
          stack added a comment - The interface Codec is marked InterfaceAudience.Private . Do we have to change this marking now? I dont follow you Anoop. The Codec remains as is. You just need to be able to configure the replication source to use a codec that ships tags (do you have one?) rather than the default no tags (and the default for replication should be no tags so it can replicate from 0.98 to 0.96).
          Hide
          Anoop Sam John added a comment -

          Got it Stack. We will provide codec that can handle tag.

          Show
          Anoop Sam John added a comment - Got it Stack. We will provide codec that can handle tag.
          Hide
          Anoop Sam John added a comment -

          Changes
          1. Not serializing the tags over RPC. KeyValueCodec ignores tags
          2. Added a codec KeyValueCodecWithTags which can serialize tags also
          3. Renamed CellCodecV2 to CellCodecWithTags to be consistent with KeyValueCodecWithTags
          4. Adding a new config "hbase.replication.rpc.codec" Using which the codec class name to be used by replication can be specified.

          Show
          Anoop Sam John added a comment - Changes 1. Not serializing the tags over RPC. KeyValueCodec ignores tags 2. Added a codec KeyValueCodecWithTags which can serialize tags also 3. Renamed CellCodecV2 to CellCodecWithTags to be consistent with KeyValueCodecWithTags 4. Adding a new config "hbase.replication.rpc.codec" Using which the codec class name to be used by replication can be specified.
          Hide
          stack added a comment -

          If we remove tags from Put, how do tags make it into the system at all?

          Patch seems fine. I see where the replication codec is being inserted. For sure we know this works? We haven't forgot anything? There is no test for it.

          Good stuff lads.

          Show
          stack added a comment - If we remove tags from Put, how do tags make it into the system at all? Patch seems fine. I see where the replication codec is being inserted. For sure we know this works? We haven't forgot anything? There is no test for it. Good stuff lads.
          Hide
          ramkrishna.s.vasudevan added a comment -

          If we remove tags from Put, how do tags make it into the system at all?

          The agreement is that 0.98.0 will not support tags from client side. all tags will be server side only. As ACL and Visibility does tags will be passed via Operation Attributes and the respective CPs will take care.
          If really user needs to add tags, then he has to write his own CP, pass tags via Operation Attributes and use it. (This is ideally not easy for the user), but as we have this stripping tags from server to clients using codec, we have finalised on this. Thoughts? Stack

          Show
          ramkrishna.s.vasudevan added a comment - If we remove tags from Put, how do tags make it into the system at all? The agreement is that 0.98.0 will not support tags from client side. all tags will be server side only. As ACL and Visibility does tags will be passed via Operation Attributes and the respective CPs will take care. If really user needs to add tags, then he has to write his own CP, pass tags via Operation Attributes and use it. (This is ideally not easy for the user), but as we have this stripping tags from server to clients using codec, we have finalised on this. Thoughts? Stack
          Hide
          ramkrishna.s.vasudevan added a comment -

          I am +1 on patch. KeyValueCodecWithTags and CellcodecwithTags both would be needed right? I think better. so +1.

          Show
          ramkrishna.s.vasudevan added a comment - I am +1 on patch. KeyValueCodecWithTags and CellcodecwithTags both would be needed right? I think better. so +1.
          Hide
          Hadoop QA added a comment -

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

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

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

          +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

          -1 javadoc. The javadoc tool appears to have generated 3 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 patch appears to cause mvn site goal to fail.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12624069/HBASE-10322_V3.patch against trunk revision . ATTACHMENT ID: 12624069 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 21 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. -1 javadoc . The javadoc tool appears to have generated 3 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 patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8478//console This message is automatically generated.
          Hide
          stack added a comment -

          OK. So CPs add tags for now. Good.

          How we know replication will pick up the codec config and work? You fellas tried it?

          Show
          stack added a comment - OK. So CPs add tags for now. Good. How we know replication will pick up the codec config and work? You fellas tried it?
          Hide
          ramkrishna.s.vasudevan added a comment -

          I tried it atleast with testcases. That is why added it in Replication source and replication sink. If any replication experts would see that, it would be better.
          Can test in a small cluster too.

          Show
          ramkrishna.s.vasudevan added a comment - I tried it atleast with testcases. That is why added it in Replication source and replication sink. If any replication experts would see that, it would be better. Can test in a small cluster too.
          Hide
          stack added a comment -

          I'd suggest you test in small cluster.

          I'd be +1 on committing this in meantime.

          Andrew can make the call on whether to wait on test completion before RC'ing or not.

          Show
          stack added a comment - I'd suggest you test in small cluster. I'd be +1 on committing this in meantime. Andrew can make the call on whether to wait on test completion before RC'ing or not.
          Hide
          Anoop Sam John added a comment -

          Let me try adding a test Stack.
          I have to write it as a separate test class which spins 2 clusters. I have to set the replication codec class to KVCodecWithTags and add a CP to handle the tags as we don't support tags from client to server. Don't want existing tests to get these changes. Is that fine?

          Show
          Anoop Sam John added a comment - Let me try adding a test Stack. I have to write it as a separate test class which spins 2 clusters. I have to set the replication codec class to KVCodecWithTags and add a CP to handle the tags as we don't support tags from client to server. Don't want existing tests to get these changes. Is that fine?
          Hide
          stack added a comment -

          Anoop Sam John Whatever you think boss. It'd just be good if we knew things basically worked here before we cut RC.

          Show
          stack added a comment - Anoop Sam John Whatever you think boss. It'd just be good if we knew things basically worked here before we cut RC.
          Hide
          Hadoop QA added a comment -

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

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

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

          +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

          -1 javadoc. The javadoc tool appears to have generated 3 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 patch appears to cause mvn site goal to fail.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12624103/HBASE-10322_V4.patch against trunk revision . ATTACHMENT ID: 12624103 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 23 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. -1 javadoc . The javadoc tool appears to have generated 3 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 patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8481//console This message is automatically generated.
          Hide
          Anoop Sam John added a comment -

          V5 fixing javadoc warns

          Show
          Anoop Sam John added a comment - V5 fixing javadoc warns
          Hide
          Andrew Purtell added a comment -

          I'd be +1 on committing this in meantime.

          +1 for commit of v5 as is. Thanks for seeing this through Anoop and Ram.

          Please add a release note about HConstants.REPLICATION_CODEC_CONF_KEY.

          Fix the comment above this change on commit:

          diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java
          index a612b18..305a76a 100644
          --- hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java
          +++ hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java
          @@ -1300,7 +1300,7 @@ public class RpcClient {
             Codec getCodec() {
               // For NO CODEC, "hbase.client.rpc.codec" must be the empty string AND
               // "hbase.client.default.rpc.codec" -- because default is to do cell block encoding.
          -    String className = conf.get("hbase.client.rpc.codec", getDefaultCodec(this.conf));
          +    String className = conf.get(HConstants.RPC_CODEC_CONF_KEY, getDefaultCodec(this.conf));
               if (className == null || className.length() == 0) return null;
               try {
                 return (Codec)Class.forName(className).newInstance();
          

          Andrew can make the call on whether to wait on test completion before RC'ing or not.

          I'd really like a test if we get one in. Would have to be a LargeTest given it spins up two clusters. If it proves difficult then we can skip it. Or, if it flakes during RC testing, we can revert it for 0.98.1. Therefore it makes sense to me to do it as a follow up issue.

          Please also update the replication section of the manual to inform the user what HConstants.REPLICATION_CODEC_CONF_KEY does. We also need an update of the section talking about tags that setting HConstants.REPLICATION_CODEC_CONF_KEY to the tags-aware codec is required to replicate tags from one cluster to another.

          stack: The security coprocessors use operation attributes to ship metadata to the server. The downside is you have to take care because all cells bundled in the op will get the same metadata and the server has to rewrite the incoming cells, but the upside is we don't care about any limitations we might have with tags on the client. We can make tags "first class" for 1.0. We will have to look at things like negotiating codecs on the connection at that time.

          Show
          Andrew Purtell added a comment - I'd be +1 on committing this in meantime. +1 for commit of v5 as is. Thanks for seeing this through Anoop and Ram. Please add a release note about HConstants.REPLICATION_CODEC_CONF_KEY. Fix the comment above this change on commit: diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java index a612b18..305a76a 100644 --- hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java +++ hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java @@ -1300,7 +1300,7 @@ public class RpcClient { Codec getCodec() { // For NO CODEC, "hbase.client.rpc.codec" must be the empty string AND // "hbase.client.default.rpc.codec" -- because default is to do cell block encoding. - String className = conf.get("hbase.client.rpc.codec", getDefaultCodec(this.conf)); + String className = conf.get(HConstants.RPC_CODEC_CONF_KEY, getDefaultCodec(this.conf)); if (className == null || className.length() == 0) return null; try { return (Codec)Class.forName(className).newInstance(); Andrew can make the call on whether to wait on test completion before RC'ing or not. I'd really like a test if we get one in. Would have to be a LargeTest given it spins up two clusters. If it proves difficult then we can skip it. Or, if it flakes during RC testing, we can revert it for 0.98.1. Therefore it makes sense to me to do it as a follow up issue. Please also update the replication section of the manual to inform the user what HConstants.REPLICATION_CODEC_CONF_KEY does. We also need an update of the section talking about tags that setting HConstants.REPLICATION_CODEC_CONF_KEY to the tags-aware codec is required to replicate tags from one cluster to another. stack : The security coprocessors use operation attributes to ship metadata to the server. The downside is you have to take care because all cells bundled in the op will get the same metadata and the server has to rewrite the incoming cells, but the upside is we don't care about any limitations we might have with tags on the client. We can make tags "first class" for 1.0. We will have to look at things like negotiating codecs on the connection at that time.
          Hide
          Andrew Purtell added a comment -

          And if making tags "first class" we will have to look at thorny issues like deciding what user is allowed to attach what tags or what user is allowed to see what tags. There's a lot involved with it and I think we will end up punting on it all, but we can save this discussion for the next JIRA

          Show
          Andrew Purtell added a comment - And if making tags "first class" we will have to look at thorny issues like deciding what user is allowed to attach what tags or what user is allowed to see what tags. There's a lot involved with it and I think we will end up punting on it all, but we can save this discussion for the next JIRA
          Hide
          Anoop Sam John added a comment -

          Would have to be a LargeTest given it spins up two clusters. If it proves difficult then we can skip it.

          Pls see TestReplicationWithTags in the patch

          Show
          Anoop Sam John added a comment - Would have to be a LargeTest given it spins up two clusters. If it proves difficult then we can skip it. Pls see TestReplicationWithTags in the patch
          Hide
          Andrew Purtell added a comment - - edited

          Pls see TestReplicationWithTags in the patch

          Yes, can we split this out to a subtask and commit the rest now?
          Edit: Also please see comments above about manual updates. Could be done at the same time.

          Show
          Andrew Purtell added a comment - - edited Pls see TestReplicationWithTags in the patch Yes, can we split this out to a subtask and commit the rest now? Edit: Also please see comments above about manual updates. Could be done at the same time.
          Hide
          Hadoop QA added a comment -

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

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

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

          +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

          +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 patch appears to cause mvn site goal to fail.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12624161/HBASE-10322_V5.patch against trunk revision . ATTACHMENT ID: 12624161 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 23 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +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 patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8483//console This message is automatically generated.
          Hide
          Anoop Sam John added a comment -

          V5 patch - TestReplicationWithTags

          The test and book update will be given in a follow on task soon.

          I will commit V6 in some time unless hear objections. Thanks all for the help.

          Show
          Anoop Sam John added a comment - V5 patch - TestReplicationWithTags The test and book update will be given in a follow on task soon. I will commit V6 in some time unless hear objections. Thanks all for the help.
          Hide
          ramkrishna.s.vasudevan added a comment -

          +1 on V6. Thanks everyone for the active participation and bringing this to a conclusion.

          Show
          ramkrishna.s.vasudevan added a comment - +1 on V6. Thanks everyone for the active participation and bringing this to a conclusion.
          Hide
          Hadoop QA added a comment -

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

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

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

          +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

          +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

          +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 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 patch appears to cause mvn site goal to fail.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12624263/HBASE-10322_V6.patch against trunk revision . ATTACHMENT ID: 12624263 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 21 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +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 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 patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8486//console This message is automatically generated.
          Hide
          Anoop Sam John added a comment -

          Committed to Trunk and 98.
          Will file a jira for the book update and test as per Andy's suggestion.

          Note : There is no difference btw V5 and V6 other than the latter removed a test class. So the Findbugs warns might have come from other commits I guess.

          Show
          Anoop Sam John added a comment - Committed to Trunk and 98. Will file a jira for the book update and test as per Andy's suggestion. Note : There is no difference btw V5 and V6 other than the latter removed a test class. So the Findbugs warns might have come from other commits I guess.
          Hide
          Anoop Sam John added a comment -

          Thanks alot all for helping us to arrive at a conclusion.

          Show
          Anoop Sam John added a comment - Thanks alot all for helping us to arrive at a conclusion.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK #4847 (See https://builds.apache.org/job/HBase-TRUNK/4847/)
          HBASE-10322 Strip tags from KV while sending back to client on reads. (anoopsamjohn: rev 1560265)

          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionKey.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecV2.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecWithTags.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodec.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodecWithTags.java
          • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecV2.java
          • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecWithTags.java
          • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestKeyValueCodecWithTags.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestEncodedSeekers.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/PerformanceEvaluation.java
          Show
          Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #4847 (See https://builds.apache.org/job/HBase-TRUNK/4847/ ) HBASE-10322 Strip tags from KV while sending back to client on reads. (anoopsamjohn: rev 1560265) /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionKey.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecV2.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecWithTags.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodec.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodecWithTags.java /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecV2.java /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecWithTags.java /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestKeyValueCodecWithTags.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestEncodedSeekers.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/PerformanceEvaluation.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in HBase-0.98 #101 (See https://builds.apache.org/job/HBase-0.98/101/)
          HBASE-10322 Strip tags from KV while sending back to client on reads. (anoopsamjohn: rev 1560266)

          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionKey.java
          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java
          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java
          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java
          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecV2.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecWithTags.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodec.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodecWithTags.java
          • /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecV2.java
          • /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecWithTags.java
          • /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestKeyValueCodecWithTags.java
          • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java
          • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java
          • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestEncodedSeekers.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/PerformanceEvaluation.java
          Show
          Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #101 (See https://builds.apache.org/job/HBase-0.98/101/ ) HBASE-10322 Strip tags from KV while sending back to client on reads. (anoopsamjohn: rev 1560266) /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionKey.java /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecV2.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecWithTags.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodec.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodecWithTags.java /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecV2.java /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecWithTags.java /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestKeyValueCodecWithTags.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestEncodedSeekers.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/PerformanceEvaluation.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #95 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/95/)
          HBASE-10322 Strip tags from KV while sending back to client on reads. (anoopsamjohn: rev 1560266)

          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionKey.java
          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java
          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java
          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java
          • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecV2.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecWithTags.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodec.java
          • /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodecWithTags.java
          • /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecV2.java
          • /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecWithTags.java
          • /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestKeyValueCodecWithTags.java
          • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java
          • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java
          • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestEncodedSeekers.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java
          • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/PerformanceEvaluation.java
          Show
          Hudson added a comment - FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #95 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/95/ ) HBASE-10322 Strip tags from KV while sending back to client on reads. (anoopsamjohn: rev 1560266) /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionKey.java /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MultiServerCallable.java /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClient.java /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecV2.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/CellCodecWithTags.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodec.java /hbase/branches/0.98/hbase-common/src/main/java/org/apache/hadoop/hbase/codec/KeyValueCodecWithTags.java /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecV2.java /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestCellCodecWithTags.java /hbase/branches/0.98/hbase-common/src/test/java/org/apache/hadoop/hbase/codec/TestKeyValueCodecWithTags.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCellCodec.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestEncodedSeekers.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestTags.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/PerformanceEvaluation.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in HBase-TRUNK #5313 (See https://builds.apache.org/job/HBase-TRUNK/5313/)
          HBASE-10398 HBase book updates for Replication after HBASE-10322. (Misty) (anoopsamjohn: rev da8f0a336d9a3b516fc1f5d33c462b1ef4996117)

          • src/main/docbkx/book.xml
          • src/main/docbkx/security.xml
          • src/main/docbkx/ops_mgt.xml
          Show
          Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #5313 (See https://builds.apache.org/job/HBase-TRUNK/5313/ ) HBASE-10398 HBase book updates for Replication after HBASE-10322 . (Misty) (anoopsamjohn: rev da8f0a336d9a3b516fc1f5d33c462b1ef4996117) src/main/docbkx/book.xml src/main/docbkx/security.xml src/main/docbkx/ops_mgt.xml
          Hide
          Enis Soztutar added a comment -

          Closing this issue after 0.99.0 release.

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

            People

            • Assignee:
              Anoop Sam John
              Reporter:
              Anoop Sam John
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development