feat: opt-in HTTP keep-alive via keep_alive_connections#55
Open
erimicel wants to merge 2 commits intotypesense:masterfrom
Open
feat: opt-in HTTP keep-alive via keep_alive_connections#55erimicel wants to merge 2 commits intotypesense:masterfrom
erimicel wants to merge 2 commits intotypesense:masterfrom
Conversation
Currently `Typesense::ApiCall#perform_request` builds a fresh `Faraday.new(...)` (and therefore a new TCP and TLS handshake) on every request. On hot endpoints this can dominate the Typesense round-trip latency. This adds an opt-in `keep_alive_connections` configuration option (default `false`, so existing users see no behaviour change). When enabled: * Faraday connections are cached per `(thread, node)` rather than constructed per request. Net::HTTP is not thread-safe, so per-thread caching keeps concurrent callers isolated while still respecting the existing node round-robin. * Connections use the `:net_http_persistent` Faraday adapter with a 30s idle timeout, so reused sockets are dropped before most load balancers cull them. * On any rescued network error, the cached connection is dropped before the gem retries, so a half-closed keep-alive socket cannot fail the retry as well. Pair with `num_retries >= 1` for transparent recovery from server- or load-balancer-side idle timeouts. The `:net_http_persistent` adapter and its `net-http-persistent` runtime dependency are listed in the gemspec, and `require 'faraday/net_http_persistent'` is gated on the option being enabled, so loading the gem with the option off does not import the new dependency at runtime. New RSpec coverage: * connection reuse on the same thread * per-node cache keying * per-thread cache isolation * per-instance cache isolation * eviction on network error * timeouts propagate to the cached connection * the option defaults to false and the legacy per-request connection path is preserved
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Typesense::ApiCall#perform_request(lib/typesense/api_call.rb:72) constructs a freshFaraday.new(...)on every call, which in turn builds a newNet::HTTPinstance per request. For HTTPS clusters this means the TCP connection and TLS handshake are repeated on every search/index call, even when the same client instance is being reused as a process-wide singleton.We profiled this on a hot search endpoint and the
OpenSSL::X509::Store#set_default_pathsplus the TLS handshake consistently dominated the request, despite the client itself being a singleton. The fix at the application layer is impossible while the gem rebuilds the Faraday connection on every call.What
Adds an opt-in
keep_alive_connectionsconfig option (defaultfalse— no behaviour change for existing users).When enabled:
(thread, node)(Net::HTTPis not thread-safe), keyed byprotocol://host:port. Existing node round-robin andnearest_nodelogic are unchanged.:net_http_persistentadapter;keep_alive_idle_timeout_seconds(default 30s) controls how long an idle socket is reused before being dropped — set this at or below your load balancer's idle timeout.num_retries >= 1for transparent recovery.ApiCall#object_id, so multipleTypesense::Clientinstances don't share sockets.Default-off safety
require 'faraday/net_http_persistent'is gated on the flag, so the new dependency isn't loaded when keep-alive is off. The legacy per-requestFaraday.newcodepath is preserved verbatim. Rescue/retry list is unchanged.Tests
spec/typesense/api_call_spec.rb(+10 examples, all green): connection reuse, per-node keying, per-thread isolation, per-instance isolation, eviction on network error, timeout propagation, custom + defaultkeep_alive_idle_timeout_seconds, default-off path.PR Checklist