From 3bc3255733546fc7ae70dfb7670ed409eec1b8ed Mon Sep 17 00:00:00 2001 From: kiku98 Date: Mon, 13 Jul 2026 17:43:22 +0200 Subject: [PATCH] fix(message-db): reconnect stale pooled connections dropped by the server Pooled Mysql2::Client connections were handed out without verifying they were still alive. A connection parked in the pool during a quiet period could be closed server-side once it idled past `wait_timeout`, and the next query on it raised "disconnected by the server because of inactivity". This error was not matched by the pool's reconnect guard, so it propagated up, aborted message processing and left queued messages stuck. - Ping each pooled connection on checkout; discard and replace it if the server has already closed it. - Broaden the dead-connection pattern to also match the inactivity / server-disconnect wording as defence-in-depth for connections that die mid-block. --- lib/postal/message_db/connection_pool.rb | 35 +++++++++++++++++-- .../postal/message_db/connection_pool_spec.rb | 31 ++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/lib/postal/message_db/connection_pool.rb b/lib/postal/message_db/connection_pool.rb index cf145f287..79f73bf95 100644 --- a/lib/postal/message_db/connection_pool.rb +++ b/lib/postal/message_db/connection_pool.rb @@ -11,6 +11,13 @@ def initialize @lock = Mutex.new end + # Matches the errors mysql2 raises when a pooled connection has been + # dropped by the server (e.g. idle longer than `wait_timeout`). The + # inactivity variant is important: a connection parked in the pool during + # a quiet period is silently closed server-side, and the next query on it + # raises "disconnected by the server because of inactivity". + DEAD_CONNECTION_PATTERN = /(lost connection|gone away|not connected|disconnected by the server|inactivity)/i + def use retried = false do_not_checkin = false @@ -19,7 +26,7 @@ def use yield connection rescue Mysql2::Error => e - if e.message =~ /(lost connection|gone away|not connected)/i + if e.message =~ DEAD_CONNECTION_PATTERN # If the connection has failed for a connectivity reason # we won't add it back in to the pool so that it'll reconnect # next time. @@ -41,14 +48,36 @@ def use private def checkout - @lock.synchronize do - return @connections.pop unless @connections.empty? + connection = @lock.synchronize do + @connections.pop unless @connections.empty? + end + + if connection + # A pooled connection may have been closed by the server while it sat + # idle (e.g. past `wait_timeout`). Verify it is still alive before + # handing it out. `ping` returns false for a dead connection rather + # than raising, so we discard it and fall through to a fresh one. + return connection if connection_alive?(connection) + + close_connection(connection) end add_new_connection checkout end + def connection_alive?(connection) + connection.ping + rescue Mysql2::Error + false + end + + def close_connection(connection) + connection.close + rescue Mysql2::Error + nil + end + def checkin(connection) @lock.synchronize do @connections << connection diff --git a/spec/lib/postal/message_db/connection_pool_spec.rb b/spec/lib/postal/message_db/connection_pool_spec.rb index 53d2aadba..3befdbc55 100644 --- a/spec/lib/postal/message_db/connection_pool_spec.rb +++ b/spec/lib/postal/message_db/connection_pool_spec.rb @@ -52,5 +52,36 @@ end.to raise_error Mysql2::Error expect(clients_seen.uniq.size).to eq 2 end + + it "does not check in connections when the server closes the connection for inactivity" do + expect do + pool.use do + raise Mysql2::Error, "The client was disconnected by the server because of inactivity." + end + end.to raise_error Mysql2::Error + expect(pool.connections).to eq [] + end + + it "reuses a pooled connection that is still alive" do + first = nil + pool.use { |c| first = c } + second = nil + pool.use { |c| second = c } + expect(second).to be first + end + + it "discards a pooled connection that has been closed by the server" do + dead = nil + pool.use { |c| dead = c } + + # Simulate the server having dropped the idle connection: ping fails. + allow(dead).to receive(:ping).and_return(false) + + fresh = nil + pool.use { |c| fresh = c } + + expect(fresh).not_to be dead + expect(pool.connections).to eq [fresh] + end end end