From 96341b4c7ba5bce503b9a481ddebf39c8eb92205 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sat, 8 Aug 2026 16:01:04 +1200 Subject: [PATCH] =?UTF-8?q?ble:=20stop=20the=20auto-continue=20empty-offlo?= =?UTF-8?q?ad=20storm=20=E2=80=94=20guard=202a=20on=20real=20rows=20(#1144?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A real WHOOP 4.0 capture showed the offload auto-continue (#364/#451) firing 7 bursts of 24 empty offloads (~168 in 27 min), each with a 2M-PHY round-trip — a battery/radio storm. Root cause: guard 2a (strapNewest - frontier > 300s -> continue) returns true regardless of whether the offload pulled any rows. When the strap advertises a newest ahead of our frontier but the offload hands back chunkRows=0 (a phantom gap), the frontier can't advance, the gap stays > 300s, and 2a re-fires to the full 24 cap. Guard 3 (lastTrimAdvanced) doesn't catch it — the trim u32 climbs on empty ENDs. Fix (both platforms, shouldAutoContinue kept byte-for-behaviour): an EMPTY session (rowsPersistedThisSession <= 0) never auto-continues, whatever the reported gap. Collapses each 24-spin into a single empty offload + fall back to the 15-min floor. Healthy deep backlogs persist real rows (245-251/chunk), so they're unaffected. Tests: added the phantom-gap regression (gap holds, rows=0 -> stop) both platforms; continue-expecting cases now pass rows>0 (a frontier can't advance with 0 rows). Verified: Kotlin testFullDebugUnitTest green; Swift shouldAutoContinue via isolated harness. --- Strand/BLE/BLEManager.swift | 9 +++++++ StrandTests/BackfillContinuationTests.swift | 22 ++++++++++++++++ .../main/java/com/noop/ble/WhoopBleClient.kt | 9 +++++++ .../com/noop/ble/BackfillContinuationTest.kt | 26 +++++++++++++++++++ 4 files changed, 66 insertions(+) diff --git a/Strand/BLE/BLEManager.swift b/Strand/BLE/BLEManager.swift index 7b1af0b351..8105d47d0d 100644 --- a/Strand/BLE/BLEManager.swift +++ b/Strand/BLE/BLEManager.swift @@ -363,6 +363,15 @@ struct BackfillContinuation { guard stillConnected else { return false } // 1 guard consecutiveCount < maxAutoContinues else { return false } // 4 (cap) guard lastTrimAdvanced else { return false } // 3 (don't spin on a frozen cursor) + // 3b (#1144): an EMPTY session (0 rows persisted) never auto-continues, whatever the reported + // frontier gap. When the strap advertises a `newest` AHEAD of our frontier but the offload for that + // range hands back 0 rows (a PHANTOM gap — a timestamp it won't actually offload), 2a below would + // latch `true` forever: the frontier can't advance without rows, so `newest - frontier` stays > gap + // and it re-fires to the full cap in empty offloads (the storm observed on a real 4.0). Guard 3 + // doesn't catch it — the trim u32 climbs on empty ENDs. Stop and let the periodic floor retry; + // healthy backlog sessions persist real rows so this only bites the empty spin. (Makes 2b's row + // check the ONE authority for both cases.) + guard rowsPersistedThisSession > 0 else { return false } // #928: a strap clock set in the FUTURE makes "newest" read ahead of ANY real frontier, so 2a // would report backlog forever and drive up to the full cap in EMPTY offloads on every connect. // A newest more than futureSkewSeconds past the wall clock is implausible: exclude it from 2a. diff --git a/StrandTests/BackfillContinuationTests.swift b/StrandTests/BackfillContinuationTests.swift index d7fb17866e..b92dc6f949 100644 --- a/StrandTests/BackfillContinuationTests.swift +++ b/StrandTests/BackfillContinuationTests.swift @@ -27,10 +27,26 @@ final class BackfillContinuationTests: XCTestCase { strapNewestTs: 1_800_000_000, // strap newest ourFrontierTs: 1_800_000_000 - 86_400, // our frontier a full day behind wallNowUnix: wallNow, + rowsPersistedThisSession: 200, lastTrimAdvanced: true, consecutiveCount: 0)) } + /// #1144 phantom-gap spin: the strap reports a `newest` well AHEAD of our frontier (so guard 2a's gap + /// holds) and the trim advanced — but the offload persisted ZERO rows. Continuing would re-fire 2a to + /// the full cap in empty offloads, since the frontier can't advance without rows. An empty session must + /// stop regardless of the reported gap. + func testStopsWhenGapHoldsButSessionWasEmpty() { + XCTAssertFalse(BackfillContinuation.shouldAutoContinue( + stillConnected: true, + strapNewestTs: 1_800_000_000, + ourFrontierTs: 1_800_000_000 - 400, // 400s behind → 2a's 300s gap holds + wallNowUnix: wallNow, + rowsPersistedThisSession: 0, // …but nothing was actually offloaded + lastTrimAdvanced: true, // trim u32 climbs on empty ENDs — not enough + consecutiveCount: 0)) + } + /// A dropped link must NOT auto-continue — the normal reconnect path owns it. func testStopsWhenDisconnected() { XCTAssertFalse(BackfillContinuation.shouldAutoContinue( @@ -70,6 +86,7 @@ final class BackfillContinuationTests: XCTestCase { strapNewestTs: 1_800_000_000, ourFrontierTs: 1_800_000_000 - 301, wallNowUnix: wallNow, + rowsPersistedThisSession: 200, lastTrimAdvanced: true, consecutiveCount: 0, behindGapSeconds: 300)) @@ -97,6 +114,7 @@ final class BackfillContinuationTests: XCTestCase { strapNewestTs: 1_800_000_000, ourFrontierTs: 1_800_000_000 - 86_400, wallNowUnix: wallNow, + rowsPersistedThisSession: 200, lastTrimAdvanced: true, consecutiveCount: cap - 1)) // At the cap, stop. @@ -223,6 +241,7 @@ final class BackfillContinuationTests: XCTestCase { strapNewestTs: strapNewest, ourFrontierTs: frontier, wallNowUnix: wallNow, + rowsPersistedThisSession: 200, lastTrimAdvanced: true, consecutiveCount: count) { // Each pass drains ~a day of the oldest backlog and counts as one auto-continue. @@ -246,6 +265,7 @@ final class BackfillContinuationTests: XCTestCase { strapNewestTs: 1_800_000_000, ourFrontierTs: 1_800_000_000 - 86_400, // a full day behind wallNowUnix: wallNow, + rowsPersistedThisSession: 200, lastTrimAdvanced: true, consecutiveCount: 10)) // well past the old cap of 6 } @@ -380,6 +400,7 @@ final class BackfillContinuationTests: XCTestCase { strapNewestTs: wallNow + 48 * 3600, ourFrontierTs: wallNow - 86_400, wallNowUnix: wallNow, + rowsPersistedThisSession: 200, lastTrimAdvanced: true, consecutiveCount: 0)) // One second past the cap: excluded, and an empty session must not continue. @@ -401,6 +422,7 @@ final class BackfillContinuationTests: XCTestCase { strapNewestTs: wallNow + 3600, // an hour ahead: plausible skew, not a broken clock ourFrontierTs: wallNow - 86_400, // a real day of backlog wallNowUnix: wallNow, + rowsPersistedThisSession: 200, lastTrimAdvanced: true, consecutiveCount: 0)) } diff --git a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt index c1b4039892..cda1c9f05e 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -1452,6 +1452,15 @@ class WhoopBleClient( if (!stillConnected) return false // 1 if (consecutiveCount >= maxAutoContinues) return false // 4 (cap) if (!lastTrimAdvanced) return false // 3 (don't spin on a frozen cursor) + // 3b (#1144): an EMPTY session (0 rows persisted) never auto-continues, whatever the reported + // frontier gap. When the strap advertises a `newest` AHEAD of our frontier but the offload for + // that range hands back chunkRows=0 (a PHANTOM gap — a timestamp it won't actually offload), 2a + // below would latch `true` forever: the frontier can't advance without rows, so `newest - + // frontier` stays > gap and it re-fires to the full cap in empty offloads (the storm observed on + // a real 4.0). `lastTrimAdvanced` (3) doesn't catch it — the trim u32 climbs on empty ENDs. Stop + // and let the 15-min floor retry; healthy backlog sessions persist real rows so this only bites + // the empty spin. (Makes 2b's row check the ONE authority for both cases.) + if (rowsPersistedThisSession <= 0) return false // #928: a strap clock set in the FUTURE makes "newest" read ahead of ANY real frontier, so 2a // would report backlog forever and drive up to the full cap in EMPTY offloads on every // connect. A newest more than [futureSkewSeconds] past [wallNowUnix] (the REAL wall clock, diff --git a/android/app/src/test/java/com/noop/ble/BackfillContinuationTest.kt b/android/app/src/test/java/com/noop/ble/BackfillContinuationTest.kt index 6cd2574274..9f1a3fba82 100644 --- a/android/app/src/test/java/com/noop/ble/BackfillContinuationTest.kt +++ b/android/app/src/test/java/com/noop/ble/BackfillContinuationTest.kt @@ -37,6 +37,26 @@ class BackfillContinuationTest { wallNowUnix = wallNow, lastTrimAdvanced = true, consecutiveCount = 0, + rowsPersistedThisSession = 200, + ), + ) + } + + /** #1144 phantom-gap spin: the strap reports a `newest` well AHEAD of our frontier (so guard 2a's gap + * holds) and the trim advanced — but the offload persisted ZERO rows. Continuing would re-fire 2a to + * the full cap in empty offloads, because the frontier can't advance without rows. An empty session + * must stop regardless of the reported gap. */ + @Test + fun stops_whenGapHoldsButSessionWasEmpty() { + assertFalse( + WhoopBleClient.shouldAutoContinue( + stillConnected = true, + strapNewestTs = 1_800_000_000L, + ourFrontierTs = 1_800_000_000L - 400L, // 400s behind → 2a's 300s gap holds + wallNowUnix = wallNow, + lastTrimAdvanced = true, // trim u32 climbs on empty ENDs — not enough + consecutiveCount = 0, + rowsPersistedThisSession = 0, // …but nothing was actually offloaded ), ) } @@ -94,6 +114,7 @@ class BackfillContinuationTest { lastTrimAdvanced = true, consecutiveCount = 0, behindGapSeconds = 300L, + rowsPersistedThisSession = 200, ), ) } @@ -125,6 +146,7 @@ class BackfillContinuationTest { wallNowUnix = wallNow, lastTrimAdvanced = true, consecutiveCount = cap - 1, + rowsPersistedThisSession = 200, ), ) assertFalse( @@ -153,6 +175,7 @@ class BackfillContinuationTest { wallNowUnix = wallNow, lastTrimAdvanced = true, consecutiveCount = 10, // well past the old cap of 6 + rowsPersistedThisSession = 200, ), ) } @@ -288,6 +311,7 @@ class BackfillContinuationTest { wallNowUnix = wallNow, lastTrimAdvanced = true, consecutiveCount = count, + rowsPersistedThisSession = 200, // each pass lands real rows (that's why the frontier advances) ) ) { frontier += 86_400L @@ -455,6 +479,7 @@ class BackfillContinuationTest { wallNowUnix = wallNow, lastTrimAdvanced = true, consecutiveCount = 0, + rowsPersistedThisSession = 200, ), ) // One second past the cap: excluded, and an empty session must not continue. @@ -483,6 +508,7 @@ class BackfillContinuationTest { wallNowUnix = wallNow, lastTrimAdvanced = true, consecutiveCount = 0, + rowsPersistedThisSession = 200, ), ) }