finagle-core: Offload early by default with OffloadFilter

Problem:
The toggle `com.twitter.finagle.OffloadEarly`, which moves the position of
OffloadFilter to the top of the StackClient and the bottom of StackServer has
been enabled for 6 months.

Solution:
Remove the toggle. The default behavior is to offload work to the offload pool
early.

JIRA Issues: CSL-11286

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

CHANGELOG.rst

@@ -14,6 +14,10 @@ Runtime Behavior Changes
 
 * finagle: Bump version of Jackson to 2.11.4. ``PHAB_ID=D727879``
 
+* finagle-core: OffloadFilter hands off work from Netty I/O thread to the offload CPU thread pool
+  right after we enter the Finagle stack by default. Previously this could be enabled via a toggle.
+  The `com.twitter.finagle.OffloadEarly` toggle has been removed. ``PHAB_ID=D733526``
+
 21.8.0 (No 21.7.0 Release)
 --------------------------
 

finagle-core/src/main/resources/com/twitter/toggles/configs/com.twitter.finagle.core.json

@@ -1,10 +1,5 @@
 {
   "toggles": [
-    {
-      "id": "com.twitter.finagle.OffloadEarly",
-      "description": "Hand off work to the Offload pool right after we enter the Finagle stack",
-      "fraction": 0.0
-    },
     {
       "id": "com.twitter.finagle.context.MarshalledContextLookupId",
       "description": "Case insenstive broadcast context key lookups",

finagle-core/src/main/scala/com/twitter/finagle/client/StackClient.scala

@@ -130,14 +130,7 @@ object StackClient {
    * @see [[com.twitter.finagle.filter.ExceptionSourceFilter]]
    * @see [[com.twitter.finagle.client.LatencyCompensation]]
    */
-  def endpointStack[Req, Rep]: Stack[ServiceFactory[Req, Rep]] =
-    // temporary standard behaviour for those calling this API from outside
-    // of StackClient.scala
-    endpointStack(false)
-
-  private def endpointStack[Req, Rep](
-    shouldOffloadEarly: Boolean
-  ): Stack[ServiceFactory[Req, Rep]] = {
+  def endpointStack[Req, Rep]: Stack[ServiceFactory[Req, Rep]] = {
     // Ensure that we have performed global initialization.
     com.twitter.finagle.Init()
 
@@ -146,17 +139,14 @@ object StackClient {
      */
     val stk = new StackBuilder[ServiceFactory[Req, Rep]](nilStack[Req, Rep])
 
-    if (shouldOffloadEarly) {
-
-      /**
-       * `OffloadFilter` shifts future continuations (callbacks and
-       * transformations) off of IO threads into a configured `FuturePool`.
-       * This module is intentionally placed at the top of the stack
-       * such that execution context shifts as client's response leaves
-       * the stack and enters the application code.
-       */
-      stk.push(OffloadFilter.client)
-    }
+    /**
+     * `OffloadFilter` shifts future continuations (callbacks and
+     * transformations) off of IO threads into a configured `FuturePool`.
+     * This module is intentionally placed at the top of the stack
+     * such that execution context shifts as client's response leaves
+     * the stack and enters the application code.
+     */
+    stk.push(OffloadFilter.client)
 
     /**
      * `prepConn` is the bottom of the stack by definition. This position represents
@@ -322,9 +312,7 @@ object StackClient {
      * as "module A is pushed after module B".
      */
 
-    val shouldOffloadEarly = offloadEarly() || com.twitter.finagle.offload.auto()
-
-    val stk = new StackBuilder(endpointStack[Req, Rep](shouldOffloadEarly))
+    val stk = new StackBuilder(endpointStack[Req, Rep])
 
     /*
      * These modules balance requests across cluster endpoints and
@@ -488,9 +476,6 @@ object StackClient {
     stk.push(Failure.module)
     stk.push(ClientTracingFilter.module)
     stk.push(ForwardAnnotation.module)
-    if (!shouldOffloadEarly) {
-      stk.push(OffloadFilter.client)
-    }
     stk.push(RegistryEntryLifecycle.module)
     stk.push(ClientExceptionTracingFilter.module())
     stk.push(TraceInitializerFilter.clientModule)

finagle-core/src/main/scala/com/twitter/finagle/server/StackServer.scala

@@ -112,15 +112,6 @@ object StackServer {
 
     val stk = new StackBuilder[ServiceFactory[Req, Rep]](stack.nilStack[Req, Rep])
 
-    val shouldOffloadEarly = offloadEarly() || com.twitter.finagle.offload.auto()
-
-    if (!shouldOffloadEarly) {
-      // This module is placed at the bottom of the stack and shifts Future execution context
-      // from IO threads into a configured FuturePool right before user-defined Service.apply is
-      // being called.
-      stk.push(OffloadFilter.server)
-    }
-
     stk.push(ServerTracingFilter.module)
 
     // this goes near the bottom of the stack so it is close to where Service.apply happens.
@@ -165,12 +156,9 @@ object StackServer {
 
     // forks the execution if the current scheduler supports forking
     stk.push(ForkingSchedulerFilter.server)
-
-    if (shouldOffloadEarly) {
-      // This module is placed at the top of the stack and shifts Future execution context
-      // from IO threads into a configured FuturePool right after Netty.
-      stk.push(OffloadFilter.server)
-    }
+    // This module is placed at the top of the stack and shifts Future execution context
+    // from IO threads into a configured FuturePool right after Netty.
+    stk.push(OffloadFilter.server)
 
     // The TraceInitializerFilter must be pushed after most other modules so that
     // any Tracing produced by those modules is enclosed in the appropriate

finagle-core/src/test/scala/com/twitter/finagle/OffloadEarlyTest.scala

@@ -19,23 +19,11 @@ class OffloadEarlyTest extends AnyFunSuite {
     assert(total - offloadAt < offloadAt)
   }
 
-  test("offload early is off by default in clients") {
-    offloadAtTheBottom(StackClient.newStack[Unit, Unit])
+  test("offload early is on by default in clients") {
+    offloadAtTheTop(StackClient.newStack[Unit, Unit])
   }
 
-  test("offload early is off by default in servers") {
-    offloadAtTheTop(StackServer.newStack[Unit, Unit])
-  }
-
-  test("a toggle moves offload filter earlier in clients") {
-    com.twitter.finagle.toggle.flag.overrides.let("com.twitter.finagle.OffloadEarly", 1.0) {
-      offloadAtTheTop(StackClient.newStack[Unit, Unit])
-    }
-  }
-
-  test("a toggle moves offload filter earlier in servers") {
-    com.twitter.finagle.toggle.flag.overrides.let("com.twitter.finagle.OffloadEarly", 1.0) {
-      offloadAtTheBottom(StackServer.newStack[Unit, Unit])
-    }
+  test("offload early is on by default in servers") {
+    offloadAtTheBottom(StackServer.newStack[Unit, Unit])
   }
 }