finagle/finagle-mysql: Introduce param for configuring INTERACTIVE cl…

…ient flag

Problem

By default, finagle-mysql creates an interactive client, which means the wait_timeout value
comes from the System_variables::net_interactive_timeout setting. However, on the database side,
users may configure the System_variables::net_wait_timeout and expect this to affect their client.

Solution

Introduce an `interactive` param for configuring whether the client is interactive or not. It is interactive by default.

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

CHANGELOG.rst

@@ -10,9 +10,14 @@ Unreleased
 New Features
 ~~~~~~~~~~~~
 
-* finagle-memcached: Implement compressing cache using lz4 compression filter. ``PHAB_ID=D1130236``
 * finagle-core: Implement Toggle aware SimpleFilter. ``PHAB_ID=D1130236``
 
+* finagle-memcached: Implement compressing cache using lz4 compression filter. ``PHAB_ID=D1130236``
+
+* finagle-mysql: Add a param `c.t.f.mysql.param.Interactive` for configuring whether the finagle-mysql
+  client is interactive. It is true by default (the existing behaviour).``PHAB_ID=D1138775``
+
+
 Bug Fixes
 ~~~~~~~~~~
 

finagle-mysql/src/main/scala/com/twitter/finagle/mysql/HandshakeSettings.scala

@@ -3,15 +3,14 @@ package com.twitter.finagle.mysql
 import com.twitter.conversions.StorageUnitOps._
 import com.twitter.finagle.Stack
 import com.twitter.finagle.mysql.MysqlCharset.Utf8_general_ci
-import com.twitter.finagle.mysql.param.{
-  CachingSha2PasswordAuth,
-  CachingSha2PasswordMissServerCache,
-  Charset,
-  Credentials,
-  Database,
-  FoundRows,
-  PathToServerRsaPublicKey
-}
+import com.twitter.finagle.mysql.param.CachingSha2PasswordAuth
+import com.twitter.finagle.mysql.param.CachingSha2PasswordMissServerCache
+import com.twitter.finagle.mysql.param.Charset
+import com.twitter.finagle.mysql.param.Credentials
+import com.twitter.finagle.mysql.param.Database
+import com.twitter.finagle.mysql.param.FoundRows
+import com.twitter.finagle.mysql.param.Interactive
+import com.twitter.finagle.mysql.param.PathToServerRsaPublicKey
 import com.twitter.util.StorageUnit
 
 /**
@@ -32,6 +31,11 @@ import com.twitter.util.StorageUnit
  * of found (matched) rows, not the number of changed rows for
  * UPDATE and INSERT ... ON DUPLICATE KEY UPDATE statements.
  *
+ * @param interactive if the client is interactive or not. If the client
+ * is interactive, System_variables::net_interactive_timeout is used for the
+ * wait_timeout. If the client is not interactive, System_variables::net_wait_timeout
+ * is used.
+ *
  * @param maxPacketSize max size of a command packet that the
  * client intends to send to the server. The largest possible
  * packet that can be transmitted to or from a MySQL 5.5 server or
@@ -55,6 +59,7 @@ private[mysql] final case class HandshakeSettings(
   clientCapabilities: Capability = Capability.baseCapabilities,
   charset: Short = Utf8_general_ci,
   enableFoundRows: Boolean = true,
+  interactive: Boolean = true,
   maxPacketSize: StorageUnit = 1.gigabyte,
   enableCachingSha2PasswordAuth: Boolean = false,
   pathToServerRsaPublicKey: String = "",
@@ -63,10 +68,10 @@ private[mysql] final case class HandshakeSettings(
   require(maxPacketSize <= 1.gigabyte, s"Max packet size ($maxPacketSize) cannot exceed 1 gigabyte")
 
   /**
-   * Optionally adds either or both the `ConnectWithDB` and
-   * `FoundRows` capabilities to the initial `clientCapabilities`
-   * based on the `database` and `enableFoundRows` values
-   * passed in to the constructor.
+   * Optionally adds the `ConnectWithDB`, `FoundRows` `PluginAuth`, and `Interactive`
+   * capabilities to the initial `clientCapabilities`, based on the `database`,
+   * `enableFoundRows`, `enableCachingSha2PasswordAuth`, and  `interactive` values
+   * passed in to the constructor respectively.
    *
    * @note This method does not include the `SSL` capability
    * by default. For one that does, see `sslCalculatedClientCapabilities`.
@@ -76,6 +81,7 @@ private[mysql] final case class HandshakeSettings(
       .set(database.isDefined, Capability.ConnectWithDB)
       .set(enableFoundRows, Capability.FoundRows)
       .set(enableCachingSha2PasswordAuth, Capability.PluginAuth)
+      .set(interactive, Capability.Interactive)
 
   /**
    * Adds the `SSL` capability to the `calculatedClientCapabilities`.
@@ -97,6 +103,7 @@ private[mysql] object HandshakeSettings {
       database = prms[Database].db,
       charset = prms[Charset].charset,
       enableFoundRows = prms[FoundRows].enabled,
+      interactive = prms[Interactive].enabled,
       enableCachingSha2PasswordAuth = prms[CachingSha2PasswordAuth].enabled,
       pathToServerRsaPublicKey = prms[PathToServerRsaPublicKey].path,
       causeAuthCacheMiss = prms[CachingSha2PasswordMissServerCache].causeMiss

finagle-mysql/src/main/scala/com/twitter/finagle/mysql/param/params.scala

@@ -47,6 +47,17 @@ object FoundRows {
   implicit val param: Stack.Param[FoundRows] = Stack.Param(FoundRows(true))
 }
 
+/**
+ * A class eligible for configuring a mysql client's CLIENT_INTERACTIVE flag
+ * during the Handshake phase. If the client is interactive,
+ * System_variables::net_interactive_timeout is used for the wait_timeout. If the client is not
+ * interactive, System_variables::net_wait_timeout is used.
+ */
+case class Interactive(enabled: Boolean)
+object Interactive {
+  implicit val param: Stack.Param[Interactive] = Stack.Param(Interactive(true))
+}
+
 /**
  * A class eligible for configuring the maximum number of prepare
  * statements.  After creating `num` prepare statements, we'll start purging

finagle-mysql/src/test/scala/com/twitter/finagle/mysql/HandshakeSettingsTest.scala

@@ -1,7 +1,11 @@
 package com.twitter.finagle.mysql
 
 import com.twitter.finagle.Stack
-import com.twitter.finagle.mysql.param.{Charset, Credentials, Database, FoundRows}
+import com.twitter.finagle.mysql.param.Charset
+import com.twitter.finagle.mysql.param.Credentials
+import com.twitter.finagle.mysql.param.Database
+import com.twitter.finagle.mysql.param.Interactive
+import com.twitter.finagle.mysql.param.FoundRows
 import org.scalatest.funsuite.AnyFunSuite
 
 class HandshakeSettingsTest extends AnyFunSuite {
@@ -11,6 +15,7 @@ class HandshakeSettingsTest extends AnyFunSuite {
     Capability.MultiResults
   )
   private val withFoundRows = initial + Capability.FoundRows
+  private val withInteractive = initial + Capability.Interactive
 
   test("HandshakeSettings adds FoundRows by default") {
     val settings = HandshakeSettings(clientCapabilities = initial)
@@ -22,6 +27,16 @@ class HandshakeSettingsTest extends AnyFunSuite {
     assert(settings.calculatedClientCapabilities == initial)
   }
 
+  test("HandshakeSettings adds Interactive by default") {
+    val settings = HandshakeSettings(clientCapabilities = initial)
+    assert(settings.calculatedClientCapabilities == withInteractive)
+  }
+
+  test("HandshakeSettings returns initial when interactive is disabled") {
+    val settings = HandshakeSettings(clientCapabilities = initial, interactive = false)
+    assert(settings.calculatedClientCapabilities == initial)
+  }
+
   test("HandshakeSettings adds ConnectWithDB if database is defined") {
     val settings = HandshakeSettings(clientCapabilities = initial, database = Some("test"))
     val withDB = withFoundRows + Capability.ConnectWithDB
@@ -40,13 +55,15 @@ class HandshakeSettingsTest extends AnyFunSuite {
       Charset(MysqlCharset.Binary) +
       Credentials(Some("user123"), Some("pass123")) +
       Database(Some("test")) +
-      FoundRows(false)
+      FoundRows(false) +
+      Interactive(false)
     val settings = HandshakeSettings(params)
     assert(settings.username == Some("user123"))
     assert(settings.password == Some("pass123"))
     assert(settings.database == Some("test"))
     assert(settings.charset == MysqlCharset.Binary)
     assert(!settings.enableFoundRows)
+    assert(!settings.interactive)
   }
 
 }

finagle-mysql/src/test/scala/com/twitter/finagle/mysql/unit/CapabilityTest.scala

@@ -45,6 +45,11 @@ class CapabilityTest extends AnyFunSuite {
     assert(withFoundRows.has(Capability.FoundRows))
     val withoutFoundRows = c.set(false, Capability.FoundRows)
     assert(!withoutFoundRows.has(Capability.FoundRows))
+
+    val withInteractive = c.set(true, Capability.Interactive)
+    assert(withInteractive.has(Capability.Interactive))
+    val withoutInteractive = c.set(false, Capability.Interactive)
+    assert(!withoutInteractive.has(Capability.Interactive))
   }
 
   test("toString lists capabilities") {