Reuse MySQL connection across flushes; never swallow flush errors

[y-ken]

Summary

out_mysql_bulk's write/1 opened a fresh MySQL connection on every flush and closed it only on the success path. Two problems:

1. A failing INSERT (e.g. a NOT NULL violation on bad input) skipped the close and leaked the connection on every Fluentd retry.
2. Even on success, reconnecting per flush is wasteful.

This PR makes the connection persist and be reused across successful flushes, while keeping errors propagating correctly.

Behavior

• Success: keep the connection open and reuse it on the next flush.
• Reconnect only when there is no usable handler (first flush, or it was discarded by the error path) or when the target database changed — the INSERT uses an unqualified table name, so it must run on a connection bound to the right database (placeholder-driven database can vary per chunk).
• Error: drop the (possibly broken) connection so the next retry reconnects — this also recovers from a server-side "MySQL server has gone away" — then re-raise.
• Shutdown: release the held connection via #close.

Why not swallow the error (re: #65)

#65 proposes rescue Exception + log.warn to stop the endless retries the author saw. That removes the symptom but introduces a worse one:

• Fluentd's buffered output decides success/failure by whether write/1 raises. Swallowing makes Fluentd treat the flush as successful and purge the chunk → every buffered record is silently dropped. Because this is a bulk insert, one bad row fails the whole statement, so the entire batch (potentially thousands of good rows) is lost.
• rescue Exception (vs StandardError) also swallows Interrupt/SignalException, and the proposed log line drops the exception class/message.

Operators who want to cap retries on permanently-bad data should use Fluentd's own retry_max_times / <secondary>, which this change leaves working.

Known limitation (pre-existing)

As before, write/1 uses a single @handler instance variable and is not safe for flush_thread_count > 1. This PR does not change that; thread-local/pooled connections would be a separate, larger change.

Tests

• unit (test_out_mysql_bulk.rb, no live MySQL): a stubbed handler proves write/1 reuses one connection across successful flushes and closes it on shutdown, and that a failing INSERT re-raises while closing and dropping the connection.
• integration (test_out_mysql_bulk_integration.rb): a NOT NULL violation (the exact error from Add try,catch statement for query #65) must raise out of write/1 and insert nothing; a second successful flush reuses the same connection object.

Supersedes #65.

[y-ken]

Design rationale (for reviewers)

Two coupled changes here. Writing the reasoning down so the trade-offs are explicit.

1. Errors must propagate out of write (not be rescued)

Fluentd's buffered-output contract: write(chunk) returning normally means "this chunk was durably written" → Fluentd purges the chunk. Raising means "failed" → Fluentd retries, and after retry_max_times routes to <secondary> or drops the chunk with proper error logging/metrics.

So rescue-and-log (as in #65) is not "tolerating an error" — it tells Fluentd the data is safely stored when it isn't, and the whole chunk is discarded. Because this is a bulk insert, one bad row fails the statement, so an entire batch is dropped silently.

This is not a tunable trade-off: there is no configuration in which silently turning a failed write into a "success" is correct for an at-least-once pipeline. The reporter's actual problem (a permanently-failing row causing endless retries) is exactly what retry_max_times / <secondary> are for, and both keep working here.

→ No trade-off — this restores correct semantics.

2. Reuse the connection across successful flushes

Previously a connection was opened and closed on every flush. Now it is kept open and reused, reconnecting only when:

• there is no live handler (first flush, or the previous flush hit the error path), or
• the target database changed — the INSERT uses an unqualified table name (INSERT INTO users ...), so it depends on the connection's current database. database can be placeholder-driven and vary per chunk, so reusing across databases would write to the wrong one. The per-database guard is therefore a correctness requirement, not an optimization.

On error the connection is dropped and re-created on the next retry; on shutdown it is released via #close.

→ One genuine behavioral trade-off (below).

Trade-offs to confirm before merge

1. Stale connection after a long idle gap. A reused connection can be closed by the server (wait_timeout, default 8h; or a network blip). When that happens the next query raises, we drop the connection, re-raise, and Fluentd retries — the retry reconnects and succeeds. So no data loss, but the first flush after an idle period longer than wait_timeout costs one failed-then-retried cycle, whereas the old "reconnect every time" code never hit this. In practice flush intervals are far below wait_timeout, so this is rare. If we prefer to avoid it entirely, reconnect: true or a pre-flush ping would do it at the cost of an extra round-trip/complexity — left out as not worth it, but easy to add if preferred.

2. flush_thread_count > 1 is still unsupported. write uses a single @handler ivar and is not safe for concurrent flush threads. This was already true before this PR (the old code also shared @handler); reuse only widens the window. Real support means thread-local or pooled connections — a separate, larger change. We just need to confirm we are OK keeping the existing single-flush-thread assumption.

Everything else (error propagation, per-database reconnect, close-on-shutdown, the connection-leak fix) has no competing downside that I can see.

What to confirm to reach agreement / merge

• CI green across the matrix (Ruby 3.2–4.0 + MySQL 8.4). The suite was not run locally (mysql2 native build unavailable in my env), so the integration tests have only executed via CI — please sanity-check that run.
• Agreement that flush errors should propagate (i.e. Add try,catch statement for query #65's swallow approach is rejected).
• Acceptance of connection reuse, including the idle / "server has gone away" nuance in trade-off 'tag' and 'time'  #1.
• Acknowledgement that flush_thread_count > 1 remains unsupported (trade-off Fix typo s/flush_intervals/flush_interval/ #2, pre-existing).
• Decide whether to close Add try,catch statement for query #65 as superseded by this PR.