Remove body read timeout feature

The goal of the feature was to prevent long body reads in GCP, this however does not prevent the slow incoming connection handling at the framework level. Therefore, as this adds unnecessary complexity with possible negative performance impact, the feature is removed. The configuration parameter is no longer used, but can remain as is.
snowplow · Sep 3, 2024 · 1a34e95 · 1a34e95
1 parent ffff1e0
commit 1a34e95
Show file tree

Hide file tree

Showing 25 changed files with 2 additions and 78 deletions.
diff --git a/core/src/main/resources/reference.conf b/core/src/main/resources/reference.conf
@@ -95,7 +95,6 @@
     maxConnections = 1024
     idleTimeout = 610 seconds
     responseHeaderTimeout = 5 seconds
-    bodyReadTimeout = 1 second
     maxRequestLineLength = 20480
     maxHeadersLength = 40960
   }

diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Config.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Config.scala
@@ -161,7 +161,6 @@ object Config {
     maxConnections: Int,
     idleTimeout: FiniteDuration,
     responseHeaderTimeout: FiniteDuration,
-    bodyReadTimeout: FiniteDuration,
     maxRequestLineLength: Int,
     maxHeadersLength: Int
   )

diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Pipes.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Pipes.scala
diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Routes.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Routes.scala
@@ -10,7 +10,6 @@
   */
 package com.snowplowanalytics.snowplow.collector.core
 
-import scala.concurrent.duration.FiniteDuration
 import cats.implicits._
 import cats.effect.{Async, Sync}
 import org.http4s._
@@ -22,7 +21,6 @@ class Routes[F[_]: Async](
   enableDefaultRedirect: Boolean,
   enableRootResponse: Boolean,
   enableCrossdomainTracking: Boolean,
-  bodyReadTimeout: FiniteDuration,
   service: IService[F]
 ) extends Http4sDsl[F] {
 
@@ -51,7 +49,7 @@ class Routes[F[_]: Async](
     case req @ POST -> Root / vendor / version =>
       val path = service.determinePath(vendor, version)
       service.cookie(
-        body          = req.bodyText.through(Pipes.timeoutOnIdle(bodyReadTimeout)).compile.string.map(Some(_)),
+        body          = req.bodyText.compile.string.map(Some(_)),
         path          = path,
         request       = req,
         pixelExpected = false,

diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Run.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/Run.scala
@@ -98,7 +98,6 @@ object Run {
           config.enableDefaultRedirect,
           config.rootResponse.enabled,
           config.crossDomain.enabled,
-          config.networking.responseHeaderTimeout,
           collectorService
         ).value,
         if (config.ssl.enable) config.ssl.port else config.port,

diff --git a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/PipesSpec.scala b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/PipesSpec.scala
diff --git a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/RoutesSpec.scala b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/RoutesSpec.scala
@@ -73,9 +73,7 @@ class RoutesSpec extends Specification {
   ) = {
     val service = new TestService()
     val routes =
-      new Routes(enabledDefaultRedirect, enableRootResponse, enableCrossdomainTracking, 500.millis, service)
-        .value
-        .orNotFound
+      new Routes(enabledDefaultRedirect, enableRootResponse, enableCrossdomainTracking, service).value.orNotFound
     val routesWithHsts = HttpServer.hstsMiddleware(Config.HSTS(enableHsts, 180.days), routes)
     (service, routesWithHsts)
   }

diff --git a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/TestUtils.scala b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/TestUtils.scala
@@ -121,7 +121,6 @@ object TestUtils {
       1024,
       610.seconds,
       5.seconds,
-      1.second,
       20480,
       40960
     ),

diff --git a/kafka/src/it/resources/collector.hocon b/kafka/src/it/resources/collector.hocon
@@ -26,6 +26,5 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 }
diff --git a/...rc/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/KafkaConfigSpec.scala b/...rc/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/KafkaConfigSpec.scala
@@ -172,7 +172,6 @@ object KafkaConfigSpec {
       maxConnections        = 1024,
       idleTimeout           = 610.seconds,
       responseHeaderTimeout = 5.seconds,
-      bodyReadTimeout       = 1.second,
       maxRequestLineLength  = 20480,
       maxHeadersLength      = 40960
     ),

diff --git a/kinesis/src/it/resources/collector-cookie-anonymous.hocon b/kinesis/src/it/resources/collector-cookie-anonymous.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
   "cookie": {

diff --git a/kinesis/src/it/resources/collector-cookie-attributes-1.hocon b/kinesis/src/it/resources/collector-cookie-attributes-1.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
   "cookie": {

diff --git a/kinesis/src/it/resources/collector-cookie-attributes-2.hocon b/kinesis/src/it/resources/collector-cookie-attributes-2.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
   "cookie": {

diff --git a/kinesis/src/it/resources/collector-cookie-domain.hocon b/kinesis/src/it/resources/collector-cookie-domain.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
   "cookie": {

diff --git a/kinesis/src/it/resources/collector-cookie-fallback.hocon b/kinesis/src/it/resources/collector-cookie-fallback.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
   "cookie": {

diff --git a/kinesis/src/it/resources/collector-cookie-no-domain.hocon b/kinesis/src/it/resources/collector-cookie-no-domain.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
   "cookie": {

diff --git a/kinesis/src/it/resources/collector-custom-paths.hocon b/kinesis/src/it/resources/collector-custom-paths.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
 

diff --git a/kinesis/src/it/resources/collector-doNotTrackCookie-disabled.hocon b/kinesis/src/it/resources/collector-doNotTrackCookie-disabled.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
   "doNotTrackCookie": {

diff --git a/kinesis/src/it/resources/collector-doNotTrackCookie-enabled.hocon b/kinesis/src/it/resources/collector-doNotTrackCookie-enabled.hocon
@@ -33,7 +33,6 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 
   "doNotTrackCookie": {

diff --git a/kinesis/src/it/resources/collector.hocon b/kinesis/src/it/resources/collector.hocon
@@ -33,6 +33,5 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 }
diff --git a/...scala/com.snowplowanalytics.snowplow.collectors.scalastream/sinks/KinesisConfigSpec.scala b/...scala/com.snowplowanalytics.snowplow.collectors.scalastream/sinks/KinesisConfigSpec.scala
@@ -126,7 +126,6 @@ object KinesisConfigSpec {
       maxConnections        = 1024,
       idleTimeout           = 610.seconds,
       responseHeaderTimeout = 5.seconds,
-      bodyReadTimeout       = 1.second,
       maxRequestLineLength  = 20480,
       maxHeadersLength      = 40960
     ),

diff --git a/nsq/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/NsqConfigSpec.scala b/nsq/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/NsqConfigSpec.scala
@@ -159,7 +159,6 @@ object NsqConfigSpec {
       maxConnections        = 1024,
       idleTimeout           = 610.seconds,
       responseHeaderTimeout = 5.seconds,
-      bodyReadTimeout       = 1.second,
       maxRequestLineLength  = 20480,
       maxHeadersLength      = 40960
     ),

diff --git a/pubsub/src/it/resources/collector.hocon b/pubsub/src/it/resources/collector.hocon
@@ -18,6 +18,5 @@ collector {
 
   networking {
     responseHeaderTimeout = 10 seconds
-    bodyReadTimeout = 2 seconds
   }
 }
diff --git a/pubsub/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/ConfigSpec.scala b/pubsub/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/ConfigSpec.scala
@@ -115,7 +115,6 @@ object ConfigSpec {
       maxConnections        = 1024,
       idleTimeout           = 610.seconds,
       responseHeaderTimeout = 5.seconds,
-      bodyReadTimeout       = 1.second,
       maxRequestLineLength  = 20480,
       maxHeadersLength      = 40960
     ),

diff --git a/sqs/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/SqsConfigSpec.scala b/sqs/src/test/scala/com.snowplowanalytics.snowplow.collectors.scalastream/SqsConfigSpec.scala
@@ -116,7 +116,6 @@ object SqsConfigSpec {
       maxConnections        = 1024,
       idleTimeout           = 610.seconds,
       responseHeaderTimeout = 5.seconds,
-      bodyReadTimeout       = 1.second,
       maxRequestLineLength  = 20480,
       maxHeadersLength      = 40960
     ),
-Original file line number
+Diff line change
@@ Expand Up / @@ -121,7 +121,6 @@ object TestUtils { @@
 ,
 .seconds,
 .seconds,
-.second,
 ,
         ),
@@ Expand Down @@