feat(proxy): expose target pool tuning, retry windows and request timeouts

lib/kamal/configuration/docs/proxy.yml

@@ -242,19 +242,78 @@ proxy:
   # will forward them if it is set to `false`.
   forward_headers: true
 
-  # Response timeout
+  # Two different deadlines
   #
-  # How long to wait for requests to complete before timing out, defaults to 30 seconds:
+  # `response_timeout` bounds how long the app may take to *start* answering —
+  # its clock stops once response headers arrive. `request_timeout` bounds the
+  # whole request, including streaming the body back to the client.
+  #
+  # They are not interchangeable, and the usual confusion is setting the first
+  # and still seeing requests hang: a slow trickle of body bytes never trips
+  # `response_timeout`, because the app answered promptly. WebSocket and
+  # event-stream responses are exempt from `request_timeout`.
+  #
+  # `response_timeout` defaults to 30 seconds; `request_timeout` defaults to 0,
+  # meaning no limit.
   response_timeout: 10
+  request_timeout: 30
 
   # Per-path timeouts
   #
-  # Override the response timeout for requests under specific path prefixes.
-  # Values are Go duration strings ("90s", "5m") or plain seconds; 0 disables
-  # the timeout entirely, which suits streaming and SSE endpoints:
+  # Override either deadline below a path prefix. Values are Go duration strings
+  # ("90s", "5m") or plain seconds; 0 removes the limit for that prefix, which
+  # suits streaming and SSE endpoints.
   path_timeouts:
     "/api/reports": "5m"
     "/stream": 0
+  path_request_timeouts:
+    "/uploads": "10m"
+    "/stream": 0
+
+  # Target connections and retries
+  #
+  # The connection pool between the proxy and this app's containers, and how
+  # hard the proxy tries to place a request on a healthy one.
+  #
+  # `max_conns` caps simultaneous connections per target, idle ones included.
+  # Requests over the cap queue rather than failing, and a streaming response
+  # holds its connection until the body closes — so pair a low cap with
+  # `request_timeout`. `max_idle_conns` caps the idle connections kept open.
+  # Both apply *per pool*, and every entry in `path_timeouts` adds another pool.
+  #
+  # `idle_conn_timeout` is how long an idle connection is kept; set it below the
+  # app's own keep-alive timeout, or the proxy will hand a request to a socket
+  # the app has already closed. `dial_timeout` bounds establishing a connection
+  # — `response_timeout` cannot cover that, since its clock starts only once the
+  # request has been written.
+  #
+  # `try_duration` keeps looking for a healthy target for that long before
+  # giving up, with `try_interval` between attempts. Only idempotent requests
+  # without a body are re-sent to another target.
+  #
+  # ### What 0 means here
+  #
+  # Not the same thing for every key, because the proxy resolves its own
+  # defaults from a zero:
+  #
+  # * `max_conns: 0` — unlimited
+  # * `try_duration: 0` — a single attempt, no retrying
+  # * `request_timeout: 0` — no limit
+  # * `max_idle_conns: 0` — the proxy's default of 100, **not** "keep none"
+  # * `idle_conn_timeout: 0` — the proxy's default of 90s
+  # * `dial_timeout: 0` — the proxy's default of 30s
+  # * `try_interval: 0` — the proxy's default of 250ms
+  #
+  # So there is no way to ask for zero idle connections; leave the key unset
+  # unless you mean to change it.
+  target:
+    max_conns: 100
+    max_idle_conns: 10
+    idle_conn_timeout: 90
+    dial_timeout: 5
+    disable_keep_alives: false
+    try_duration: 30
+    try_interval: 1
 
   # Path-based routing
   #

lib/kamal/configuration/proxy.rb

@@ -166,7 +166,9 @@ def deploy_options
       "read-target": proxy_config["read_targets"].presence,
       "read-target-websockets": proxy_config["read_target_websockets"] ? true : nil,
       "writer-affinity-timeout": seconds_duration(proxy_config["writer_affinity_timeout"]),
-      "path-timeout": path_timeout_args,
+      "path-timeout": path_timeout_args("path_timeouts"),
+      "request-timeout": seconds_duration(proxy_config["request_timeout"]),
+      "path-request-timeout": path_timeout_args("path_request_timeouts"),
       "buffer-requests": proxy_config.fetch("buffering", { "requests": true }).fetch("requests", true),
       "buffer-responses": proxy_config.fetch("buffering", { "responses": true }).fetch("responses", true),
       "buffer-memory": proxy_config.dig("buffering", "memory"),
@@ -181,7 +183,8 @@ def deploy_options
       "log-response-header": proxy_config.dig("logging", "response_headers"),
       "error-pages": error_pages
     }.merge(tls_domains_options).merge(tls_options).merge(cache_options).merge(compress_options)
-      .merge(access_control_options).merge(traffic_options).merge(lifecycle_options).compact
+      .merge(access_control_options).merge(traffic_options).merge(lifecycle_options)
+      .merge(target_options).compact
 
     if load_balancing?
       opts.delete(:host)
@@ -276,6 +279,27 @@ def tls_options
       }.compact
     end
 
+    # The connection pool between the proxy and this app's containers, and how
+    # hard the proxy tries to place a request on a healthy one.
+    #
+    # Every value is passed through exactly as written. kamal-proxy resolves its
+    # own defaults from a zero (target_pool.go), and it has to do that server
+    # side because restored state and older RPC clients never see the CLI — so
+    # substituting a default here would be both redundant and wrong.
+    def target_options
+      target = proxy_config["target"] || {}
+
+      {
+        "target-max-conns": target["max_conns"],
+        "target-max-idle-conns": target["max_idle_conns"],
+        "target-idle-conn-timeout": seconds_duration(target["idle_conn_timeout"]),
+        "target-dial-timeout": seconds_duration(target["dial_timeout"]),
+        "target-disable-keep-alives": target["disable_keep_alives"] ? true : nil,
+        "target-try-duration": seconds_duration(target["try_duration"]),
+        "target-try-interval": seconds_duration(target["try_interval"])
+      }.compact
+    end
+
     # Session affinity and scale-to-zero: which target a client keeps, and
     # whether the targets are running at all.
     #
@@ -424,8 +448,11 @@ def basic_auth_credential
       "#{basic_auth["username"]}:#{password}"
     end
 
-    def path_timeout_args
-      if (timeouts = proxy_config["path_timeouts"]).present?
+    # Serves both --path-timeout and --path-request-timeout, which kamal-proxy
+    # reads with one parser. A duration may be a Go string ("5m") or plain
+    # seconds; 0 is a value, meaning no limit for that prefix.
+    def path_timeout_args(key)
+      if (timeouts = proxy_config[key]).present?
         timeouts.map do |prefix, duration|
           duration = seconds_duration(duration) if duration.is_a?(Numeric)
           "#{prefix}=#{duration}"

lib/kamal/configuration/validator.rb

@@ -47,7 +47,7 @@ def validate_against_example!(validation_config, example)
               case key.to_s
               when "options", "args"
                 validate_type! value, Hash
-              when "labels", "path_timeouts"
+              when "labels", "path_timeouts", "path_request_timeouts"
                 validate_hash_of! value, example_value.first[1].class
               else
                 validate_against_example! value, example_value unless validate_key_override!(key, value)

lib/kamal/configuration/validator/proxy.rb

@@ -57,6 +57,7 @@ def validate!
       validate_access_control!
       validate_traffic_shaping!
       validate_lifecycle!
+      validate_tuning!
 
       if run_config = config["run"]
         if run_config["bind_ips"].present?
@@ -181,6 +182,41 @@ def validate_tls!(tls)
     # '<name>: <value>' encoding kamal-proxy cuts at the first colon.
     TOKEN = /\A[!#$%&'*+\-.^_`|~0-9A-Za-z]+\z/
 
+    POOL_KEYS = %w[ max_conns max_idle_conns idle_conn_timeout dial_timeout try_duration try_interval ].freeze
+
+    # A zero is meaningful for most of these, so only negatives are refused.
+    # kamal-proxy clamps them rather than failing when it reads saved state, and
+    # rejects them from a client — this is the client.
+    def validate_tuning!
+      target = config["target"] || {}
+
+      with_context("target") do
+        POOL_KEYS.each do |key|
+          error "#{key} cannot be negative" if target[key].to_f.negative?
+        end
+
+        # A retry interval paces attempts within a duration; without one there is
+        # only ever a single attempt for it to pace.
+        if target["try_interval"].present? && target["try_duration"].to_i.zero?
+          error "try_interval has no effect without try_duration"
+        end
+      end
+
+      if config["request_timeout"].to_f.negative?
+        with_context("request_timeout") { error "cannot be negative" }
+      end
+
+      # Both maps reach kamal-proxy's one parsePathTimeouts, which refuses a
+      # negative duration.
+      %w[ path_timeouts path_request_timeouts ].each do |key|
+        with_context(key) do
+          (config[key] || {}).each do |prefix, duration|
+            error "'#{prefix}' cannot be negative" if duration.is_a?(Numeric) && duration.negative?
+          end
+        end
+      end
+    end
+
     # Session affinity and scale-to-zero. The one rule NOT here is that sleep
     # needs proxy/run/docker_socket: a role can carry `sleep` while the root
     # carries the socket, and this validator runs against the role's own proxy

test/configuration/proxy/tuning_test.rb

@@ -0,0 +1,142 @@
+require "test_helper"
+
+class ConfigurationProxyTuningTest < ActiveSupport::TestCase
+  setup do
+    @deploy = {
+      service: "app", image: "dhh/app", registry: { "username" => "dhh", "password" => "secret" },
+      builder: { "arch" => "amd64" }, servers: [ "1.1.1.1" ]
+    }
+  end
+
+  test "no tuning keys leave the deploy command unchanged" do
+    options = deploy_options({})
+
+    assert_empty options.keys.grep(/^target-(max|idle|dial|disable|try)/)
+    assert_not options.key?(:"request-timeout")
+    assert_not options.key?(:"path-request-timeout")
+  end
+
+  # AC1 — two different deadlines, both reachable, named apart.
+  test "response_timeout and request_timeout are separate flags" do
+    options = deploy_options "response_timeout" => 20, "request_timeout" => 30
+
+    assert_equal "20s", options[:"target-timeout"]
+    assert_equal "30s", options[:"request-timeout"]
+  end
+
+  # AC2 — the same builder, not a parallel copy.
+  test "both path timeout maps are built by the same code path" do
+    options = deploy_options \
+      "path_timeouts" => { "/uploads" => 300 },
+      "path_request_timeouts" => { "/uploads" => 600, "/stream" => 0 }
+
+    assert_equal [ "/uploads=300s" ], options[:"path-timeout"]
+    assert_equal [ "/uploads=600s", "/stream=0s" ], options[:"path-request-timeout"]
+  end
+
+  test "a path timeout may be given as a go duration string" do
+    assert_equal [ "/reports=5m" ], deploy_options("path_request_timeouts" => { "/reports" => "5m" })[:"path-request-timeout"]
+  end
+
+  test "the target block becomes the pool and retry flags" do
+    options = deploy_options "target" => {
+      "max_conns" => 100, "max_idle_conns" => 10, "idle_conn_timeout" => 90,
+      "dial_timeout" => 5, "disable_keep_alives" => true,
+      "try_duration" => 30, "try_interval" => 1
+    }
+
+    assert_equal 100, options[:"target-max-conns"]
+    assert_equal 10, options[:"target-max-idle-conns"]
+    assert_equal "90s", options[:"target-idle-conn-timeout"]
+    assert_equal "5s", options[:"target-dial-timeout"]
+    assert_equal true, options[:"target-disable-keep-alives"]
+    assert_equal "30s", options[:"target-try-duration"]
+    assert_equal "1s", options[:"target-try-interval"]
+  end
+
+  test "disable_keep_alives false emits nothing, matching the proxy default" do
+    assert_not deploy_options("target" => { "disable_keep_alives" => false }).key?(:"target-disable-keep-alives")
+  end
+
+  # AC3 — the bug e1e5c10 fixed on the proxy side. A literal 0 is a value, not
+  # an absence: unlimited for max_conns, no limit for request_timeout, a single
+  # attempt for try_duration.
+  test "an explicit zero reaches the proxy rather than being dropped" do
+    options = deploy_options \
+      "request_timeout" => 0,
+      "target" => {
+        "max_conns" => 0, "max_idle_conns" => 0, "idle_conn_timeout" => 0,
+        "dial_timeout" => 0, "try_duration" => 0
+      }
+
+    assert_equal "0s", options[:"request-timeout"]
+    assert_equal 0, options[:"target-max-conns"]
+    assert_equal 0, options[:"target-max-idle-conns"]
+    assert_equal "0s", options[:"target-idle-conn-timeout"]
+    assert_equal "0s", options[:"target-dial-timeout"]
+    assert_equal "0s", options[:"target-try-duration"]
+  end
+
+  test "an explicit zero survives all the way to the command line" do
+    args = configuration("request_timeout" => 0, "target" => { "max_conns" => 0 })
+      .proxy.deploy_command_args(target: "1.1.1.1")
+
+    assert_includes args, "--request-timeout=\"0s\""
+    assert_includes args, "--target-max-conns=\"0\""
+  end
+
+  # Validation
+
+  test "negative pool values are rejected" do
+    {
+      "max_conns" => "max_conns", "max_idle_conns" => "max_idle_conns",
+      "idle_conn_timeout" => "idle_conn_timeout", "dial_timeout" => "dial_timeout"
+    }.each_key do |key|
+      error = assert_raises(Kamal::ConfigurationError) { configuration "target" => { key => -1 } }
+      assert_equal "proxy/target: #{key} cannot be negative", error.message
+    end
+  end
+
+  test "negative retry values are rejected" do
+    error = assert_raises(Kamal::ConfigurationError) { configuration "target" => { "try_duration" => -1 } }
+    assert_equal "proxy/target: try_duration cannot be negative", error.message
+  end
+
+  test "try_interval without try_duration is rejected" do
+    error = assert_raises(Kamal::ConfigurationError) { configuration "target" => { "try_interval" => 1 } }
+
+    assert_equal "proxy/target: try_interval has no effect without try_duration", error.message
+  end
+
+  test "try_interval with a zero try_duration is rejected too" do
+    error = assert_raises(Kamal::ConfigurationError) do
+      configuration "target" => { "try_duration" => 0, "try_interval" => 1 }
+    end
+
+    assert_equal "proxy/target: try_interval has no effect without try_duration", error.message
+  end
+
+  test "a negative request_timeout is rejected" do
+    error = assert_raises(Kamal::ConfigurationError) { configuration "request_timeout" => -1 }
+
+    assert_equal "proxy/request_timeout: cannot be negative", error.message
+  end
+
+  # kamal-proxy's parsePathTimeouts rejects these; both maps go through it.
+  test "a negative path timeout is rejected in either map" do
+    assert_equal "proxy/path_timeouts: '/uploads' cannot be negative",
+      assert_raises(Kamal::ConfigurationError) { configuration "path_timeouts" => { "/uploads" => -1 } }.message
+
+    assert_equal "proxy/path_request_timeouts: '/uploads' cannot be negative",
+      assert_raises(Kamal::ConfigurationError) { configuration "path_request_timeouts" => { "/uploads" => -1 } }.message
+  end
+
+  private
+    def configuration(proxy_config)
+      Kamal::Configuration.new @deploy.merge(proxy: proxy_config)
+    end
+
+    def deploy_options(proxy_config)
+      configuration(proxy_config).proxy.deploy_options
+    end
+end

test/fixtures/deploy_with_every_proxy_option.yml

@@ -44,6 +44,17 @@ proxy:
   writer_affinity_timeout: 30
   path_timeouts:
     /uploads: 300
+  request_timeout: 30
+  path_request_timeouts:
+    /uploads: 600
+  target:
+    max_conns: 100
+    max_idle_conns: 10
+    idle_conn_timeout: 90
+    dial_timeout: 5
+    disable_keep_alives: true
+    try_duration: 30
+    try_interval: 1
   healthcheck:
     path: /up
     interval: 2

test/proxy_flag_coverage_test.rb

@@ -37,9 +37,6 @@ class ProxyFlagCoverageTest < ActiveSupport::TestCase
   # list shrinking to empty is what "R7 is done" means.
   R7_BACKLOG = {
     "deploy" => {
-      80 => %w[ target-dial-timeout target-disable-keep-alives target-idle-conn-timeout
-                target-max-conns target-max-idle-conns target-try-duration target-try-interval
-                request-timeout path-request-timeout ],
       81 => %w[ exclude-metrics-path ]
     },
     "run" => {