From 6fa30c3e623721dd64113330b1325e8880af08f1 Mon Sep 17 00:00:00 2001 From: brianlball Date: Tue, 18 Aug 2026 12:46:53 -0400 Subject: [PATCH] Harden Resque post-fork reconnect against saturated Redis (maxclients) Stock resque 2.6 Worker#reconnect only retries Redis::BaseConnectionError. A saturated Redis accepts the socket and replies "-ERR max number of clients reached" (Redis::CommandError), so the forked child dies on its first Redis use with no retry - and reporting the failure needs Redis too, so the job vanishes unrecorded and InitializeAnalysis strands its analysis in 'queued' forever (2026-08-18 k8s outage: large spot worker fleet pushed connected_clients past maxclients). - config/initializers/resque_reconnect_retry.rb: prepend override that also retries Redis::CommandError (covers maxclients and AOF LOADING), 5 tries with backoff (~30s total), then re-raises into the stock failure path; no-op in delayed_job deployments (guarded on defined?(Resque::Worker)); retry count tunable via RESQUE_RECONNECT_RETRIES - spec/lib/resque_reconnect_retry_spec.rb: unit specs, tagged depends_resque (resque only loads in resque envs); wired into the docker CI job via docker/server/run-server-tests.sh - verified in a local replica of the CI docker env (nrel/openstudio-server:develop + mongo as db + redis as queue): 6 examples, 0 failures Co-Authored-By: Claude Fable 5 --- docker/server/run-server-tests.sh | 4 + .../initializers/resque_reconnect_retry.rb | 47 +++++++++++ .../spec/lib/resque_reconnect_retry_spec.rb | 78 +++++++++++++++++++ 3 files changed, 129 insertions(+) create mode 100644 server/config/initializers/resque_reconnect_retry.rb create mode 100644 server/spec/lib/resque_reconnect_retry_spec.rb diff --git a/docker/server/run-server-tests.sh b/docker/server/run-server-tests.sh index b6bcdc502..810ea3372 100755 --- a/docker/server/run-server-tests.sh +++ b/docker/server/run-server-tests.sh @@ -21,6 +21,10 @@ done #cd /opt/openstudio/server && bundle exec rspec; (( exit_status = exit_status || $? )) # Socket-level specs for the persistent worker->web HTTP client. Fast, no stack needed. cd /opt/openstudio/server && bundle exec rspec spec/lib/os_http_spec.rb; (( exit_status = exit_status || $? )) +# Unit specs for the Resque::Worker#reconnect hardening (retry Redis::CommandError, +# e.g. "max number of clients reached"). Resque only loads under RAILS_ENV=docker, +# so they run here. Fast, no stack needed. +cd /opt/openstudio/server && bundle exec rspec spec/lib/resque_reconnect_retry_spec.rb; (( exit_status = exit_status || $? )) # Model/request specs for seed.zip upload validation + InitializeAnalysis failure handling (issue #841). # These need only rails+mongo, so run them first - they are fast and leave the db empty. cd /opt/openstudio/server && bundle exec rspec spec/models/analysis_init_spec.rb spec/requests/analyses_upload_spec.rb; (( exit_status = exit_status || $? )) diff --git a/server/config/initializers/resque_reconnect_retry.rb b/server/config/initializers/resque_reconnect_retry.rb new file mode 100644 index 000000000..2f8e0d15e --- /dev/null +++ b/server/config/initializers/resque_reconnect_retry.rb @@ -0,0 +1,47 @@ +# ******************************************************************************* +# OpenStudio(R), Copyright (c) Alliance for Sustainable Energy, LLC. +# See also https://openstudio.net/license +# ******************************************************************************* + +# Resque forks a child per job, and the child must open a new Redis connection +# before performing (Resque::Worker#reconnect). Stock resque (2.6) only retries +# Redis::BaseConnectionError there. But a saturated Redis accepts the socket and +# replies "-ERR max number of clients reached" (similarly "-LOADING ..." while +# replaying the AOF after a restart), which redis-rb raises as +# Redis::CommandError - not a BaseConnectionError - so the child dies on its +# first job instantly, with no retry. Worse, reporting that failure needs Redis +# too, so the job vanishes without a trace and, for InitializeAnalysis, the +# analysis is stranded in 'queued' forever (2026-08-18 k8s outage: large spot +# worker fleets pushed connected_clients past maxclients). +# +# This override also retries Redis::CommandError with a longer backoff. During +# reconnect the only commands on the wire are connection setup (AUTH/SELECT), so +# a CommandError here is effectively connection-level and safe to retry. If the +# retries are exhausted the error is re-raised and handled exactly as before. +# +# This file must load after config/initializers/redis.rb (alphabetical order +# guarantees it), and is a no-op in delayed_job deployments where resque is +# never required. +if defined?(Resque::Worker) + module ResqueReconnectRetry + MAX_TRIES = Integer(ENV.fetch('RESQUE_RECONNECT_RETRIES', 5)) + + def reconnect + tries = 0 + begin + data_store.reconnect + rescue Redis::BaseConnectionError, Redis::CommandError => e + if (tries += 1) <= MAX_TRIES + log_with_severity :error, "Error reconnecting to Redis (#{e.class}: #{e.message}); retry #{tries}/#{MAX_TRIES}" + sleep(tries * 2) + retry + else + log_with_severity :error, "Error reconnecting to Redis (#{e.class}: #{e.message}); giving up after #{MAX_TRIES} retries" + raise + end + end + end + end + + Resque::Worker.prepend(ResqueReconnectRetry) +end diff --git a/server/spec/lib/resque_reconnect_retry_spec.rb b/server/spec/lib/resque_reconnect_retry_spec.rb new file mode 100644 index 000000000..243c20e28 --- /dev/null +++ b/server/spec/lib/resque_reconnect_retry_spec.rb @@ -0,0 +1,78 @@ +# ******************************************************************************* +# OpenStudio(R), Copyright (c) Alliance for Sustainable Energy, LLC. +# See also https://openstudio.net/license +# ******************************************************************************* + +require 'rails_helper' + +# Regression specs for the 2026-08-18 redis-maxclients outage: resque's post-fork +# reconnect only retried Redis::BaseConnectionError, so a saturated server's +# "-ERR max number of clients reached" reply (Redis::CommandError) killed the +# child on its first Redis use, the failure could not be reported (reporting +# needs Redis too), and the job vanished - stranding analyses in 'queued'. +# config/initializers/resque_reconnect_retry.rb makes reconnect retry +# CommandError as well, with backoff. Resque only loads in resque deployments, +# so these run in the docker CI job (see docker/server/run-server-tests.sh). +RSpec.describe 'ResqueReconnectRetry', depends_resque: true do + let(:worker) { Resque::Worker.new(:spec_queue) } + let(:data_store) { double('data_store') } + let(:max_tries) { ResqueReconnectRetry::MAX_TRIES } + + before do + allow(worker).to receive(:data_store).and_return(data_store) + allow(worker).to receive(:log_with_severity) + allow(worker).to receive(:sleep) # no real waiting in specs + end + + it 'overrides Resque::Worker#reconnect' do + expect(worker.method(:reconnect).owner).to eq ResqueReconnectRetry + end + + it 'reconnects once and does not sleep when the connection succeeds' do + expect(data_store).to receive(:reconnect).once + worker.reconnect + expect(worker).not_to have_received(:sleep) + end + + it 'retries Redis::CommandError (e.g. maxclients saturation) with backoff until it succeeds' do + calls = 0 + allow(data_store).to receive(:reconnect) do + calls += 1 + raise Redis::CommandError, 'ERR max number of clients reached' if calls < 3 + end + + expect { worker.reconnect }.not_to raise_error + expect(calls).to eq 3 + expect(worker).to have_received(:sleep).with(2).ordered + expect(worker).to have_received(:sleep).with(4).ordered + end + + it 'still retries Redis::BaseConnectionError (stock resque behavior preserved)' do + calls = 0 + allow(data_store).to receive(:reconnect) do + calls += 1 + raise Redis::CannotConnectError, 'Error connecting to Redis' if calls < 2 + end + + expect { worker.reconnect }.not_to raise_error + expect(calls).to eq 2 + end + + it 're-raises after exhausting the retries' do + allow(data_store).to receive(:reconnect) + .and_raise(Redis::CommandError, 'ERR max number of clients reached') + + expect { worker.reconnect }.to raise_error(Redis::CommandError, /max number of clients/) + # initial attempt + MAX_TRIES retries + expect(data_store).to have_received(:reconnect).exactly(max_tries + 1).times + expect(worker).to have_received(:sleep).exactly(max_tries).times + end + + it 'does not swallow or retry unrelated errors' do + allow(data_store).to receive(:reconnect).and_raise(RuntimeError, 'boom') + + expect { worker.reconnect }.to raise_error(RuntimeError, 'boom') + expect(data_store).to have_received(:reconnect).once + expect(worker).not_to have_received(:sleep) + end +end