grant: replace the rate check with a token bucket
server-release / image (push) Successful in 15s
server-test / test (push) Successful in 32s
server-release / release (push) Successful in 33s

The live throughput test found it: a 3-second run delivered 104 packets and
stopped after 50 milliseconds.

The rate check exempted the first 50 ms entirely, meaning to be lenient at
startup. The effect was the opposite. A sender could dump an unbounded burst
into that free window, and the instant the check switched on it compared those
bytes against 50 ms worth of allowance and refused everything until real time
caught up. Every short test passed — downtrain sends 50 packets, big_send seven
— and every sustained send died about fifty milliseconds in.

A token bucket (allowance = burst + rate x elapsed) has no such cliff; it is
smooth from t=0. The burst is 100 ms of the allowed rate, floored at one
ordinary datagram so a single packet is never refused outright. The floor is
deliberately one datagram: at 8 kbps a 64 KB floor would be sixty-four seconds'
worth, which is precisely the instant dump the ceiling exists to prevent. The
existing rate test caught that when I first tried it, and it was right.

Second half of the same bug: callers treated any refusal as terminal. TryAllow
now says why, so a sender can pace through a transient "too fast just now" and
still stop dead on a spent budget or an expired grant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
mrambossek
2026-08-01 14:10:30 +02:00
co-authored by Claude Fable 5
parent 3333788d9e
commit 3c9af04e6f
4 changed files with 193 additions and 14 deletions
@@ -0,0 +1,68 @@
// SPDX-FileCopyrightText: 2026 Echolot contributors
// SPDX-License-Identifier: GPL-3.0-or-later
package app.echo_lot.engine
import app.echo_lot.measurement.TestStatus
import app.echo_lot.protocol.ControlClient
import app.echo_lot.protocol.ProbeSession
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
/**
* Downstream throughput against a LIVE server. Self-skips without ECHOLOT_LIVE_*.
*
* The assertions are about *honesty* rather than speed: a rate is only a measurement if the run
* was ended by the clock and the sender's own count backs it up. A test that just asserted "some
* Mbps arrived" would pass equally well against a broken implementation.
*/
class LiveThroughputTest {
private val url = System.getenv("ECHOLOT_LIVE_URL")
private val pin = System.getenv("ECHOLOT_LIVE_PIN")
private val cred = System.getenv("ECHOLOT_LIVE_CRED")
private val udp = System.getenv("ECHOLOT_LIVE_UDP")
private val target = System.getenv("ECHOLOT_LIVE_TARGET") ?: "fmr"
@Test
fun measuresDownstreamRateAndSaysWhatLimitedIt() {
if (url == null || pin == null || cred == null || udp == null) {
println("LiveThroughputTest skipped (no ECHOLOT_LIVE_* env)"); return
}
val control = ControlClient(url, setOf(pin), "0.2.0")
val session = control.createSession(cred, target)
val (host, port) = udp.split(":").let { it[0] to it[1].toInt() }
val (test, findings) = ProbeSession(cred, session, host, port).use { ps ->
ps.echo() // prime: the grant binds to the observed source
ThroughputMeasurement(SystemIdSource()).run(
cred, session.sessionId, control, ps, sessionRef = "sess-1",
durationS = 3, kbps = 20_000,
)
}
control.deleteSession(cred, session.sessionId)
val m = assertNotNull(test.metrics).toString()
println("throughput: ${test.status} $m")
for (f in findings) println("finding ${f.code} [${f.severity}] ${f.title}")
assertEquals(TestStatus.OK, test.status, "no throughput traffic arrived: $m")
// The sender's own count must be present — without it, loss cannot be attributed and the
// number is not a measurement.
assertTrue(m.contains("sender_packets"), "no sender report to compare against: $m")
assertTrue(m.contains("limited_by"), "the result must say what ended the run: $m")
val received = Regex(""""received_kbps":(\d+)""").find(m)?.groupValues?.get(1)?.toInt()
assertNotNull(received)
assertTrue(received > 0, "measured 0 kbps: $m")
println("received ${received / 1000} Mbit/s")
// A run this short and this far below the ceiling should end on the clock. Anything else
// means the grant was the constraint, and then the rate says nothing about the path.
assertTrue(m.contains(""""limited_by":"duration""""),
"the run did not end on the clock, so the rate measures the server, not the path: $m")
}
}