Fixes #39359 - Reuse persistent Foreman connections and retry stale POST /register#4
Open
pablomh wants to merge 2 commits into
Open
Fixes #39359 - Reuse persistent Foreman connections and retry stale POST /register#4pablomh wants to merge 2 commits into
pablomh wants to merge 2 commits into
Conversation
… singletons Replace the per-instance Net::HTTP with two class-level persistent connections shared across all ForemanRequest consumers: - foreman_http: shared connection for POST /register, templates, and other Foreman-bound traffic, serialized via FOREMAN_MUTEX - registration_get_http: dedicated connection for GET /register cache-miss fetches, serialized via REGISTRATION_GET_MUTEX The two connections never block each other, so registration cache fills do not delay POST traffic. Both use the same http_init (same SSL config, same timeouts) and are lazily started on first use. On transport error the broken Net::HTTP is replaced with a fresh instance so the next caller gets a clean connection. Also adds three configurable timeout settings: - foreman_request_timeout (default 200s, above sub-man's 180s) - foreman_connect_timeout (default: Ruby's default) - foreman_keep_alive_timeout (default 10s, below RHEL's 15s) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Route GET /register cache-miss fetches through send_request_direct (the dedicated registration_get_http connection) so they do not compete with POST traffic for the shared foreman_http connection. Add a single retry for POST /register on transport errors (EOFError, ECONNRESET, etc). The retry is scoped to host_register only, not the generic transport layer. Foreman's POST /register controller uses Host.find_or_initialize_by, making a single replay safe after a stale keep-alive connection failure. The request body is captured before the first attempt so the retry can rebuild a fresh Net::HTTP::Post from the same data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
Net::HTTPwith two class-level persistent connections shared across allForemanRequestconsumers:foreman_http— shared connection for POST /register, templates, and other Foreman-bound trafficregistration_get_http— dedicated connection for GET /register cache-miss fetchesforeman_request_timeout(default 200s),foreman_connect_timeout,foreman_keep_alive_timeout(default 10s)Net::HTTPwith a fresh instance so the next caller gets a clean connectionhost_registeronly — Foreman's controller usesHost.find_or_initialize_by, making a single replay safeBackground
Tested under incremental registration load (76→1140 concurrent hosts, 6 capsules, Satellite
--tuning large). Key findings:net-http-persistent(PR Fixes #XXXXX - Reuse persistent Foreman connections theforeman/smart-proxy#950) was incompatible with WEBrick's thread-per-request model —connection_pool's thread-local tracking caused CLOSE-WAIT accumulation andConnectionPool::TimeoutErrorunder loadKeepAliveTimeoutwhen workers are busy, causing stale connection EOFErrors on POST /registerfind_hostusesHost.find_or_initialize_by(name: ...)— a find-or-create that produces the same result on replayCommits
Test plan
bundle exec ruby -Itest test/request_test.rb— 18 tests, 0 failures🤖 Generated with Claude Code