You are viewing a plain text version of this content. The canonical link for it is here.
Posted to distributedlog-commits@bookkeeper.apache.org by zh...@apache.org on 2017/09/12 07:16:58 UTC

[distributedlog] branch master updated: ISSUE #185: Thrift version conflicts and use heap bytebuffer for thrift serialization

This is an automated email from the ASF dual-hosted git repository.

zhaijia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/distributedlog.git


The following commit(s) were added to refs/heads/master by this push:
     new dd2baba  ISSUE #185: Thrift version conflicts and use heap bytebuffer for thrift serialization
dd2baba is described below

commit dd2baba66c0cb2c6076c395fc7f1bf8aa2ac66ec
Author: Sijie Guo <si...@apache.org>
AuthorDate: Tue Sep 12 15:16:49 2017 +0800

    ISSUE #185: Thrift version conflicts and use heap bytebuffer for thrift serialization
    
    Descriptions of the changes in this PR:
    
    - it seems that shade plugin doesn't work well with sub-modules. even the libthrift-9 was shaded, it is still imported/included in the sub-modules, causing the version conflict. Explicitly exclude the libthrift from distributedlog-core.
    
    - thrift serialization is using heap bytebuffer. so if a bytebuffer is direct, the serialization will fail. add a change to check if bytebuffer is heap, if bytebuffer is not heap, copy the content into a heap buffer. this is not good for performance, but we don't have any other choices because the limitation comes from libthrift.
    
    Author: Sijie Guo <si...@apache.org>
    
    Reviewers: Enrico Olivelli <eo...@gmail.com>, Jia Zhai <None>, Leigh Stewart <None>
    
    This closes #187 from sijie/use_heapbytebuffer, closes #185
---
 distributedlog-core-twitter/pom.xml                 |  6 ++++++
 .../client/DistributedLogClientImpl.java            | 21 +++++++++++++++++----
 distributedlog-proxy-server/pom.xml                 |  6 ++++++
 .../distributedlog-basic/pom.xml                    |  6 ++++++
 .../distributedlog-messaging/pom.xml                |  6 ++++++
 5 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/distributedlog-core-twitter/pom.xml b/distributedlog-core-twitter/pom.xml
index fb2046b..5043e05 100644
--- a/distributedlog-core-twitter/pom.xml
+++ b/distributedlog-core-twitter/pom.xml
@@ -29,6 +29,12 @@
       <groupId>org.apache.distributedlog</groupId>
       <artifactId>distributedlog-core</artifactId>
       <version>${project.parent.version}</version>
+      <exclusions>
+        <exclusion>
+          <groupId>org.apache.thrift</groupId>
+          <artifactId>libthrift</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
     <dependency>
       <groupId>com.twitter</groupId>
diff --git a/distributedlog-proxy-client/src/main/java/org/apache/distributedlog/client/DistributedLogClientImpl.java b/distributedlog-proxy-client/src/main/java/org/apache/distributedlog/client/DistributedLogClientImpl.java
index ce851b5..55b7c34 100644
--- a/distributedlog-proxy-client/src/main/java/org/apache/distributedlog/client/DistributedLogClientImpl.java
+++ b/distributedlog-proxy-client/src/main/java/org/apache/distributedlog/client/DistributedLogClientImpl.java
@@ -397,14 +397,27 @@ public class DistributedLogClientImpl implements DistributedLogClient, MonitorSe
 
         WriteOp(final String name, final ByteBuf dataBuf) {
             super(name, clientStats.getOpStats("write"));
-            this.dataBuf = dataBuf;
-            this.data = dataBuf.nioBuffer();
+            if (dataBuf.hasArray()) {
+                this.dataBuf = dataBuf;
+                this.data = dataBuf.nioBuffer();
+            } else {
+                // thrift only takes heap byte buffer
+                this.dataBuf = Unpooled.copiedBuffer(dataBuf);
+                dataBuf.release();
+                this.data = this.dataBuf.nioBuffer();
+            }
         }
 
         WriteOp(final String name, final ByteBuffer data) {
             super(name, clientStats.getOpStats("write"));
-            this.data = data;
-            this.dataBuf = Unpooled.wrappedBuffer(data);
+            if (data.hasArray()) {
+                this.data = data;
+                this.dataBuf = Unpooled.wrappedBuffer(data);
+            } else {
+                // thrift only takes heap byte buffer
+                this.dataBuf = Unpooled.copiedBuffer(data);
+                this.data = this.dataBuf.nioBuffer();
+            }
         }
 
         @Override
diff --git a/distributedlog-proxy-server/pom.xml b/distributedlog-proxy-server/pom.xml
index 56625ac..6637e88 100644
--- a/distributedlog-proxy-server/pom.xml
+++ b/distributedlog-proxy-server/pom.xml
@@ -45,6 +45,12 @@
       <groupId>org.apache.distributedlog</groupId>
       <artifactId>distributedlog-core</artifactId>
       <version>${project.parent.version}</version>
+      <exclusions>
+        <exclusion>
+          <groupId>org.apache.thrift</groupId>
+          <artifactId>libthrift</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
     <dependency>
       <groupId>com.twitter</groupId>
diff --git a/distributedlog-tutorials/distributedlog-basic/pom.xml b/distributedlog-tutorials/distributedlog-basic/pom.xml
index fb5ac54..15e5cfd 100644
--- a/distributedlog-tutorials/distributedlog-basic/pom.xml
+++ b/distributedlog-tutorials/distributedlog-basic/pom.xml
@@ -36,6 +36,12 @@
       <groupId>org.apache.distributedlog</groupId>
       <artifactId>distributedlog-core</artifactId>
       <version>${project.parent.version}</version>
+      <exclusions>
+        <exclusion>
+          <groupId>org.apache.thrift</groupId>
+          <artifactId>libthrift</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
     <dependency>
       <groupId>org.apache.distributedlog</groupId>
diff --git a/distributedlog-tutorials/distributedlog-messaging/pom.xml b/distributedlog-tutorials/distributedlog-messaging/pom.xml
index 15eded4..14ac7b6 100644
--- a/distributedlog-tutorials/distributedlog-messaging/pom.xml
+++ b/distributedlog-tutorials/distributedlog-messaging/pom.xml
@@ -36,6 +36,12 @@
       <groupId>org.apache.distributedlog</groupId>
       <artifactId>distributedlog-core</artifactId>
       <version>${project.parent.version}</version>
+      <exclusions>
+        <exclusion>
+          <groupId>org.apache.thrift</groupId>
+          <artifactId>libthrift</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
     <dependency>
       <groupId>org.apache.distributedlog</groupId>

-- 
To stop receiving notification emails like this one, please contact
['"distributedlog-commits@bookkeeper.apache.org" <di...@bookkeeper.apache.org>'].