[Server] Record only the dispatched command family; drop unused totals

processCommandContainer recorded every family in a container even though
the base if/else-if dispatch processes at most one. An unauthenticated
client could batch a session command (login) with fabricated developer,
moderator, and admin entries and forge genuine-looking samples that were
never executed or authorized. Mirror the base's selection, skip when the
handler was already deleted, and skip entries whose extension number is
-1 (which would otherwise wrap into the previous kind's id range).

[Server] Drop dead process-lifetime byte/uptime counters

txBytesTotal/rxBytesTotal added an atomic RMW to every socket write and
read for counters nothing consumes (cmdGetServerStats fills tx_bytes,
rx_bytes, and uptime_secs from the DB snapshot). Remove the two atomics
and the getTxBytesTotal/getRxBytesTotal/getUptimeSeconds getters; the
incTxBytes/incRxBytes slots and mutexes remain for the ISL legacy
counters.

[Protocol] Document kind 5 as developer in CommandStats

NumKinds is 6 and the server emits kind_index = 5 for developer
commands; the comment stopped at 4.
This commit is contained in:
Lukas Brübach 2026-09-02 14:18:34 +02:00
parent a8151b9e97
commit b855b279a7
4 changed files with 51 additions and 36 deletions

View file

@ -2,7 +2,7 @@ syntax = "proto2";
import "response.proto"; import "response.proto";
message CommandStats { message CommandStats {
optional uint32 kind_index = 1; // 0=session, 1=room, 2=game, 3=moderator, 4=admin optional uint32 kind_index = 1; // 0=session, 1=room, 2=game, 3=moderator, 4=admin, 5=developer
optional uint32 extension_number = 2; // protobuf extension number within the kind optional uint32 extension_number = 2; // protobuf extension number within the kind
optional string command_name = 3; // e.g. "session/Command_Ping" optional string command_name = 3; // e.g. "session/Command_Ping"
optional uint64 count = 4; // number of times observed optional uint64 count = 4; // number of times observed

View file

@ -89,7 +89,6 @@ void Servatrice_GameServer::incomingConnection(qintptr socketDescriptor)
Servatrice_ConnectionPool *pool = findLeastUsedConnectionPool(); Servatrice_ConnectionPool *pool = findLeastUsedConnectionPool();
auto ssi = new TcpServerSocketInterface(server, pool->getDatabaseInterface()); auto ssi = new TcpServerSocketInterface(server, pool->getDatabaseInterface());
connect(ssi, SIGNAL(incTxBytes(qint64)), this, SLOT(incTxBytes(qint64)));
ssi->moveToThread(pool->thread()); ssi->moveToThread(pool->thread());
pool->addClient(); pool->addClient();
connect(ssi, SIGNAL(destroyed()), pool, SLOT(removeClient())); connect(ssi, SIGNAL(destroyed()), pool, SLOT(removeClient()));
@ -160,7 +159,6 @@ void Servatrice_WebsocketGameServer::onNewConnection()
Servatrice_ConnectionPool *pool = findLeastUsedConnectionPool(); Servatrice_ConnectionPool *pool = findLeastUsedConnectionPool();
auto ssi = new WebsocketServerSocketInterface(server, pool->getDatabaseInterface()); auto ssi = new WebsocketServerSocketInterface(server, pool->getDatabaseInterface());
connect(ssi, SIGNAL(incTxBytes(quint64)), this, SLOT(incTxBytes(quint64)));
/* /*
* Due to a Qt limitation, websockets can't be moved to another thread. * Due to a Qt limitation, websockets can't be moved to another thread.
* This will hopefully change in Qt6 if QtWebSocket will be integrated in QtNetwork * This will hopefully change in Qt6 if QtWebSocket will be integrated in QtNetwork
@ -785,7 +783,6 @@ void Servatrice::incTxBytes(quint64 num)
txBytesMutex.lock(); txBytesMutex.lock();
txBytes += num; txBytes += num;
txBytesMutex.unlock(); txBytesMutex.unlock();
txBytesTotal.fetch_add(num, std::memory_order_relaxed);
} }
void Servatrice::incRxBytes(quint64 num) void Servatrice::incRxBytes(quint64 num)
@ -793,7 +790,6 @@ void Servatrice::incRxBytes(quint64 num)
rxBytesMutex.lock(); rxBytesMutex.lock();
rxBytes += num; rxBytes += num;
rxBytesMutex.unlock(); rxBytesMutex.unlock();
rxBytesTotal.fetch_add(num, std::memory_order_relaxed);
} }
void Servatrice::shutdownTimeout() void Servatrice::shutdownTimeout()

View file

@ -173,8 +173,6 @@ private:
int uptime; int uptime;
QMutex txBytesMutex, rxBytesMutex; QMutex txBytesMutex, rxBytesMutex;
quint64 txBytes, rxBytes; quint64 txBytes, rxBytes;
std::atomic<quint64> txBytesTotal{0}; ///< cumulative bytes sent since process start
std::atomic<quint64> rxBytesTotal{0}; ///< cumulative bytes received since process start
MetricsRegistry metricsRegistry; MetricsRegistry metricsRegistry;
int metricsSlowCommandMs = 500; int metricsSlowCommandMs = 500;
int metricsStallWarnMs = 2000; int metricsStallWarnMs = 2000;
@ -302,18 +300,6 @@ public:
{ {
return metricsRegistry; return metricsRegistry;
} }
quint64 getTxBytesTotal() const
{
return txBytesTotal.load(std::memory_order_relaxed);
}
quint64 getRxBytesTotal() const
{
return rxBytesTotal.load(std::memory_order_relaxed);
}
int getUptimeSeconds() const
{
return uptime;
}
/** /**
* Sums cards across all zones of all running games. Each game takes its * Sums cards across all zones of all running games. Each game takes its
* own gameMutex -- the hot per-game lock every game action contends on -- * own gameMutex -- the hot per-game lock every game action contends on --

View file

@ -202,25 +202,58 @@ void AbstractServerSocketInterface::processCommandContainer(const CommandContain
Server_ProtocolHandler::processCommandContainer(cont); Server_ProtocolHandler::processCommandContainer(cont);
const qint64 elapsedMs = timer.nsecsElapsed() / 1000000; const qint64 elapsedMs = timer.nsecsElapsed() / 1000000;
// A container usually holds a single command. When several are batched, // The base dispatch is an if/else-if chain — at most one family is
// each is attributed the container's total processing time. // actually processed. Recording every family in the container would
for (const auto &cmd : cont.session_command()) { // let an unauthenticated client stampforge developer/moderator/admin
servatrice->getMetricsRegistry().observeCommand(MetricsRegistry::typeIdFor(0, getPbExtension(cmd)), elapsedMs); // samples by batching them alongside a session command the server
// actually runs. Mirror the base's selection and skip entirely when
// deleted or when no family matched.
if (deleted) {
return;
} }
for (const auto &cmd : cont.room_command()) {
servatrice->getMetricsRegistry().observeCommand(MetricsRegistry::typeIdFor(1, getPbExtension(cmd)), elapsedMs); // When getPbExtension returns -1 (no extension set) and the kind is
// non-zero, typeIdFor wraps into the previous kind's range instead of
// hitting the typeId < 0 guard in observeCommand. Skip such entries.
int kind = -1;
if (cont.game_command_size()) {
kind = 2;
} else if (cont.room_command_size()) {
kind = 1;
} else if (cont.session_command_size()) {
kind = 0;
} else if (cont.moderator_command_size()) {
kind = 3;
} else if (cont.admin_command_size()) {
kind = 4;
} else if (cont.developer_command_size()) {
kind = 5;
} }
for (const auto &cmd : cont.game_command()) {
servatrice->getMetricsRegistry().observeCommand(MetricsRegistry::typeIdFor(2, getPbExtension(cmd)), elapsedMs); if (kind >= 0) {
} auto recordDispatched = [&](int familyKind, const auto &cmds) {
for (const auto &cmd : cont.moderator_command()) { for (const auto &cmd : cmds) {
servatrice->getMetricsRegistry().observeCommand(MetricsRegistry::typeIdFor(3, getPbExtension(cmd)), elapsedMs); const int ext = getPbExtension(cmd);
} if (ext >= 0) {
for (const auto &cmd : cont.admin_command()) { servatrice->getMetricsRegistry().observeCommand(MetricsRegistry::typeIdFor(familyKind, ext),
servatrice->getMetricsRegistry().observeCommand(MetricsRegistry::typeIdFor(4, getPbExtension(cmd)), elapsedMs); elapsedMs);
} }
for (const auto &cmd : cont.developer_command()) { }
servatrice->getMetricsRegistry().observeCommand(MetricsRegistry::typeIdFor(5, getPbExtension(cmd)), elapsedMs); };
if (kind == 0) {
recordDispatched(kind, cont.session_command());
} else if (kind == 1) {
recordDispatched(kind, cont.room_command());
} else if (kind == 2) {
recordDispatched(kind, cont.game_command());
} else if (kind == 3) {
recordDispatched(kind, cont.moderator_command());
} else if (kind == 4) {
recordDispatched(kind, cont.admin_command());
} else {
recordDispatched(kind, cont.developer_command());
}
} }
const int slowCommandMs = servatrice->getMetricsSlowCommandMs(); const int slowCommandMs = servatrice->getMetricsSlowCommandMs();