You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/03/17 22:53:29 UTC

[GitHub] [hbase] jojochuang opened a new pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

jojochuang opened a new pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301
 
 
   I've tried a few approaches. It turns out the quickest solution to this is with Java Reflection.
   
   (1) Create a ProtobufDecoder that is inspired by io.netty.handler.codec.protobuf.ProtobufDecoder. The original ProtobufDecoder has dependency on protobuf. Use reflection to access the shaded protobuf in HDFS when applicable.
   
   (2) Similarly, create a private class BuilderPayloadSetter that does ByteString.copyFrom() + DataTransferEncryptorMessageProto.Builder.setPayload().
   
   Manually tested with Hadoop 3.1.2 and 3.3.0-SNAPSHOT (on top of HBASE-23833 and HBASE-23998)
   
   Please let me know if this is the acceptable approach.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] jojochuang commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
jojochuang commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-601848552
 
 
   Thanks @saintstack.
   Updated the PR based on your suggestion. Manually tested with Hadoop 3.3.0-SNAPSHOT and 3.1.2.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394798394
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+
+        if (setPayloadMethod == null) {
+          try {
+            setPayloadMethod = builderClass.getMethod("setPayload", byteStringClass);
 
 Review comment:
   hmm... we can't set this till setter time I suppose? Could we do it at static class loading time?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394798019
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+
+        if (setPayloadMethod == null) {
+          try {
+            setPayloadMethod = builderClass.getMethod("setPayload", byteStringClass);
+          } catch (NoSuchMethodException e) {
+            e.printStackTrace();
+          }
+        }
+
+        try {
+          setPayloadMethod.invoke(builder, byteStringObject);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+      }
+
+      static {
+        builderClass = DataTransferEncryptorMessageProto.Builder.class;
+        byteStringClass = com.google.protobuf.ByteString.class;
 
 Review comment:
   oh. i get it. This is the HDFS ByteString. The one that will be on the CP for all versions before HDFS3.3. Ok.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-611096817
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 18s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 28s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   6m  4s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 26s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 40s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   6m  0s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 16s |  hbase-server: The patch generated 5 new + 1 unchanged - 0 fixed = 6 total (was 1)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  hadoopcheck  |  13m 32s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2.  |
   | +1 :green_heart: |  spotbugs  |   2m 38s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 22s |  The patch does not generate ASF License warnings.  |
   |  |   |  43m 57s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | dupname asflicense xml spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 9970f69d502c 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ed830222da |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 83 (vs. ulimit of 10000) |
   | modules | C: hbase-resource-bundle hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-601892216
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 37s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   6m 26s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 20s |  master passed  |
   | -1 :x: |  shadedjars  |   0m 10s |  branch has 7 errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   6m 10s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 18s |  the patch passed  |
   | -1 :x: |  shadedjars  |   0m 10s |  patch has 7 errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 13s |  hbase-resource-bundle in the patch passed.  |
   | -0 :warning: |  unit  |  79m 29s |  hbase-server in the patch failed.  |
   |  |   | 101m 25s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux d9637e4e3e47 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 080d8645ed |
   | Default Java | 2020-01-14 |
   | shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-jdk11-hadoop3-check/output/branch-shadedjars.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/testReport/ |
   | Max. process+thread count | 6525 (vs. ulimit of 10000) |
   | modules | C: hbase-resource-bundle hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] jojochuang commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r395391730
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+
+        if (setPayloadMethod == null) {
+          try {
+            setPayloadMethod = builderClass.getMethod("setPayload", byteStringClass);
+          } catch (NoSuchMethodException e) {
+            e.printStackTrace();
+          }
+        }
+
+        try {
+          setPayloadMethod.invoke(builder, byteStringObject);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+      }
+
+      static {
+        builderClass = DataTransferEncryptorMessageProto.Builder.class;
+        byteStringClass = com.google.protobuf.ByteString.class;
+        try {
+          byteStringClass = Class.forName("org.apache.hadoop.thirdparty.protobuf.ByteString");
+        } catch (ClassNotFoundException e) {
+          e.printStackTrace();
 
 Review comment:
   Done.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394794024
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
 
 Review comment:
   Smile. These gymastics need big comment here to explain magic going on here. Smile.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394797478
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+
+        if (setPayloadMethod == null) {
+          try {
+            setPayloadMethod = builderClass.getMethod("setPayload", byteStringClass);
+          } catch (NoSuchMethodException e) {
+            e.printStackTrace();
+          }
+        }
+
+        try {
+          setPayloadMethod.invoke(builder, byteStringObject);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+      }
+
+      static {
+        builderClass = DataTransferEncryptorMessageProto.Builder.class;
+        byteStringClass = com.google.protobuf.ByteString.class;
 
 Review comment:
   Is this ok? This presumes what pb is on the CLASSPATH? pb2.5?  Should it be the shaded hbase ByteString?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394798195
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
 
 Review comment:
   This a byte array copy? If we wanted to wrap w/o copy, could we do that?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-600372916
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m  6s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 58s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m  2s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 28s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 21s |  the patch passed  |
   | -1 :x: |  shadedjars  |   6m 21s |  patch has 10 errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 53s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 100m 51s |  hbase-server in the patch failed.  |
   |  |   | 130m 25s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 787f9c547833 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 9804f73c66 |
   | Default Java | 1.8.0_232 |
   | shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-jdk8-hadoop2-check/output/patch-shadedjars.txt |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-jdk8-hadoop2-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/testReport/ |
   | Max. process+thread count | 6594 (vs. ulimit of 10000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-601891991
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 44s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 28s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   5m 59s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 16s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 54s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 53s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   6m 20s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m  5s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 52s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 12s |  hbase-resource-bundle in the patch passed.  |
   | +1 :green_heart: |  unit  |  67m 59s |  hbase-server in the patch passed.  |
   |  |   | 100m 48s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 525638658ede 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 080d8645ed |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/testReport/ |
   | Max. process+thread count | 6076 (vs. ulimit of 10000) |
   | modules | C: hbase-resource-bundle hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-611185145
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 31s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 42s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   6m 55s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 44s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 43s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   6m 38s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 43s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 42s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 11s |  hbase-resource-bundle in the patch passed.  |
   | +1 :green_heart: |  unit  | 186m 35s |  hbase-server in the patch passed.  |
   |  |   | 220m 30s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 19922f5f641c 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ed830222da |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/testReport/ |
   | Max. process+thread count | 3657 (vs. ulimit of 10000) |
   | modules | C: hbase-resource-bundle hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] jojochuang commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
jojochuang commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-601893147
 
 
   The spotbugs error is unrelated, existing bug.
   Will file a jira for that.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-611154893
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 50s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 38s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   6m 21s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 14s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 55s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   6m 42s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 51s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   0m 14s |  hbase-resource-bundle in the patch passed.  |
   | +1 :green_heart: |  unit  | 128m 25s |  hbase-server in the patch passed.  |
   |  |   | 162m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 843ba934e2f2 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ed830222da |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/testReport/ |
   | Max. process+thread count | 3362 (vs. ulimit of 10000) |
   | modules | C: hbase-resource-bundle hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] jojochuang commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r395393408
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
 
 Review comment:
   yeah i think that's possible. In fact, that was the case prior to HBASE-17908. Here the code simply do what was there after HBASE-17908. I can certain try do the zero copy thing too.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394797233
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -363,7 +410,7 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload,
       if (payload != null) {
         // Was ByteStringer; fix w/o using ByteStringer. Its in hbase-protocol
         // and we want to keep that out of hbase-server.
-        builder.setPayload(ByteString.copyFrom(payload));
+        BuilderPayloadSetter.setter(builder, payload);
 
 Review comment:
   The builder is from HDFS. It is NOT a pb. It has pbs in it?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack merged pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack merged pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-601870220
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 18s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 34s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   6m 17s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 26s |  master passed  |
   | -1 :x: |  spotbugs  |   2m 20s |  hbase-server in master has 1 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   6m  0s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 14s |  hbase-server: The patch generated 5 new + 1 unchanged - 0 fixed = 6 total (was 1)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  hadoopcheck  |  12m 18s |  Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2.  |
   | +1 :green_heart: |  spotbugs  |   2m 22s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 22s |  The patch does not generate ASF License warnings.  |
   |  |   |  42m  8s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | dupname asflicense xml spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 1f1dd90cda69 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 080d8645ed |
   | spotbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-general-check/output/branch-spotbugs-hbase-server-warnings.html |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 83 (vs. ulimit of 10000) |
   | modules | C: hbase-resource-bundle hbase-server U: . |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/2/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394797856
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+
+        if (setPayloadMethod == null) {
+          try {
+            setPayloadMethod = builderClass.getMethod("setPayload", byteStringClass);
+          } catch (NoSuchMethodException e) {
+            e.printStackTrace();
+          }
+        }
+
+        try {
+          setPayloadMethod.invoke(builder, byteStringObject);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+      }
+
+      static {
+        builderClass = DataTransferEncryptorMessageProto.Builder.class;
+        byteStringClass = com.google.protobuf.ByteString.class;
 
 Review comment:
   Seems like it should be hbase internal shaded ByteString, not this com.google one.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] jojochuang commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r395392593
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+
+        if (setPayloadMethod == null) {
+          try {
+            setPayloadMethod = builderClass.getMethod("setPayload", byteStringClass);
 
 Review comment:
   Moved to static loading time.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394798728
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/ProtobufDecoder.java
 ##########
 @@ -0,0 +1,116 @@
+package org.apache.hadoop.hbase.io.asyncfs;
+
+import org.apache.hbase.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.hbase.thirdparty.io.netty.buffer.ByteBufUtil;
+import org.apache.hbase.thirdparty.io.netty.channel.ChannelHandlerContext;
+import org.apache.hbase.thirdparty.io.netty.handler.codec.MessageToMessageDecoder;
+import org.apache.hbase.thirdparty.io.netty.util.internal.ObjectUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.List;
+
+/** Modified based on io.netty.handler.codec.protobuf.ProtobufDecoder */
 
 Review comment:
   Needs comment on what this is doing.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394797576
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+
+        if (setPayloadMethod == null) {
+          try {
+            setPayloadMethod = builderClass.getMethod("setPayload", byteStringClass);
+          } catch (NoSuchMethodException e) {
+            e.printStackTrace();
+          }
+        }
+
+        try {
+          setPayloadMethod.invoke(builder, byteStringObject);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+      }
+
+      static {
+        builderClass = DataTransferEncryptorMessageProto.Builder.class;
+        byteStringClass = com.google.protobuf.ByteString.class;
 
 Review comment:
   Or this is just a default setting?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394798491
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/ProtobufDecoder.java
 ##########
 @@ -0,0 +1,116 @@
+package org.apache.hadoop.hbase.io.asyncfs;
 
 Review comment:
   license?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-600353458
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 23s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 29s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 15s |  master passed  |
   | -1 :x: |  spotbugs  |   2m 13s |  hbase-server in master has 1 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 34s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 17s |  hbase-server: The patch generated 9 new + 1 unchanged - 0 fixed = 10 total (was 1)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m  8s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.  |
   | -1 :x: |  spotbugs  |   2m 39s |  hbase-server generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)  |
   ||| _ Other Tests _ |
   | -1 :x: |  asflicense  |   0m 13s |  The patch generated 1 ASF License warnings.  |
   |  |   |  48m  0s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | FindBugs | module:hbase-server |
   |  |  Possible null pointer dereference of FanOutOneBlockAsyncDFSOutputSaslHelper$SaslNegotiateHandler$BuilderPayloadSetter.setPayloadMethod in org.apache.hadoop.hbase.io.asyncfs.FanOutOneBlockAsyncDFSOutputSaslHelper$SaslNegotiateHandler$BuilderPayloadSetter.setter(DataTransferProtos$DataTransferEncryptorMessageProto$Builder, byte[]) on exception path  Dereferenced at FanOutOneBlockAsyncDFSOutputSaslHelper.java:FanOutOneBlockAsyncDFSOutputSaslHelper$SaslNegotiateHandler$BuilderPayloadSetter.setPayloadMethod in org.apache.hadoop.hbase.io.asyncfs.FanOutOneBlockAsyncDFSOutputSaslHelper$SaslNegotiateHandler$BuilderPayloadSetter.setter(DataTransferProtos$DataTransferEncryptorMessageProto$Builder, byte[]) on exception path  Dereferenced at FanOutOneBlockAsyncDFSOutputSaslHelper.java:[line 381] |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 1d8bc2f6f13b 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 9804f73c66 |
   | spotbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-general-check/output/branch-spotbugs-hbase-server-warnings.html |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | spotbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-general-check/output/new-spotbugs-hbase-server.html |
   | asflicense | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-general-check/output/patch-asflicense-problems.txt |
   | Max. process+thread count | 83 (vs. ulimit of 10000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#issuecomment-600370935
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 56s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 11s |  master passed  |
   | -1 :x: |  shadedjars  |   0m  9s |  branch has 7 errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 21s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  7s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  7s |  the patch passed  |
   | -1 :x: |  shadedjars  |   0m  9s |  patch has 7 errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 39s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 101m 14s |  hbase-server in the patch passed.  |
   |  |   | 121m 22s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1301 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux fa45c7f4b53a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 9804f73c66 |
   | Default Java | 2020-01-14 |
   | shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-jdk11-hadoop3-check/output/branch-shadedjars.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/testReport/ |
   | Max. process+thread count | 5770 (vs. ulimit of 10000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1301/1/console |
   | versions | git=2.17.1 maven=2018-06-17T18:33:14Z) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [hbase] saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1301: HBASE-23833. The relocated hadoop-thirdparty protobuf breaks HBase asyncwal	
URL: https://github.com/apache/hbase/pull/1301#discussion_r394797719
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/asyncfs/FanOutOneBlockAsyncDFSOutputSaslHelper.java
 ##########
 @@ -355,6 +353,55 @@ private void sendSaslMessage(ChannelHandlerContext ctx, byte[] payload) throws I
       return Collections.singletonList(new CipherOption(CipherSuite.AES_CTR_NOPADDING));
     }
 
+    private static class BuilderPayloadSetter {
+      private static Class<?> byteStringClass;
+      private static Class<?> builderClass;
+      private static Method copyFromMethod;
+      private static Method setPayloadMethod = null;
+
+      static void setter(DataTransferEncryptorMessageProto.Builder builder, byte[] payload) {
+        Object byteStringObject = null;
+        try {
+          byteStringObject = copyFromMethod.invoke(null, payload);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+
+        if (setPayloadMethod == null) {
+          try {
+            setPayloadMethod = builderClass.getMethod("setPayload", byteStringClass);
+          } catch (NoSuchMethodException e) {
+            e.printStackTrace();
+          }
+        }
+
+        try {
+          setPayloadMethod.invoke(builder, byteStringObject);
+        } catch (IllegalAccessException e) {
+          e.printStackTrace();
+        } catch (InvocationTargetException e) {
+          e.printStackTrace();
+        }
+      }
+
+      static {
+        builderClass = DataTransferEncryptorMessageProto.Builder.class;
+        byteStringClass = com.google.protobuf.ByteString.class;
+        try {
+          byteStringClass = Class.forName("org.apache.hadoop.thirdparty.protobuf.ByteString");
+        } catch (ClassNotFoundException e) {
+          e.printStackTrace();
 
 Review comment:
   This is printed out once on class loading? We can prettify later.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services