Skip to content

Commit 1c9c629

Browse files
committed
fix(mqtt): shed retransmissions, not capture, when duty cycle starves the pool
Load-testing the restored token bucket at 'set dutycycle 1' showed MQTT capture dropping to exactly the TX rate. Queued retransmissions hold static- pool packets with no expiry, so throttling parks the whole pool in the send queue; Dispatcher::checkRecv() then discards received packets before logRx() ever feeds the bridge — each completed TX frees exactly one packet for exactly one more RX. Observer builds now use RxReservePacketManager (fork-owned header): once the free pool drops below a quarter of the pool, outbound packets are refused and freed, so RX allocation and MQTT capture continue at full rate while the node sheds repeat load it has no TX budget for anyway. Non-observer builds keep upstream pool behavior via the same factory; StaticPoolPacketManager stays byte-identical to upstream.
1 parent 4b7a312 commit 1c9c629

5 files changed

Lines changed: 73 additions & 3 deletions

File tree

MQTT_INTERNALS.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,23 @@ The observer feature is kept out of upstream-tracked files through three mechani
3535

3636
Remaining integration points in upstream files:
3737
- `examples/simple_repeater/MyMesh.{h,cpp}`, `examples/simple_room_server/MyMesh.{h,cpp}` -
38-
bridge/alerter/SNMP wiring and packet-feed hooks, guarded by `#ifdef WITH_MQTT_BRIDGE`
38+
bridge/alerter/SNMP wiring and packet-feed hooks, guarded by `#ifdef WITH_MQTT_BRIDGE`;
39+
plus the `createObserverPacketManager()` call in each constructor (see below)
3940
- `src/helpers/CommonCLI.{h,cpp}` - the three CLI hooks, `MQTTPrefs` load/save/migration
4041
- `src/Dispatcher.{h,cpp}` - radio watchdog block, guarded by `#ifdef WITH_MQTT_BRIDGE`
4142

43+
### Capture vs. duty-cycle throttling
44+
45+
RX processing needs a free packet from the static pool before `logRx()` (and thus the
46+
MQTT uplink) can run — `Dispatcher::checkRecv()` silently discards received data when
47+
the pool is empty. Because the outbound queue holds pool packets with no expiry,
48+
duty-cycle throttling can park the entire pool waiting on TX budget, capping capture at
49+
the TX rate. Observer builds therefore use `RxReservePacketManager` (fork-owned,
50+
`src/helpers/RxReservePacketManager.h`), which refuses to queue retransmissions once
51+
the free pool drops below a reserve (a quarter of the pool) — the node sheds repeat
52+
load it has no TX budget for, and capture continues at full rate. Non-observer builds
53+
keep the upstream pool behavior.
54+
4255
### `/mqtt_prefs` file format
4356

4457
`/mqtt_prefs` is written with an 8-byte `MQTTPrefsHeader` (`magic`, `version`,

RESTORE_UPSTREAM_NOTES.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,18 @@ This touches core TX timing. Confirm on hardware:
4242
verify `next_tx_time` spacing under load).
4343
- The observer radio watchdog still recovers a stuck radio (`radio_watchdog_minutes`).
4444

45+
### Known interaction: throttling starves MQTT capture (mitigated)
46+
47+
Found during the on-device duty-cycle load test (`set dutycycle 1`): queued
48+
retransmissions hold static-pool packets with no expiry, so heavy throttling parks the
49+
whole pool in the send queue. `Dispatcher::checkRecv()` then drops received packets
50+
before `logRx()` runs, capping MQTT capture at the TX rate (each TX frees one packet
51+
for one RX). This is inherent upstream behavior — the old fork's `next_tx` spacing had
52+
the same steady-state drain — but it defeats the observer's purpose. Mitigated on
53+
observer builds by `RxReservePacketManager` (`src/helpers/RxReservePacketManager.h`):
54+
retransmissions are shed once the free pool drops below a quarter of the pool, keeping
55+
RX capture at full rate. See MQTT_INTERNALS.md "Capture vs. duty-cycle throttling".
56+
4557
## Phase 2 — CAD and FEM RX gain (NOT done; needs care + device testing)
4658

4759
Still missing at HEAD, also dropped by `22eb9b87`, still present upstream:

examples/simple_repeater/MyMesh.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "MyMesh.h"
22
#include <algorithm>
33
#include <stdlib.h> // for qsort()
4+
#include <helpers/RxReservePacketManager.h>
45

56
/* ------------------------------ Config -------------------------------- */
67

@@ -901,7 +902,7 @@ void MyMesh::sendNodeDiscoverReq() {
901902

902903
MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondClock &ms, mesh::RNG &rng,
903904
mesh::RTCClock &rtc, mesh::MeshTables &tables)
904-
: mesh::Mesh(radio, ms, rng, rtc, *new StaticPoolPacketManager(32), tables),
905+
: mesh::Mesh(radio, ms, rng, rtc, *createObserverPacketManager(32), tables),
905906
region_map(key_store), temp_map(key_store),
906907
_cli(board, rtc, sensors, region_map, acl, &_prefs, this),
907908
telemetry(MAX_PACKET_PAYLOAD - 4),

examples/simple_room_server/MyMesh.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "MyMesh.h"
2+
#include <helpers/RxReservePacketManager.h>
23

34
#define REPLY_DELAY_MILLIS 1500
45
#define PUSH_NOTIFY_DELAY_MILLIS 2000
@@ -634,7 +635,7 @@ void MyMesh::onAckRecv(mesh::Packet *packet, uint32_t ack_crc) {
634635

635636
MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondClock &ms, mesh::RNG &rng,
636637
mesh::RTCClock &rtc, mesh::MeshTables &tables)
637-
: mesh::Mesh(radio, ms, rng, rtc, *new StaticPoolPacketManager(32), tables),
638+
: mesh::Mesh(radio, ms, rng, rtc, *createObserverPacketManager(32), tables),
638639
region_map(key_store), temp_map(key_store),
639640
_cli(board, rtc, sensors, region_map, acl, &_prefs, this),
640641
telemetry(MAX_PACKET_PAYLOAD - 4)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
#pragma once
2+
3+
#include <MeshCore.h>
4+
#include <helpers/StaticPoolPacketManager.h>
5+
6+
// Fork-owned (not upstream-tracked). Observer builds capture every received packet
7+
// to MQTT, but RX processing needs a free pool packet first — Dispatcher::checkRecv()
8+
// discards the received bytes before logRx() when allocNew() fails. Under duty-cycle
9+
// throttling the outbound queue can park the entire pool waiting on TX budget, which
10+
// starves RX allocation and silently caps MQTT capture at the TX rate (each completed
11+
// TX frees exactly one packet for exactly one more RX).
12+
//
13+
// This manager sheds *retransmissions* instead: once the free pool drops below the
14+
// reserve, outbound packets are refused (freed straight back to the pool) so RX
15+
// allocation — and therefore capture — continues at full rate. The node was already
16+
// dropping traffic in that state; this chooses to drop repeats it has no TX budget
17+
// for anyway, rather than capture.
18+
class RxReservePacketManager : public StaticPoolPacketManager {
19+
int _rx_reserve;
20+
public:
21+
RxReservePacketManager(int pool_size, int rx_reserve)
22+
: StaticPoolPacketManager(pool_size), _rx_reserve(rx_reserve) {}
23+
24+
void queueOutbound(mesh::Packet* packet, uint8_t priority, uint32_t scheduled_for) override {
25+
if (getFreeCount() < _rx_reserve) {
26+
MESH_DEBUG_PRINTLN("RxReservePacketManager: pool below RX reserve, shedding outbound");
27+
free(packet);
28+
return;
29+
}
30+
StaticPoolPacketManager::queueOutbound(packet, priority, scheduled_for);
31+
}
32+
};
33+
34+
// The packet manager for an app build: observer builds reserve a quarter of the pool
35+
// for RX so MQTT capture survives duty-cycle throttling; non-observer builds keep the
36+
// upstream pool behavior unchanged.
37+
inline mesh::PacketManager* createObserverPacketManager(int pool_size) {
38+
#ifdef WITH_MQTT_BRIDGE
39+
return new RxReservePacketManager(pool_size, pool_size / 4);
40+
#else
41+
return new StaticPoolPacketManager(pool_size);
42+
#endif
43+
}

0 commit comments

Comments
 (0)