DL-90: Don't use stack and codec together for configuring thriftmux
merge Twitter's change on thriftmux
Author: Sijie Guo <sijieg@twitter.com>
Author: Jordan Bull <jbull@twitter.com>
Author: Sijie Guo <sijie@apache.org>
Author: Leigh Stewart <lstewart@twitter.com>
Author: Dave Rusek <dave.rusek@gmail.com>
Author: Dave Rusek <drusek@twitter.com>
Reviewers: Leigh Stewart <lstewart@apache.org>
Closes #62 from sijie/merge/DL-90
diff --git a/distributedlog-client/src/main/java/com/twitter/distributedlog/client/proxy/ProxyClient.java b/distributedlog-client/src/main/java/com/twitter/distributedlog/client/proxy/ProxyClient.java
index 03dd3c2..4c79327 100644
--- a/distributedlog-client/src/main/java/com/twitter/distributedlog/client/proxy/ProxyClient.java
+++ b/distributedlog-client/src/main/java/com/twitter/distributedlog/client/proxy/ProxyClient.java
@@ -75,30 +75,36 @@
this.clientId = clientId;
this.clientStats = clientStats;
// client builder
- ClientBuilder builder = setDefaultSettings(null == clientBuilder ? getDefaultClientBuilder() : clientBuilder);
- if (clientConfig.getThriftMux()) {
- builder = enableThriftMux(builder, clientId);
- }
- this.clientBuilder = builder;
+ ClientBuilder builder = setDefaultSettings(
+ null == clientBuilder ? getDefaultClientBuilder(clientConfig) : clientBuilder);
+ this.clientBuilder = configureThriftMux(builder, clientId, clientConfig);
}
@SuppressWarnings("unchecked")
- private ClientBuilder enableThriftMux(ClientBuilder builder, ClientId clientId) {
- return builder.stack(ThriftMux.client().withClientId(clientId));
+ private ClientBuilder configureThriftMux(ClientBuilder builder,
+ ClientId clientId,
+ ClientConfig clientConfig) {
+ if (clientConfig.getThriftMux()) {
+ return builder.stack(ThriftMux.client().withClientId(clientId));
+ } else {
+ return builder.codec(ThriftClientFramedCodec.apply(Option.apply(clientId)));
+ }
}
- private ClientBuilder getDefaultClientBuilder() {
- return ClientBuilder.get()
- .hostConnectionLimit(1)
+ private ClientBuilder getDefaultClientBuilder(ClientConfig clientConfig) {
+ ClientBuilder builder = ClientBuilder.get()
.tcpConnectTimeout(Duration.fromMilliseconds(200))
.connectTimeout(Duration.fromMilliseconds(200))
.requestTimeout(Duration.fromSeconds(1));
+ if (!clientConfig.getThriftMux()) {
+ builder = builder.hostConnectionLimit(1);
+ }
+ return builder;
}
@SuppressWarnings("unchecked")
private ClientBuilder setDefaultSettings(ClientBuilder builder) {
return builder.name(clientName)
- .codec(ThriftClientFramedCodec.apply(Option.apply(clientId)))
.failFast(false)
.noFailureAccrual()
// disable retries on finagle client builder, as there is only one host per finagle client
diff --git a/distributedlog-core/src/main/java/com/twitter/distributedlog/DistributedLogConstants.java b/distributedlog-core/src/main/java/com/twitter/distributedlog/DistributedLogConstants.java
index 32def94..e798a0f 100644
--- a/distributedlog-core/src/main/java/com/twitter/distributedlog/DistributedLogConstants.java
+++ b/distributedlog-core/src/main/java/com/twitter/distributedlog/DistributedLogConstants.java
@@ -58,7 +58,7 @@
public static final String COMPLETED_LOGSEGMENT_PREFIX = "logrecs";
public static final String DISALLOW_PLACEMENT_IN_REGION_FEATURE_NAME = "disallow_bookie_placement";
static final byte[] CONTROL_RECORD_CONTENT = "control".getBytes(UTF_8);
- public static final byte[] KEEPALIVE_RECORD_CONTENT = "keepalive".getBytes(UTF_8);
+ static final byte[] KEEPALIVE_RECORD_CONTENT = "keepalive".getBytes(UTF_8);
// An ACL that gives all permissions to node creators and read permissions only to everyone else.
public static final List<ACL> EVERYONE_READ_CREATOR_ALL =
