finagle-core: Remove deprecated ClientBuilder methods

Problem Methods in c.t.f.ClientBuilder were deprecated because they were not broadly applicable to clients. Now that a release has been done with those deprecations, the methods can be removed. Solution Remove the methods. RB_ID=893147 TBR=true
twitter · Dec 12, 2016 · b1f927c · b1f927c
1 parent bf0e9ca
commit b1f927c
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 201 deletions.
diff --git a/CHANGES b/CHANGES
@@ -16,6 +16,22 @@ Runtime Behavior Changes
 Breaking API Changes
 ~~~~~~~~~~~~~~~~~~~~
 
+* finagle-core: `c.t.f.builder.ClientBuilder` remove deprecated methods.
+  The same functionality is available through the Stack-based APIs or
+  `ClientBuilder.configured`, with the exception of `channelFactory`, which
+  has no analog because it exposes a Netty 3 API. ``RB_ID=893147``
+
+    - `channelFactory`
+    - `expHostConnectionBufferSize`
+    - `hostConnectionIdleTime`
+    - `hostConnectionMaxIdleTime`
+    - `hostConnectionMaxLifeTime`
+    - `hostConnectionMaxWaiters`
+    - `readerIdleTimeout`
+    - `recvBufferSize`
+    - `sendBufferSize`
+    - `writerIdleTimeout`
+
   * The `codec` method has been removed from the kestrel MultiReader object. Configure
     a ClientBuilder protocol using the default thrift StackClient, Thrift.client, via the
     `stack` method of ClientBuilder. ``RB_ID=894297``

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
@@ -22,7 +22,7 @@ import java.net.{InetSocketAddress, SocketAddress}
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.logging.Level
 import javax.net.ssl.SSLContext
-import org.jboss.netty.channel.{Channel, ChannelFactory}
+import org.jboss.netty.channel.Channel
 import scala.annotation.{implicitNotFound, varargs}
 
 /**
@@ -671,40 +671,6 @@ class ClientBuilder[Req, Rep, HasCluster, HasCodec, HasHostConnectionLimit] priv
   def keepAlive(value: Boolean): This =
     configured(params[Transport.Liveness].copy(keepAlive = Some(value)))
 
-  /**
-   * The maximum time a connection may have received no data.
-   *
-   * To migrate to the Stack-based APIs, use `TransportParams.readTimeout`.
-   * For example:
-   * {{{
-   * import com.twitter.finagle.Http
-   *
-   * Http.client.withTransport.readTimeout(duration)
-   * }}}
-   */
-  @deprecated(
-    "Use `configured` or the Stack-based API `TransportParams.readTimeout`",
-    "2016-10-05")
-  def readerIdleTimeout(duration: Duration): This =
-    configured(params[Transport.Liveness].copy(readTimeout = duration))
-
-  /**
-   * The maximum time a connection may not have sent any data.
-   *
-   * To migrate to the Stack-based APIs, use `TransportParams.writeTimeout`.
-   * For example:
-   * {{{
-   * import com.twitter.finagle.Http
-   *
-   * Http.client.withTransport.writeTimeout(duration)
-   * }}}
-   */
-  @deprecated(
-    "Use `configured` or the Stack-based API `TransportParams.writeTimeout`",
-    "2016-10-05")
-  def writerIdleTimeout(duration: Duration): This =
-    configured(params[Transport.Liveness].copy(writeTimeout = duration))
-
   /**
    * Report stats to the given `StatsReceiver`.  This will report
    * verbose global statistics and counters, that in turn may be
@@ -794,89 +760,6 @@ class ClientBuilder[Req, Rep, HasCluster, HasCodec, HasHostConnectionLimit] priv
   def hostConnectionCoresize(value: Int): This =
     configured(params[DefaultPool.Param].copy(low = value))
 
-  /**
-   * The amount of time a connection is allowed to linger (when it
-   * otherwise would have been closed by the pool) before being
-   * closed.
-   *
-   * @note not all protocol implementations support this style of connection
-   *       pooling, such as `com.twitter.finagle.ThriftMux` and
-   *       `com.twitter.finagle.Memcached`.
-   */
-  @deprecated("Use `configured`", "2016-10-18")
-  def hostConnectionIdleTime(timeout: Duration): This =
-    configured(params[DefaultPool.Param].copy(idleTime = timeout))
-
-  /**
-   * The maximum queue size for the connection pool.
-   *
-   * To migrate to the Stack-based APIs, use `SessionPoolingParams.maxWaiters`.
-   * For example:
-   * {{{
-   * import com.twitter.finagle.Http
-   *
-   * Http.client.withSessionPool.maxWaiters(nWaiters)
-   * }}}
-   *
-   * @note not all protocol implementations support this style of connection
-   *       pooling, such as `com.twitter.finagle.ThriftMux` and
-   *       `com.twitter.finagle.Memcached`.
-   */
-  @deprecated(
-    "Use `configured` or or the Stack-based API `SessionPoolingParams.maxWaiters`",
-    "2016-10-18")
-  def hostConnectionMaxWaiters(nWaiters: Int): This =
-    configured(params[DefaultPool.Param].copy(maxWaiters = nWaiters))
-
-  /**
-   * The maximum time a connection is allowed to linger unused.
-   *
-   * To migrate to the Stack-based APIs, use `SessionParams.maxIdleTime`.
-   * For example:
-   * {{{
-   * import com.twitter.finagle.Http
-   *
-   * Http.client.withSession.maxIdleTime(timeout)
-   * }}}
-   */
-  @deprecated("Use `configured`", "2016-10-18")
-  def hostConnectionMaxIdleTime(timeout: Duration): This =
-    configured(params[ExpiringService.Param].copy(idleTime = timeout))
-
-  /**
-   * The maximum time a connection is allowed to exist, regardless of occupancy.
-   *
-   * To migrate to the Stack-based APIs, use `SessionParams.maxLifeTime`.
-   * For example:
-   * {{{
-   * import com.twitter.finagle.Http
-   *
-   * Http.client.withSession.maxLifeTime(timeout)
-   * }}}
-   */
-  @deprecated("Use `configured`", "2016-10-18")
-  def hostConnectionMaxLifeTime(timeout: Duration): This =
-    configured(params[ExpiringService.Param].copy(lifeTime = timeout))
-
-  /**
-   * Experimental option to buffer `size` connections from the pool.
-   * The buffer is fast and lock-free, reducing contention for
-   * services with very high requests rates. The buffer size should
-   * be sized roughly to the expected concurrency. Buffers sized by
-   * power-of-twos may be faster due to the use of modular
-   * arithmetic.
-   *
-   * @note This will be integrated into the mainline pool, at
-   * which time the experimental option will go away.
-   *
-   * @note not all protocol implementations support this style of connection
-   *       pooling, such as `com.twitter.finagle.ThriftMux` and
-   *       `com.twitter.finagle.Memcached`.
-   */
-  @deprecated("Use `configured`", "2016-10-05")
-  def expHostConnectionBufferSize(size: Int): This =
-    configured(params[DefaultPool.Param].copy(bufferSize = size))
-
   /**
    * Configure a [[com.twitter.finagle.service.ResponseClassifier]]
    * which is used to determine the result of a request/response.
@@ -1038,50 +921,6 @@ class ClientBuilder[Req, Rep, HasCluster, HasCodec, HasHostConnectionLimit] priv
   def retryBudget(budget: RetryBudget, backoffSchedule: Stream[Duration]): This =
     configured(Retries.Budget(budget, backoffSchedule))
 
-  /**
-   * Sets the TCP send buffer size.
-   *
-   * To migrate to the Stack-based APIs, use `TransportParams.sendBufferSize`.
-   * For example:
-   * {{{
-   * import com.twitter.finagle.Http
-   *
-   * Http.client.withTransport.sendBufferSize(value)
-   * }}}
-   */
-  @deprecated(
-    "Use `configured` or the Stack-based API `TransportParams.sendBufferSize`",
-    "2016-10-05")
-  def sendBufferSize(value: Int): This =
-    configured(params[Transport.BufferSizes].copy(send = Some(value)))
-
-  /**
-   * Sets the TCP recv buffer size.
-   *
-   * To migrate to the Stack-based APIs, use `TransportParams.receiveBufferSize`.
-   * For example:
-   * {{{
-   * import com.twitter.finagle.Http
-   *
-   * Http.client.withTransport.receiveBufferSize(value)
-   * }}}
-   */
-  @deprecated(
-    "Use `configured` or the Stack-based API `TransportParams.receiveBufferSize`",
-    "2016-10-05")
-  def recvBufferSize(value: Int): This =
-    configured(params[Transport.BufferSizes].copy(recv = Some(value)))
-
-  /**
-   * Use the given channel factory instead of the default. Note that
-   * when using a non-default ChannelFactory, finagle can't
-   * meaningfully reference count factory usage, and so the caller is
-   * responsible for calling `releaseExternalResources()`.
-   */
-  @deprecated("Use `configured`", "2016-10-05")
-  def channelFactory(cf: ChannelFactory): This =
-    configured(Netty3Transporter.ChannelFactory(cf))
-
   /**
    * Encrypt the connection with SSL.  Hostname verification will be
    * provided against the given hostname.

diff --git a/finagle-core/src/test/scala/com/twitter/finagle/builder/EndToEndTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/builder/EndToEndTest.scala
@@ -226,13 +226,19 @@ class EndToEndTest extends FunSuite with StringClient with StringServer {
 
     // start with an empty cluster
     val cluster = new DynamicCluster[SocketAddress](Seq[SocketAddress]())
-    val client = ClientBuilder()
+
+    val client = {
+      val cb = ClientBuilder()
       .cluster(cluster)
       .codec(StringCodec)
       .daemon(true) // don't create an exit guard
       .hostConnectionLimit(1)
-      .hostConnectionMaxWaiters(5)
-      .build()
+
+      val maxWaiters = cb.params[DefaultPool.Param].copy(
+           maxWaiters = 5)
+
+      cb.configured(maxWaiters).build()
+    }
 
     val responses = new Array[Future[String]](5)
     0 until 5 foreach { i =>
@@ -269,16 +275,21 @@ class EndToEndTest extends FunSuite with StringClient with StringServer {
       .build(never)
 
     val mem = new InMemoryStatsReceiver
-    val client = ClientBuilder()
-      .name("client")
-      .hosts(server.boundAddress.asInstanceOf[InetSocketAddress])
-      .codec(StringCodec)
-      .daemon(true) // don't create an exit guard
-      .requestTimeout(10.millisecond)
-      .hostConnectionLimit(1)
-      .hostConnectionMaxWaiters(1)
-      .reportTo(mem)
-      .build()
+    val client = {
+      val cb = ClientBuilder()
+        .name("client")
+        .hosts(server.boundAddress.asInstanceOf[InetSocketAddress])
+        .codec(StringCodec)
+        .daemon(true) // don't create an exit guard
+        .requestTimeout(10.millisecond)
+        .hostConnectionLimit(1)
+        .reportTo(mem)
+
+      val maxWaiters = cb.params[DefaultPool.Param].copy(
+        maxWaiters = 1)
+
+      cb.configured(maxWaiters).build()
+    }
 
     // generate com.twitter.finagle.IndividualRequestTimeoutException
     intercept[IndividualRequestTimeoutException] { Await.result(client("hi"), 1.second) }
@@ -293,16 +304,21 @@ class EndToEndTest extends FunSuite with StringClient with StringServer {
 
   test("ClientBuilder should be properly instrumented on service acquisition failure") {
     val mem = new InMemoryStatsReceiver
-    val client = ClientBuilder()
+    val client = {
+      val cb = ClientBuilder()
         .name("client")
         .addrs(Address.failing)
         .codec(StringCodec)
         .daemon(true) // don't create an exit guard
         .requestTimeout(10.millisecond)
         .hostConnectionLimit(1)
-        .hostConnectionMaxWaiters(1)
         .reportTo(mem)
-        .build()
+
+      val maxWaiters = cb.params[DefaultPool.Param].copy(
+           maxWaiters = 1)
+
+      cb.configured(maxWaiters).build()
+    }
 
     // generate com.twitter.finagle.ChannelWriteException
     val traceId = Trace.id
@@ -335,15 +351,20 @@ class EndToEndTest extends FunSuite with StringClient with StringServer {
       .build(always)
 
     val mem = new InMemoryStatsReceiver
-    val client = ClientBuilder()
-      .name("testClient")
-      .hosts(server.boundAddress.asInstanceOf[InetSocketAddress])
-      .codec(StringCodec)
-      .hostConnectionLimit(1)
-      .hostConnectionMaxWaiters(1)
-      .reportTo(mem)
-      .retries(1)
-      .build()
+    val client = {
+      val cb = ClientBuilder()
+        .name("testClient")
+        .hosts(server.boundAddress.asInstanceOf[InetSocketAddress])
+        .codec(StringCodec)
+        .hostConnectionLimit(1)
+        .reportTo(mem)
+        .retries(1)
+
+      val maxWaiters = cb.params[DefaultPool.Param].copy(
+        maxWaiters = 1)
+
+      cb.configured(maxWaiters).build()
+    }
 
     Await.result(client("ping"), 10.second)
     Await.ready(server.close(), 1.second)

diff --git a/finagle-core/src/test/scala/com/twitter/finagle/builder/ServerChannelConfigurationTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/builder/ServerChannelConfigurationTest.scala
@@ -23,10 +23,9 @@ class ServerChannelConfigurationTest
     val lifeTime = 1.seconds
     val address = new InetSocketAddress(InetAddress.getLoopbackAddress, 0)
     val server = ServerBuilder()
-      .stack(Server())
+      .stack(Server().withSession.maxLifeTime(lifeTime))
       .bindTo(address)
       .name("FinagleServer")
-      .hostConnectionMaxLifeTime(lifeTime)
       .build(identityService)
 
     val client: Service[String, String] = ClientBuilder()
@@ -46,10 +45,11 @@ class ServerChannelConfigurationTest
     val idleTime = 1.second
     val address = new InetSocketAddress(InetAddress.getLoopbackAddress, 0)
     val server = ServerBuilder()
-      .stack(Server())
+      .stack(Server()
+        .withSession.maxIdleTime(idleTime)
+      )
       .bindTo(address)
       .name("FinagleServer")
-      .hostConnectionMaxIdleTime(idleTime)
       .build(identityService)
 
     val client: Service[String, String] = ClientBuilder()

diff --git a/finagle-core/src/test/scala/com/twitter/finagle/integration/Base.scala b/finagle-core/src/test/scala/com/twitter/finagle/integration/Base.scala
@@ -97,7 +97,7 @@ trait IntegrationBase extends FunSuite with MockitoSugar {
     val clientBuilder = ClientBuilder()
       .name(name)
       .codec(codecFactory)
-      .channelFactory(channelFactory)
+      .configured(Netty3Transporter.ChannelFactory(channelFactory))
       .daemon(true) // don't create an exit guard
       .hosts(Seq(clientAddress))
       .reportTo(statsReceiver)

diff --git a/finagle-example/src/main/scala/com/twitter/finagle/example/memcache/MemcacheStress.scala b/finagle-example/src/main/scala/com/twitter/finagle/example/memcache/MemcacheStress.scala
@@ -6,6 +6,7 @@ import com.twitter.concurrent.NamedPoolThreadFactory
 import com.twitter.finagle.builder.ClientBuilder
 import com.twitter.finagle.memcached
 import com.twitter.finagle.memcached.protocol.text.Memcached
+import com.twitter.finagle.netty3.Netty3Transporter
 import com.twitter.finagle.stats.OstrichStatsReceiver
 import com.twitter.finagle.{Service, ServiceFactory}
 import com.twitter.io.Buf
@@ -54,13 +55,13 @@ object MemcacheStress extends App {
       .hosts(config.hosts())
 
     if (config.nworkers() > 0)
-      builder = builder.channelFactory(
+      builder = builder.configured(Netty3Transporter.ChannelFactory(
           new NioClientSocketChannelFactory(
             Executors.newCachedThreadPool(new NamedPoolThreadFactory("memcacheboss")),
             Executors.newCachedThreadPool(new NamedPoolThreadFactory("memcacheIO")),
             config.nworkers()
           )
-        )
+        ))
 
     if (config.stats())    builder = builder.reportTo(new OstrichStatsReceiver)
     if (config.tracing())  com.twitter.finagle.tracing.Trace.enable()

diff --git a/finagle-exception/src/main/scala/com/twitter/finagle/exception/Reporter.scala b/finagle-exception/src/main/scala/com/twitter/finagle/exception/Reporter.scala
@@ -94,12 +94,11 @@ object Reporter {
     val service = ClientBuilder() // these are from the zipkin tracer
       .name("exception_reporter")
       .hosts(new InetSocketAddress(scribeHost, scribePort))
-      .stack(Thrift.client)
+      .stack(Thrift.client
+        // somewhat arbitrary, but bounded timeouts
+        .withSessionPool.maxWaiters(250))
       .reportTo(ClientStatsReceiver)
       .hostConnectionLimit(5)
-      // using an arbitrary, but bounded number of waiters to avoid memory leaks
-      .hostConnectionMaxWaiters(250)
-      // somewhat arbitrary, but bounded timeouts
       .timeout(1.second)
       .daemon(true)
       .build()