Reuse connections to KB (keep-alive) through a fixed persistent adapter (1.4.0)

CHANGELOG.md

@@ -6,7 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [unreleased]
-- See diff: https://github.com/barkibu/kb-ruby/compare/v1.3.0...HEAD
+- See diff: https://github.com/barkibu/kb-ruby/compare/v1.4.0...HEAD
+
+## [1.4.0]
+- Reuse connections to KB (keep-alive) through `KB::PersistentAdapter`, a subclass of faraday-net_http_persistent 1.2's adapter over net-http-persistent 4. New runtime dependencies: `faraday-net_http_persistent ~> 1.2`, `net-http-persistent ~> 4.0` (4.0.8 accepts `connection_pool` 2.2.4 up to < 4). On by default; `KB.config.request.keep_alive = false` restores one connection per call. New `KB.config.request.idle_timeout` (default 30s).
+- One `Net::HTTP::Persistent` per process is shared by every `KB::Client`, so all model clients reuse the same connections to the KB host.
+- Fixes over the stock adapter: timeouts are set on the checked-out connection, not on the shared object (the stock adapter would leak one call's `read_timeout:` override into other threads' calls); SSL options are not re-applied per client (the stock adapter's per-instance cert store would drop every TLS connection whenever a different model client made a call); a connect timeout stays `Faraday::ConnectionFailed` and `Net::HTTP::Persistent::Error` becomes `Faraday::ConnectionFailed` wrapping the underlying `Errno`, as with faraday-net_http.
+- `request.kb_client` payload gains `connections` (`"new"`/`"reused"` per attempt); the Datadog subscriber tags it as `kb.connections`. `Errno::EHOSTDOWN` joins the never-sent retry class.
 
 ## [1.3.0]
 - Retry transport failures once (`faraday-retry`, already in the Faraday 1.10 bundle, now an explicit dependency). `KB::RetryPolicy` decides by the underlying error, not the Faraday class: failures where the request never left (`Net::OpenTimeout`, `ECONNREFUSED`, `EHOSTUNREACH`, `ENETUNREACH`, `EADDRNOTAVAIL`, `SocketError`) retry for every verb; any other transport failure (read/write timeout, reset, EOF, TLS) retries for GET/HEAD only (and not when the call raised its own `read_timeout:`, so a 30s read isn't doubled); HTTP error responses never retry. New settings `KB.config.request.retries` (default 1, 0 disables) and `retry_interval` (default 0.1s, randomized up to 2x). Worst-case latency is now two attempts' worth of phase budgets.

Gemfile.lock

@@ -1,18 +1,20 @@
 PATH
   remote: .
   specs:
-    barkibu-kb (1.3.0)
+    barkibu-kb (1.4.0)
       activemodel (>= 4.0.2)
       activerecord
       activesupport (>= 3.0.0)
       dry-configurable (~> 0.9)
       faraday
       faraday-net_http (~> 1.0)
+      faraday-net_http_persistent (~> 1.2)
       faraday-retry (~> 1.0)
       faraday_middleware
       i18n
-    barkibu-kb-fake (1.3.0)
-      barkibu-kb (= 1.3.0)
+      net-http-persistent (~> 4.0)
+    barkibu-kb-fake (1.4.0)
+      barkibu-kb (= 1.4.0)
       countries
       sinatra
       webmock
@@ -110,6 +112,8 @@ GEM
     multipart-post (2.3.0)
     mustermann (3.0.0)
       ruby2_keywords (~> 0.0.1)
+    net-http-persistent (4.0.8)
+      connection_pool (>= 2.2.4, < 4)
     parallel (1.24.0)
     parser (3.3.0.5)
       ast (~> 2.4.1)

README.md

@@ -106,14 +106,54 @@ Worst case, a call now takes two attempts' worth of phase budgets plus the
 interval, e.g. a GET that read-times-out twice takes about 2 x (1 + 3 + 5)s with
 the default timeouts.
 
+#### Keep-alive connections
+
+KB calls reuse TCP/TLS connections instead of opening a new one per call
+(`KB::PersistentAdapter`, net-http-persistent under faraday-net_http_persistent).
+One pool per process is shared by every model's client, and each thread checks a
+connection out per call. A pooled connection that has been idle longer than
+`idle_timeout` is closed and reopened on the next call.
+
+```ruby
+# config/initializers/kb_ruby.rb
+KB.config.request.keep_alive = true # default; false opens a connection per call (faraday-net_http)
+KB.config.request.idle_timeout = 30 # default, seconds
+```
+
+Keep `idle_timeout` below the Heroku router's own idle close: it drops an idle
+client connection after about 55 seconds (measured against KB staging, both
+`kb-staging.barkibu.com` and the herokuapp.com host, 2026-09-23). A connection the
+router already closed is detected before reuse and reopened. A close that races
+the next request in the same instant is not detectable: a GET is retried on a
+fresh connection, a POST fails as `Faraday::ConnectionFailed` wrapping
+`EOFError`. Staying below the router's limit keeps that race away.
+
+Per-call timeouts (`read_timeout:` on `KB::Client#request`) apply to that call
+only, and error classes are the same as without keep-alive: a connect timeout is
+`Faraday::ConnectionFailed` wrapping `Net::OpenTimeout`, a refused connection is
+`Faraday::ConnectionFailed` wrapping `Errno::ECONNREFUSED`.
+
+Two differences from `keep_alive = false`: `connect_timeout` and `idle_timeout`
+are read once per process, when the shared pool is built on the first KB call
+(set them in an initializer, or call `KB::PersistentAdapter.reset!` after changing
+them), and `HTTP(S)_PROXY` environment variables are not honoured.
+
+On forking servers (Puma cluster, Sidekiq swarm), connection_pool drops pooled
+connections in the child after fork on Ruby >= 3.1. A preloading parent that
+calls KB before forking shares those sockets with its children; the parent
+recovers by reconnecting on the next call.
+
 #### Instrumentation
 
 Every KB call emits one `request.kb_client` event through
 `ActiveSupport::Notifications`, wrapping the whole call: cache lookup, TCP
 connect, TLS, write, read and JSON parsing. The payload carries `verb`, `path`,
 `base_url`, `cache_hit` (GET calls only), `status` (when a response arrived),
 `retries` and `retry_errors` (only when the call was retried: the count, and the
-underlying error class of each failed attempt, e.g. `["Net::OpenTimeout"]`) and
+underlying error class of each failed attempt, e.g. `["Net::OpenTimeout"]`),
+`connections` (with keep-alive: `"new"` or `"reused"` per attempt, e.g.
+`["reused", "new"]` for a call whose reused connection failed and whose retry
+opened a fresh one) and
 ActiveSupport's `exception` / `exception_object` when the call raised. The event
 covers the whole call including retries, so `exception` is set only when every
 attempt failed. Subscribe to it for logging, metrics or anything else:
@@ -145,8 +185,9 @@ tracer's own Net::HTTP spans nest under it. It inherits the app's service
 knowledge-base service either: it measures the client's whole call, not a KB
 operation. Resources are low-cardinality (`GET /v1/pets/birthdays`,
 `GET /v1/pets/?/contracts`). Tags: `peer.hostname` (the KB host used),
-`kb.method`, `kb.cache_hit` (GET calls only), `http.status_code`, `kb.retries`
-and `kb.retry_errors` (retried calls only), plus the standard
+`kb.method`, `kb.cache_hit` (GET calls only), `http.status_code`,
+`kb.connections` (e.g. `reused` or `reused,new`), `kb.retries` and
+`kb.retry_errors` (retried calls only), plus the standard
 `error.type`/`error.message` when the call raises. A span with `kb.retries` and
 no error is a failure the retry absorbed.
 

barkibu-kb.gemspec

@@ -53,7 +53,9 @@ Gem::Specification.new do |spec|
   spec.add_runtime_dependency 'activesupport', '>= 3.0.0'
   spec.add_runtime_dependency 'faraday'
   spec.add_runtime_dependency 'faraday-net_http', '~> 1.0'
+  spec.add_runtime_dependency 'faraday-net_http_persistent', '~> 1.2'
   spec.add_runtime_dependency 'faraday_middleware'
   spec.add_runtime_dependency 'faraday-retry', '~> 1.0'
   spec.add_runtime_dependency 'i18n'
+  spec.add_runtime_dependency 'net-http-persistent', '~> 4.0'
 end

lib/barkibu-kb.rb

@@ -24,6 +24,12 @@ module KB
     # Retries after a transport failure, per KB::RetryPolicy. 0 disables retries.
     setting :retries, default: 1
     setting :retry_interval, default: 0.1
+    # Reuse connections to KB across calls (KB::PersistentAdapter). false falls
+    # back to one connection per call (faraday-net_http).
+    setting :keep_alive, default: true
+    # Seconds an idle pooled connection is kept before the next call reconnects.
+    # Keep it below the Heroku router's idle close (~55s, measured on KB staging).
+    setting :idle_timeout, default: 30
   end
 end
 
@@ -33,6 +39,7 @@ module KB
 require 'kb/client_resolver'
 require 'kb/errors'
 require 'kb/retry_policy'
+require 'kb/persistent_adapter'
 require 'kb/client'
 
 require 'kb/concerns'

lib/kb/client.rb

@@ -3,6 +3,7 @@ class Client
     # Emitted once per KB call, wrapping cache lookup and the HTTP request.
     # Payload: verb, path, base_url, cache_hit (GET only), status (when a response arrived),
     # retries / retry_errors (only when the call was retried, see KB::RetryPolicy),
+    # connections ("new" / "reused" per attempt, keep-alive transport only),
     # plus ActiveSupport's exception/exception_object when the call raised.
     REQUEST_EVENT = 'request.kb_client'.freeze
 
@@ -113,7 +114,7 @@ def connection
             logger.filter(/(X-api-key:\s)("\w+")/, '\1[API_KEY_SCRUBBED]')
           end
         end
-        conn.adapter :net_http
+        conn.adapter(KB.config.request.keep_alive ? KB::PersistentAdapter : :net_http)
       end
     end
 

lib/kb/instrumentation/datadog.rb

@@ -72,16 +72,18 @@ def finish(_name, _id, payload)
 
           span.set_tag('kb.cache_hit', payload[:cache_hit].to_s) if payload.key?(:cache_hit)
           span.set_tag('http.status_code', payload[:status].to_s) if payload[:status]
-          tag_retries(span, payload)
+          tag_transport(span, payload)
           span.set_error(payload[:exception_object]) if payload[:exception_object]
           span.finish
         end
 
         private
 
+        # `kb.connections`: "new"/"reused" per attempt (keep-alive transport).
         # Only on retried calls: `kb.retries` (numeric) and the distinct underlying
         # errors that triggered them, e.g. `Net::OpenTimeout`.
-        def tag_retries(span, payload)
+        def tag_transport(span, payload)
+          span.set_tag('kb.connections', payload[:connections].join(',')) if payload[:connections]
           return unless payload[:retries]
 
           span.set_tag('kb.retries', payload[:retries])

lib/kb/persistent_adapter.rb

@@ -0,0 +1,139 @@
+require 'faraday/net_http_persistent'
+require 'net/http/persistent'
+
+module KB
+  # Keep-alive transport for KB calls: faraday-net_http_persistent 1.2 with the
+  # fixes it needs to be a drop-in for the net_http adapter.
+  #
+  # - One Net::HTTP::Persistent per process, shared by every KB::Client. Each
+  #   model class memoizes its own client, but the pool is keyed by host and
+  #   port, so all of them reuse the same connections to KB.
+  # - Timeouts go on the checked-out connection, never on the shared
+  #   Net::HTTP::Persistent. The stock adapter writes every request's timeouts
+  #   onto the shared object, so one call's `read_timeout:` override (e.g. 30s
+  #   for birthdays) would leak into whatever another thread sends next.
+  # - SSL is left to Net::HTTP::Persistent's defaults (see #configure_ssl).
+  # - Error classes match the net_http adapter: a connect timeout stays
+  #   Faraday::ConnectionFailed (the stock adapter raises TimeoutError), and
+  #   Net::HTTP::Persistent::Error ("connection refused", "host down") becomes
+  #   Faraday::ConnectionFailed wrapping the underlying Errno instead of leaking
+  #   raw past KB::Error.
+  # - Each attempt reports whether it opened a new connection or reused one, into
+  #   the request.kb_client event as `connections` (e.g. ["reused", "new"]).
+  #   "reused" means the pool handed out an already-open connection. Net::HTTP may
+  #   still reconnect it silently if it sees the peer closed it; a failure that
+  #   never reached KB (connect timeout, refused) is always reported as "new".
+  #
+  # Differences from faraday-net_http, on purpose: connect_timeout and
+  # idle_timeout are read once per process, when the shared pool is built (call
+  # `reset!` after changing them at runtime), and HTTP(S)_PROXY is not honoured.
+  class PersistentAdapter < Faraday::Adapter::NetHttpPersistent
+    CURRENT_ATTEMPT = :kb_persistent_attempt
+    MUTEX = Mutex.new
+
+    class << self
+      def http
+        @http || MUTEX.synchronize { @http ||= build_http }
+      end
+
+      # Closes every pooled connection; the next call builds a fresh pool with
+      # the current KB.config.
+      def reset!
+        MUTEX.synchronize do
+          @http&.shutdown
+          @http = nil
+        end
+      end
+
+      private
+
+      def build_http
+        HTTP.new(name: 'kb-ruby').tap do |http|
+          http.idle_timeout = KB.config.request.idle_timeout
+          http.open_timeout = KB.config.request.connect_timeout
+          http.max_retries = 0 # retries belong to KB::RetryPolicy
+        end
+      end
+    end
+
+    # One request's view of the connection it was given.
+    Attempt = Struct.new(:options, :connection, :error) do
+      def apply(persistent_connection)
+        apply_timeouts(persistent_connection.http)
+        self.connection = persistent_connection.requests.zero? ? 'new' : 'reused'
+      end
+
+      def apply_timeouts(http)
+        http.open_timeout = options.open_timeout if options.open_timeout # Net::HTTP's own reconnects
+        http.read_timeout = options.read_timeout if options.read_timeout
+        http.write_timeout = options.write_timeout if options.write_timeout
+      end
+
+      # A failure before a connection was handed out, or one that never reached
+      # KB, happened while opening a connection.
+      def label
+        return 'new' if connection.nil? || (error && RetryPolicy.not_sent?(error))
+
+        connection
+      end
+    end
+
+    class HTTP < Net::HTTP::Persistent
+      def connection_for(uri)
+        super do |connection|
+          Thread.current[CURRENT_ATTEMPT]&.apply(connection)
+          yield connection
+        end
+      end
+    end
+
+    private
+
+    def net_http_connection(_env)
+      self.class.http
+    end
+
+    # Timeouts are applied per checked-out connection (see Attempt#apply).
+    def configure_request(_http, _req); end
+
+    # KB clients pass no SSL options, and Net::HTTP::Persistent's defaults are
+    # the same as Faraday's (VERIFY_PEER, system CA store). The stock method would
+    # set a cert store object per adapter instance, i.e. per model client, and
+    # every change of store object makes Net::HTTP::Persistent drop all TLS
+    # connections, so alternating model clients would never reuse one.
+    def configure_ssl(_http, _ssl); end
+
+    def perform_request(http, env)
+      attempt = Attempt.new(env[:request])
+      Thread.current[CURRENT_ATTEMPT] = attempt
+      super
+    rescue StandardError => e
+      attempt.error = e
+      raise normalize(e)
+    ensure
+      Thread.current[CURRENT_ATTEMPT] = nil
+      record(env, attempt)
+    end
+
+    # Gives the Faraday errors faraday-net_http gives for the same failure. The
+    # stock adapter's perform_request rescues first and differs in two ways:
+    # - Net::OpenTimeout becomes TimeoutError; faraday-net_http says ConnectionFailed.
+    # - "connection refused" / "host down" arrive as Net::HTTP::Persistent::Error,
+    #   with the real Errno as its `cause`. The stock adapter wraps it (refused)
+    #   or re-raises it raw (host down, not a Faraday error at all). Here it
+    #   becomes ConnectionFailed wrapping the Errno, so RetryPolicy.root_cause
+    #   finds it one level down.
+    def normalize(error)
+      cause = error.is_a?(Faraday::Error) ? error.wrapped_exception : error
+      return Faraday::ConnectionFailed.new(cause) if cause.is_a?(Net::OpenTimeout)
+      return Faraday::ConnectionFailed.new(cause.cause || cause) if cause.is_a?(Net::HTTP::Persistent::Error)
+
+      error
+    end
+
+    def record(env, attempt)
+      event = env[:request].context&.dig(:kb_event)
+      (event[:connections] ||= []) << attempt.label if event && attempt
+    end
+  end
+end

lib/kb/retry_policy.rb

@@ -28,6 +28,7 @@ module RetryPolicy
       Errno::EHOSTUNREACH,
       Errno::ENETUNREACH,
       Errno::EADDRNOTAVAIL,
+      Errno::EHOSTDOWN,
       SocketError # DNS resolution
     ].freeze
     TRANSPORT_ERRORS = [Faraday::ConnectionFailed, Faraday::TimeoutError, Faraday::SSLError].freeze
@@ -49,7 +50,8 @@ def not_sent?(error)
     # The Ruby error behind a Faraday error. `wrapped_exception` is Faraday's own
     # explicit link to it (Ruby's `cause` is only whatever was being rescued at
     # the raise, usually the same object). Faraday's adapters wrap the Ruby error
-    # one level deep, so one level is enough.
+    # one level deep, so one level is enough; KB::PersistentAdapter#normalize
+    # flattens the stock persistent adapter's deeper nesting to keep it that way.
     def root_cause(error)
       (error.respond_to?(:wrapped_exception) && error.wrapped_exception) || error.cause || error
     end

lib/kb/version.rb

@@ -1,3 +1,3 @@
 module KB
-  VERSION = '1.3.0'.freeze
+  VERSION = '1.4.0'.freeze
 end

spec/client_notifications_spec.rb

@@ -22,7 +22,7 @@
       client.request('birthdays', filters: { month: 9 })
 
       expect(events.last).to eq(verb: :get, path: 'birthdays', base_url: 'http://kb.test/v1/pets',
-                                cache_hit: false, status: 200)
+                                cache_hit: false, status: 200, connections: ['new'])
     end
 
     it 'describes a write without a cache flag' do
@@ -31,7 +31,8 @@
 
       client.create(name: 'Rex')
 
-      expect(events.last).to eq(verb: :post, path: '', base_url: 'http://kb.test/v1/pets', status: 201)
+      expect(events.last).to eq(verb: :post, path: '', base_url: 'http://kb.test/v1/pets', status: 201,
+                                connections: ['new'])
     end
 
     it 'keeps the status code and the exception when the API answers an error' do

spec/client_timeouts_spec.rb

@@ -10,8 +10,21 @@
     Faraday::Adapter::NetHttp.new(nil).build_connection(env)
   end
 
-  it 'uses the net_http adapter' do
-    expect(connection.adapter).to eq Faraday::Adapter::NetHttp
+  it 'uses the keep-alive adapter by default' do
+    expect(connection.adapter).to eq KB::PersistentAdapter
+  end
+
+  context 'with keep_alive disabled' do
+    around do |example|
+      KB.config.request.keep_alive = false
+      example.run
+    ensure
+      KB.config.request.keep_alive = true
+    end
+
+    it 'falls back to the net_http adapter' do
+      expect(connection.adapter).to eq Faraday::Adapter::NetHttp
+    end
   end
 
   it 'sets all three phase timeouts on the Net::HTTP connection' do