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 si...@apache.org on 2017/09/12 08:33:34 UTC
[distributedlog] 01/02: ISSUE #185: Thrift version conflicts and
use heap bytebuffer for thrift serialization
This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch release-0.5.0
in repository https://gitbox.apache.org/repos/asf/distributedlog.git
commit 5cccddbb7885962ad84758c110eda13fc2d62f46
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>.