finagle-memcached: Remove Ketama Partitioned Client

Problem

The Partition Aware Client has introduced to Memcached a while ago and coexisted with the KetamaPartitionedClient, and the latter creates a Twemcache client for each endpoint, which adds extra pressure to the cluster discovery service.

Solution

Most of the customers/clients have migrated to using the cluster discovery service and we can finally get rid of the old Memcached client.

Result

  # Although might be negligible, moving the current old-style Memcached clients to partition aware client can remove some request pressure to the cluster discovery service,
  # Remove redundant code and have a clean codebase to add new features.

JIRA Issues: CSL-10701

Differential Revision: https://phabricator.twitter.biz/D661460

CHANGELOG.rst

@@ -17,6 +17,14 @@ New Features
   defined in both, and both take precedence over the `Dtab.base`. The existing
   `Dtab.local` request propagation behavior remains unchanged. ``PHAB_ID=D677860``
 
+Breaking API Changes
+~~~~~~~~~~~~~~~~~~~~
+
+* finagle-memcached: Ketama Partitioned Client has been removed and the Partition Aware
+  Memcached Client has been made the default. As part of this change,
+  `com.twitter.finagle.memcached.UsePartitioningMemcachedClient` toggle has been removed,
+  and it no longer applies. ``PHAB_ID=D661460``
+
 Runtime Behavior Changes
 ~~~~~~~~~~~~~~~~~~~~~~~~
 

finagle-memcached/src/main/scala/com/twitter/finagle/Memcached.scala

@@ -1,6 +1,5 @@
 package com.twitter.finagle
 
-import com.twitter.concurrent.Broker
 import com.twitter.conversions.DurationOps._
 import com.twitter.{finagle, hashing}
 import com.twitter.finagle.client._
@@ -11,8 +10,8 @@ import com.twitter.finagle.dispatch.{
 }
 import com.twitter.finagle.liveness.{FailureAccrualFactory, FailureAccrualPolicy}
 import com.twitter.finagle.loadbalancer.{Balancers, LoadBalancerFactory}
-import com.twitter.finagle.memcached.{Toggles, _}
 import com.twitter.finagle.memcached.exp.LocalMemcached
+import com.twitter.finagle.memcached._
 import com.twitter.finagle.memcached.partitioning.MemcachedPartitioningService
 import com.twitter.finagle.memcached.protocol.text.server.ServerTransport
 import com.twitter.finagle.memcached.protocol.text.transport.{
@@ -30,21 +29,15 @@ import com.twitter.finagle.param.{
   Tracer => _,
   _
 }
-import com.twitter.finagle.partitioning.param.{KeyHasher, NumReps, WithPartitioningStrategy}
-import com.twitter.finagle.partitioning.{
-  ConsistentHashingFailureAccrualFactory,
-  HashNodeKey,
-  NodeHealth,
-  PartitionNode
-}
+import com.twitter.finagle.partitioning.param.{KeyHasher, WithPartitioningStrategy}
 import com.twitter.finagle.pool.BalancingPool
 import com.twitter.finagle.pushsession.{
   PipeliningClientPushSession,
   PushChannelHandle,
   PushStackClient,
   PushTransporter
 }
-import com.twitter.finagle.server.{Listener, ServerInfo, StackServer, StdStackServer}
+import com.twitter.finagle.server.{Listener, StackServer, StdStackServer}
 import com.twitter.finagle.service._
 import com.twitter.finagle.stats.{ExceptionStatsHandler, StatsReceiver}
 import com.twitter.finagle.tracing.{ClientDestTracingFilter, Tracer}
@@ -123,18 +116,6 @@ trait MemcachedRichClient { self: finagle.Client[Command, Response] =>
  */
 object Memcached extends finagle.Client[Command, Response] with finagle.Server[Command, Response] {
 
-  /**
-   * Toggles to the new PartitioningService based Memcached client. Note that the new implementation
-   * is automatically used when the destination is a [[Name.Path]] irrespective of this flag. During
-   * this experimental phase the old implementation will continue to be used by default when
-   * destination is a finagle [[Name.Bound]]. [[Name.Path]] based destinations were not
-   * supported previously. This flag toggles to the new client for all destination types.
-   */
-  private[finagle] val UsePartitioningMemcachedClientToggle =
-    "com.twitter.finagle.memcached.UsePartitioningMemcachedClient"
-  private[this] val toggle = Toggles(UsePartitioningMemcachedClientToggle)
-  private[this] def UsePartitioningMemcachedClient = toggle(ServerInfo().id.hashCode)
-
   object Client {
 
     private[Memcached] val ProtocolLibraryName = "memcached"
@@ -269,43 +250,15 @@ object Memcached extends finagle.Client[Command, Response] with finagle.Server[C
         TwemcacheClient(rawClient)
       }
 
-      def oldMemcachedClient(va: Var[Addr]) = {
-        DefaultLogger.fine(s"Using the old memcached client: $destination")
-
-        val finagle.param.Stats(sr) = params[finagle.param.Stats]
-        val NumReps(numReps) = params[NumReps]
-
-        val scopedSr = sr.scope(label0)
-        val healthBroker = new Broker[NodeHealth]
-
-        def newService(node: PartitionNode): Service[Command, Response] = {
-          val key = HashNodeKey.fromPartitionNode(node)
-          val stk = stack.replace(
-            FailureAccrualFactory.role,
-            ConsistentHashingFailureAccrualFactory.module[Command, Response](key, healthBroker)
+      destination match {
+        case Name.Bound(va) =>
+          partitionAwareFinagleClient()
+        case Name.Path(_path: Path) =>
+          partitionAwareFinagleClient()
+        case n =>
+          throw new IllegalArgumentException(
+            s"Memcached client only supports Bound Names or Name.Path, was: $n"
           )
-          withStack(stk).newService(Client.mkDestination(node.host, node.port), label0)
-        }
-
-        new KetamaPartitionedClient(va, newService, healthBroker, scopedSr, hasher, numReps)
-          with TwemcachePartitionedClient
-      }
-
-      if (UsePartitioningMemcachedClient) {
-        partitionAwareFinagleClient()
-      } else {
-        // By default use the new implementation only when destination is a Path (which was
-        // not supported before).
-        destination match {
-          case Name.Bound(va) =>
-            oldMemcachedClient(va)
-          case Name.Path(_path: Path) =>
-            partitionAwareFinagleClient()
-          case n =>
-            throw new IllegalArgumentException(
-              s"Memcached client only supports Bound Names or Name.Path, was: $n"
-            )
-        }
       }
     }