[Rushaway]

Summary

• Comms queue never reached MySQL (sbpp_comms.sp): Query_ProcessQueue is the callback of the SQLite queue SELECT, so its db was the SQLite handle and the INSERT INTO %s_comms was built and sent on SQLite (no such table there). Every mute/gag queued during a MySQL outage stayed in queue2 forever and never reached the panel. The insert now uses g_hDatabase.Format/Query (after a DB_Connect() guard), the same fix sbpp_main already has for its ban queue. A small in-flight counter stops a second queue pass from sending the same rows twice. I kept one INSERT per row instead of one Transaction, because each row is deleted from the queue only when its own insert succeeds, and one bad row must not block the others.
• DB handle deleted every map (sbpp_comms.sp): OnMapEnd deleted g_hDatabase ("necessary for SQL_SetCharset", which is not true). On every map this caused a reconnect, a blocking SetCharset, and a VerifyBlock for every player. Players who joined before the reconnect finished were marked as checked without a query, so muted players could talk for a moment. The handle is now kept across maps, and SetCharset runs once per new connection in GotDatabase (the driver keeps the charset across its automatic reconnects). The queue flush that used to happen as a side effect of the per-map reconnect now runs from OnMapStart when the connection is up.
• Any query error dropped the MySQL connection (sbpp_comms.sp): DB_Conn_Lost deleted g_hDatabase on every failed query, including the SQLite queue queries (CREATE TABLE queue2, queue insert/delete) that went through Query_ErrorCheck. It now takes the error text and drops the handle only for real connection errors ("Lost connection", "gone away", "Can't connect", "Broken pipe"). SQLite queue queries use their own Query_QueueErrorCheck, which only logs. The reconnect timers no longer use TIMER_FLAG_NO_MAPCHANGE, so a map change can't leave the state stuck on Wait. A failed connect now waits RetryTime before trying again, instead of reconnecting on every block check. If VerifyBlock fails with a plain SQL error, the player is rechecked after RetryTime instead of every second.
• Full table scans on every connect (all plugins): authid REGEXP '^STEAM_[0-9]:%s$' and LIKE 'STEAM_%:%s' can't use an index. They are now authid IN ('STEAM_0:%s', 'STEAM_1:%s'), with the existing authid = '%s' OR folded into the same IN list. Changed queries: the ban check (sbpp_main CheckClientBan), VerifyBlock / unblock select / queue and punishment admin sub-selects (sbpp_comms), the connect counts and sm_listbans / sm_listcomms (sbpp_checker), the alt check (sbpp_sleuth), and all admin sub-selects in sbpp_main ban/unban/queue inserts. The sb_bans lookups now use type_authid, and the sb_comms lookups use authid.
• Banlog re-ran the ban lookup (sbpp_main.sp VerifyBan): the INSERT INTO %s_banlog sub-selected the bid again with the REGEXP/IP condition (and without the expiry check). It now uses the iBid that VerifyBan already fetched.
• SourceSleuth IP lookup without the index prefix (sbpp_sleuth.sp): WHERE ip = '%s' with no leading type could not use type_ip. Both ban types store an IP, so type IN (0, 1) keeps the same rows and lets MySQL use the index.
• No connect retry / null handle use:
  • sbpp_main: a failed connect was only retried at the next admin cache rebuild. It now retries on a RetryTime timer, guarded by g_bConnecting. I left perf: set the database charset once per connection in sbpp_main #82's GotDatabase SetCharset/InsertServerInfo lines alone, and git merges this file cleanly with perf: set the database charset once per connection in sbpp_main #82.
  • sbpp_main: sm_banip, sm_unban, sm_addban and SBPP_ReportPlayer called DB.Format on a null handle. The commands now reply that the database is not available. The report native logs the failure and tells the reporter.
  • sbpp_checker: SetFailState on the first failed connect is replaced by log + 30 s retry. On connect it now checks players who are not checked yet, which also fixes the late-load check, since that used to run before the DB was connected.
  • sbpp_sleuth: a failed connect was never retried. It now retries after 30 s.
• Legacy API / raw client indexes: CloseHandle → delete (sbpp_main menus and reason pack, sbpp_comms expire timers), WritePackString / ReadPackCell → DataPack methods. sbpp_main's ClientRecheck timer and SQL_OnIPMend now get a userid instead of a client index.
• Version: CHANGELOG entry 2.2.7-srcdslab (next after perf: set the database charset once per connection in sbpp_main #82's 2.2.6). The plugin version comes from the release tag.

Not included

• SQLite_UseDatabase (sbpp_main / sbpp_comms queue) is still synchronous, but it runs only once at plugin start on a local file. Database.Connect with a missing databases.cfg entry falls back to the "default" config (MySQL), not to SQLite, so an async switch would send the queue to MySQL on installs without a queue entry. I found no clean fallback, so I left it as is.
• KEY(authid) on sb_admins (web schema struc.sql): this PR only changes the SourceMod side. The admin sub-selects are index-ready now. Manual SQL for existing installs:

  ALTER TABLE sb_admins ADD INDEX authid (authid);

• The IN ('STEAM_0:…','STEAM_1:…') form no longer matches the never-used universes STEAM_2…STEAM_9. The old REGEXP/LIKE matched them.
• Merge order with perf: set the database charset once per connection in sbpp_main #82: sbpp_main.sp merges cleanly. Both PRs add a new entry at the top of CHANGELOG.md, so whichever merges second will need that one-line conflict resolved.

Test plan

• Compiles with spcomp 1.12.0.7259 (same deps as CI): all 6 plugins, no errors, no warnings (same as main)
• CI build passes
• Change map several times: sbpp_comms logs no reconnect, and muted/gagged players stay muted with no gap after the map change
• Stop MySQL, sm_mute a player (goes to queue2), start MySQL, change map: the mute shows up in sb_comms and the row is gone from sourcecomms-queue.sq3
• Run a broken SQL statement through comms (e.g. temporarily rename sb_srvgroups): the error is logged, but there is no "Lost connection to DB" and no reconnect
• Start the server with MySQL down: sbpp_main / sbpp_checker / SourceSleuth log the failure, stay loaded, and connect by themselves once MySQL is back; sm_banip / sm_unban / sm_addban reply "database is not available" while it is down
• Banned SteamID (STEAM_0 and STEAM_1 forms) and banned IP are still kicked on connect, and a sb_banlog row is written with the right bid; EXPLAIN on the connect query shows type_authid / type_ip
• sm_listbans / sm_listcomms and the connect-time admin notice still show the right counts; SourceSleuth still detects an alt on a shared IP

🤖 Generated with Claude Code

[WareZ57]

Went through the whole diff plus the surrounding code in all four plugins. Most of it checks out. There's one small issue in the new sbpp_main retry and a nit in sbpp_checker.

What I checked and agree with

• Comms queue: confirmed on main that Query_ProcessQueue builds and sends the INSERT INTO %s_comms on the SQLite handle it got as db, so queued blocks could never reach MySQL. Moving to g_hDatabase.Format/Query fixes that, and the queued strings now get MySQL escaping instead of SQLite's. The in-flight counter is balanced on every path (SELECT error, DB_Connect() failing, truncated rows, insert callbacks). The queue DELETE and the next queue SELECT go through the same SQLite handle in order, so a row can't be read again before its delete runs.
• TIMER_FLAG_NO_MAPCHANGE removal on Timer_StopWait / Timer_ReconnectDB: on main a map change during the wait killed the timer and left the state on Wait, so DB_Connect() returned false until the plugin was reloaded. This is a good fix to have.
• SteamID lookups: IN ('STEAM_0:x', 'STEAM_1:x') returns the same rows as the old REGEXP '^STEAM_[0-9]:x$' / LIKE for universes 0 and 1. The columns use the panel charset's case-insensitive collation, so case behaves the same too. The admin sub-selects still keep the exact authid = '%s' match inside the IN list. For SourceSleuth, BanType only has 0 and 1 in the web panel, so type IN (0, 1) doesn't drop any rows.
• VerifyBan banlog: using the iBid from the row that was just matched is better than the old sub-select. That one had no expiry check and could log an expired ban's bid.
• ClientRecheck / SQL_OnIPMend now get userids. The null-handle guards on sm_banip / sm_unban / sm_addban / SBPP_ReportPlayer cover every DB. use that could run before the connection exists.
• Built locally with spcomp 1.12.0.7259 (same -i include as CI): all 6 plugins compile with no errors or warnings, on this branch and merged with #82. git merge-tree against main is clean, and with #82 only CHANGELOG.md conflicts.

Retry timers can stack up in sbpp_main

Every failed connect now schedules its own Timer_RetryConnect (GotDatabase, line 1167), and OnRebuildAdminCache() still starts a connect of its own whenever DB is null and !g_bConnecting (line 432). While MySQL is down, each map change can start a second chain on top of the running one:

1. Chain A's timer is pending, so g_bConnecting is false.
2. The map changes, OnRebuildAdminCache connects, the connect fails, and GotDatabase schedules timer B.
3. A and B now both keep retrying every RetryTime.

A chain only stops when its timer fires while another connect is still in flight. With a fast failure ("Connection refused" while MySQL restarts or is stopped) that almost never happens, so a long outage ends up with roughly one chain per map change. Each chain retries the connect, logs to sourcebans.log and re-parses the overrides backup every 15 to 60 s. It's harmless once the DB comes back, but it's easy to avoid with a single timer handle:

Handle g_hRetryConnect = null;

// GotDatabase(), failure branch
if (g_hRetryConnect == null)
    g_hRetryConnect = CreateTimer(RetryTime, Timer_RetryConnect);

public Action Timer_RetryConnect(Handle timer)
{
    g_hRetryConnect = null;
    if (DB == INVALID_HANDLE && !g_bConnecting)
    {
        g_bConnecting = true;
        Database.Connect(GotDatabase, "sourcebans");
    }
    return Plugin_Stop;
}

Nit: sbpp_checker reconnect check skips players who are still loading

OnDatabaseConnected now calls LateLoading(), but LateLoading() skips anyone who isn't IsClientInGame() (line 745). Take a player who got OnClientAuthorized while the DB was down (it returns early on g_DB == null) and is still loading when the connection comes back. That player is never checked, and the counts stay at -1 until the next map. OnClientAuthorized() doesn't need the player in game, so IsClientConnected() && IsClientAuthorized() && !IsFakeClient() should be enough there.

Out of scope, possible follow-up

Native_SBReportPlayer still puts the raw iReporter / iTarget client indexes in the DataPack, and SQL_OnReportPlayer passes them to SBPP_OnReportPlayer after the async insert. If the reporter leaves and the slot is reused, the forward gets the wrong client. In sbpp_report that's only the "Report Sent" chat line, but third-party listeners could log the wrong player. Userids would fix it, same as you did for ClientRecheck.

[Rushaway]

Thanks for the review @WareZ57. Both points are fixed in aedf11e.

Retry timers stacking in sbpp_main
GotDatabase() now keeps the retry timer in g_hRetryConnect and only schedules one when none is pending. Timer_RetryConnect() clears the handle before it reconnects. A connect started by OnRebuildAdminCache() while a retry is already pending no longer starts a second chain, so a long outage keeps a single retry loop no matter how many map changes happen. The timer still has no TIMER_FLAG_NO_MAPCHANGE, so it survives map changes.

sbpp_checker skipping players who are still loading
LateLoading() no longer requires IsClientInGame(). It now checks every connected, authorized, non-fake client whose counts are still -1. A player authorized while the database was down is checked as soon as the connection comes back, even if they are still loading.

Report native (out of scope)
I left Native_SBReportPlayer as is in this PR. Switching to userids means SBPP_OnReportPlayer would get 0 for a player who left, which changes the forward's documented contract (client index). A third-party listener that calls IsClientInGame(iReporter) without a > 0 check would then throw instead of getting false. I'd rather do that in its own PR, with the include doc updated.

Checks

• All 6 plugins compile with no errors or warnings (spcomp 1.13), on this branch and with sbpp_main.sp merged with perf: set the database charset once per connection in sbpp_main #82.
• git merge-tree against main is clean. With perf: set the database charset once per connection in sbpp_main #82, only CHANGELOG.md conflicts, as before.
• CHANGELOG 2.2.7-srcdslab item 04 now mentions that sbpp_checker checks players who joined while it was disconnected.