fix(mqtt): count failed starts and ignore bounced sessions in cf

connect()/reconnect() returning an error started no attempt, so no event
arrived and the failure went uncounted -- the low-heap case the counter exists
for. Counted now in a bridge-task-only start_failures, summed into cf: and
mqtt_connect_failures.

A renewal or corrected-clock bounce whose softDisconnect() times out gets its
old session's DISCONNECTED after the new attempt raised the flag. That session
is still marked connected until the handler runs, so the gate now also
requires !connected. An ignored late CONNECTED clears the flag too.

cf: no longer depends on dc being non-zero. Comments corrected for the
PSRAM-fallback stack cost and heap_largest's sampling point; the worst-case
test now models 6 slots.
This commit is contained in:
agessaman committed 2026-09-26 12:25:53 -07:00
1 parent 2f6d139729
commit 99a4b6fad0
4 files changed
+25 -14

No files matched your search

+2
View File
@@ -17,6 +17,8 @@
// heap alone does not — an 85 KB-free board sat unable to connect for hours.
// A low value with all slots up is normal, not a fault: established sessions
// already hold their buffers. It means only that the next slot to ask will fail.
// Sampled at status-publish time, not at the attempt, so it is a trend, not a
// per-attempt reading.
struct MQTTConnHealth {
int slots_up = -1; // -1 = not supplied, field omitted
int slots_total = -1; // enabled slots, whether or not they connect
+13 -9
View File
@@ -584,17 +584,18 @@ void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index)
replyAppendf(buf, bufsize, &pos, "> mqtt%d: %s", slot_index + 1, state);
if (slot.disconnect_count > 0) {
replyAppendf(buf, bufsize, &pos, ", dc:%lu", (unsigned long)slot.disconnect_count);
// cf: attempts that never reached onConnect. A slot retrying into a heap that
// cannot satisfy the TLS allocation moves only this counter — dc rises with it,
// but the publish counters cannot move at all while the slot is down.
if (slot.connect_failures > 0) {
replyAppendf(buf, bufsize, &pos, ", cf:%lu", (unsigned long)slot.connect_failures);
}
if (slot.first_disconnect_time > 0) {
unsigned long first_disc_age_sec = (millis() - slot.first_disconnect_time) / 1000;
replyAppendf(buf, bufsize, &pos, ", first_disc:%lus", first_disc_age_sec);
}
}
// cf: attempts that never reached onConnect, plus starts that failed outright. A
// slot retrying into a heap that cannot satisfy the TLS allocation moves only this;
// the publish counters cannot move at all while the slot is down.
const uint32_t cf = slot.connect_failures + slot.start_failures;
if (cf > 0) {
replyAppendf(buf, bufsize, &pos, ", cf:%lu", (unsigned long)cf);
}
// Connected with no errors: nothing more to say about the connection.
if (slot.connected && slot.last_error_time == 0) {
@@ -1728,6 +1729,7 @@ bool MQTTBridge::ensureSlotClient(int index) {
MQTT_DEBUG_PRINTLN("MQTT%d ignoring late CONNECTED (state=%s, gen=%lu, enabled=%d)",
index + 1, clientStateName(st),
(unsigned long)_slots[index].generation, (int)_slots[index].enabled);
_slot_attempt_pending[index] = false; // still the end of that attempt
return;
}
MQTT_DEBUG_PRINTLN("MQTT%d connected", index + 1);
@@ -1766,8 +1768,9 @@ bool MQTTBridge::ensureSlotClient(int index) {
slot.client->onDisconnect([this, index](bool sessionPresent) {
MQTT_DEBUG_PRINTLN("MQTT%d disconnected", index + 1);
// Ended an attempt that never reached onConnect. A deliberate stop clears the flag
// first, so cancelling an attempt is not counted as a failure.
if (_slot_attempt_pending[index]) {
// first; a bounced live session is still marked connected until this handler runs,
// so its late DISCONNECTED after a timed-out softDisconnect() is not counted either.
if (_slot_attempt_pending[index] && !_slots[index].connected) {
_slot_attempt_pending[index] = false;
_slots[index].connect_failures++;
}
@@ -2524,6 +2527,7 @@ esp_err_t MQTTBridge::reconnectSlotClient(int index) {
slot.applied_token_expires_at = slot.token_expires_at;
} else {
_slot_attempt_pending[index] = false;
slot.start_failures++;
}
return r;
}
@@ -3199,7 +3203,7 @@ MQTTConnHealth MQTTBridge::collectConnHealth() const {
const MQTTSlot& slot = _slots[i];
if (!slot.enabled) continue; // "none" slots are not an outage, they are unconfigured
health.slots_total++;
connect_failures += slot.connect_failures;
connect_failures += slot.connect_failures + slot.start_failures;
if (slot.circuit_breaker_tripped) breaker_slots++;
if (slot.connected) {
health.slots_up++;
+8 -3
View File
@@ -187,6 +187,10 @@ private:
// disconnect_count (which also counts dropped live sessions) and from the
// publish error counter, which cannot move at all while a slot is down.
uint32_t connect_failures;
// connect()/reconnect() calls that returned an error, so no attempt started and
// no event will arrive. Written only by the bridge task; reported summed with
// connect_failures.
uint32_t start_failures;
unsigned long first_disconnect_time; // millis() of first disconnect after boot
// Current-outage timer (used by AlertReporter to fire faults after a sustained
@@ -288,7 +292,7 @@ private:
// Set by the bridge task before connect()/reconnect(), so a failure the event task
// delivers before that call returns is still seen; cleared by the attempt's outcome
// or a deliberate stop. A disconnect that finds it set is a connect failure.
// (including an ignored late CONNECTED) or a deliberate stop.
volatile bool _slot_attempt_pending[RUNTIME_MQTT_SLOTS];
// CLI-requested forced NTP sync, marshalled onto the MQTT task (Core 0).
@@ -381,8 +385,9 @@ private:
// _device_id[65], _board_model[64], _firmware_version[64] and a 63-byte client version):
// measured 762/768 before the connection-health fields existed. Overflow is silent —
// serializeComplete() returns 0 and the bridge publishes only len>0, so status stops
// entirely. Raised to 1024, which costs no extra RAM here because status serializes into
// the shared scratch buffer. See the worst-case test in test_mqtt_payload_builder.
// entirely. Raised to 1024: no extra RAM while status serializes into the shared scratch
// buffer, but +256 B of bridge-task stack in the fallback used if the PSRAM allocation
// failed. See the worst-case test in test_mqtt_payload_builder.
static const size_t STATUS_JSON_BUFFER_SIZE = 1024;
static_assert(STATUS_JSON_BUFFER_SIZE <= PUBLISH_JSON_BUFFER_SIZE,
"status payloads serialize into the shared publish buffer");
@@ -222,11 +222,11 @@ TEST(MQTTPayloadBuilder, WorstCaseStatusWithConnectionHealthFitsProductionBuffer
char buffer[kStatusJsonBufferSize];
MQTTConnHealth health;
health.slots_up = 0;
health.slots_total = 5; // PSRAM slot cap, the widest configuration
health.slots_total = 6; // RUNTIME_MQTT_SLOTS on PSRAM, the widest
health.worst_outage_secs = 4294967; // millis() rollover / 1000, the widest value
health.heap_largest = 8388608;
health.connect_failures = 2147483647;
health.slots_breaker = 5;
health.slots_breaker = 6;
int len = MQTTPayloadBuilder::buildStatusMessage(
scratch, origin.c_str(), origin_id.c_str(), model.c_str(), firmware.c_str(),