mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 22:03:13 -06:00
Symptom 1's 'stuck on first load, fixed by toggling the playlist' is stale download backoff. On a fresh device the first downloads fail while the link is settling; DownloadCoordinator escalates an exponential backoff (15s..5min cap), and ensure() then SKIPS those items. The 60s playlist refresh re-fires onPlaylistUpdate but ensure() still skips them, and backoff was only ever cleared by forget() (content-delete) — never by a re-assignment. So the item stays stuck until the 5-min window happens to lapse; toggling the playlist is just a manual way to wait it out. Fix (storm-safe — neither reset fires on the routine same-playlist 60s refresh): 1. DownloadCoordinator.resetBackoff(id) / resetAllBackoff() — clear attempts + nextAttemptAt but KEEP inFlight (single-flight preserved, no duplicate .part). 2. onPlaylistUpdate resets backoff for each item ONLY when the content-id signature changed (first load, reassignment, toggle-back), then ensures — so a genuine (re)assignment retries immediately. Same-signature 60s refresh -> no reset -> the retry-storm guard stays intact. 3. Network onAvailable (was onLost-only) -> resetAllBackoff() + requestPlaylistRefresh, so content that failed while the link was settling retries the moment real connectivity arrives. Tests: DownloadCoordinatorTest — resetBackoff/resetAllBackoff re-attempt a backed-off item before the clock advances; existing backoff/single-flight tests unchanged. :app:testDebugUnitTest green. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
bc9e72ec0b
commit
fa31eb9cc3
|
|
@ -48,6 +48,9 @@ class MainActivity : AppCompatActivity() {
|
|||
private lateinit var config: ServerConfig
|
||||
private lateinit var contentCache: ContentCache
|
||||
private lateinit var downloadCoordinator: com.remotedisplay.player.data.DownloadCoordinator
|
||||
// #170: content-id signature of the last processed playlist, to detect a genuine content change
|
||||
// (first load / reassignment / toggle-back) vs the routine 60s same-playlist refresh.
|
||||
private var lastDownloadSig: String? = null
|
||||
private lateinit var screenshotCapture: ScreenshotCapture
|
||||
private lateinit var touchInjector: TouchInjector
|
||||
|
||||
|
|
@ -465,6 +468,13 @@ class MainActivity : AppCompatActivity() {
|
|||
}
|
||||
|
||||
private fun setupServiceCallbacks() {
|
||||
// #170: on a fresh network connection, clear stuck download backoff so content that failed
|
||||
// to download while the link was settling retries on the next sweep (the service also
|
||||
// requests a playlist refresh). Keeps single-flight; only touches failure/backoff state.
|
||||
wsService?.onNetworkAvailable = {
|
||||
if (::downloadCoordinator.isInitialized) downloadCoordinator.resetAllBackoff()
|
||||
}
|
||||
|
||||
wsService?.onPlaylistUpdate = { data ->
|
||||
try {
|
||||
// Orientation is applied in the non-wall branch below; wall mode owns the
|
||||
|
|
@ -555,6 +565,17 @@ class MainActivity : AppCompatActivity() {
|
|||
}
|
||||
} // end else (not a video wall)
|
||||
|
||||
// #170: a genuine content change (first load, reassignment, toggle-back) resets any
|
||||
// stuck download backoff below, so "download missing" isn't blocked by a backoff that
|
||||
// ballooned during the unstable first-boot window. The routine 60s same-playlist refresh
|
||||
// keeps the same signature -> no reset -> the retry-storm guard (backoff) stays intact.
|
||||
val downloadSig = (0 until assignments.length()).mapNotNull { i ->
|
||||
val a = assignments.getJSONObject(i)
|
||||
if (a.isNull("content_id")) null else a.optString("content_id", "").ifEmpty { null }
|
||||
}.sorted().joinToString(",")
|
||||
val contentChanged = downloadSig != lastDownloadSig
|
||||
lastDownloadSig = downloadSig
|
||||
|
||||
// Download any missing local content (skip remote URLs).
|
||||
// Runs for wall + single-zone; multi-zone drives its own rendering via ZoneManager
|
||||
// (the startIfNeeded below is guarded so it won't run behind zones).
|
||||
|
|
@ -586,6 +607,7 @@ class MainActivity : AppCompatActivity() {
|
|||
// URL. ensure() is non-blocking and idempotent: it re-acks cached content (SEED-A),
|
||||
// defers when the socket is down (watchdog owns recovery), respects backoff, and
|
||||
// downloads at most once. It acks ready/failed itself (deduped via onAck).
|
||||
if (contentChanged) downloadCoordinator.resetBackoff(contentId) // #170: retry now, don't wait out a stale backoff
|
||||
downloadCoordinator.ensure(contentId, filename)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -96,6 +96,22 @@ class DownloadCoordinator(
|
|||
inFlight.remove(contentId); attempts.remove(contentId); nextAttemptAt.remove(contentId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear failure backoff for [contentId] WITHOUT touching single-flight (unlike forget()).
|
||||
* A genuine (re)assignment or a fresh network transition should retry a stuck item NOW instead
|
||||
* of waiting out a backoff that ballooned to the 5-min cap during the unstable first-boot window
|
||||
* (#170). Keeping inFlight means an in-progress download is never duplicated. No-op if not failing.
|
||||
*/
|
||||
fun resetBackoff(contentId: String) {
|
||||
attempts.remove(contentId); nextAttemptAt.remove(contentId)
|
||||
}
|
||||
|
||||
/** Clear ALL failure backoff (keep single-flight) — e.g. the network just (re)connected, so any
|
||||
* item that backed off while the link was settling should re-attempt on the next sweep. */
|
||||
fun resetAllBackoff() {
|
||||
attempts.clear(); nextAttemptAt.clear()
|
||||
}
|
||||
|
||||
/** Teardown (onDestroy): cancel in-flight downloads so none orphan or pin the Activity. */
|
||||
fun shutdown() {
|
||||
try { executor.shutdownNow() } catch (_: Throwable) {}
|
||||
|
|
|
|||
|
|
@ -114,6 +114,9 @@ class WebSocketService : Service() {
|
|||
var onRegistered: ((String) -> Unit)? = null
|
||||
var onPlaylistUpdate: ((JSONObject) -> Unit)? = null
|
||||
var onContentDelete: ((String) -> Unit)? = null
|
||||
// #170: fired when the default network (re)connects — the player clears stuck download backoff
|
||||
// so items that failed to download while the link was settling retry on the next sweep.
|
||||
var onNetworkAvailable: (() -> Unit)? = null
|
||||
var onScreenshotRequest: (() -> Unit)? = null
|
||||
var onRemoteStart: (() -> Unit)? = null
|
||||
var onRemoteStop: (() -> Unit)? = null
|
||||
|
|
@ -180,6 +183,16 @@ class WebSocketService : Service() {
|
|||
// Only meaningful mid-gap; if we're still connected this is a transient handoff.
|
||||
if (disconnectedAtMs != 0L) linkLostDuringGap = true
|
||||
}
|
||||
override fun onAvailable(network: Network) {
|
||||
// #170: fresh connectivity (boot Wi-Fi coming up, reconnect after a drop). Clear
|
||||
// any download backoff that ballooned while the link was settling, then pull the
|
||||
// current playlist so missing content re-downloads promptly. requestPlaylistRefresh
|
||||
// no-ops if the socket isn't connected yet; the normal register will follow.
|
||||
handler.post {
|
||||
onNetworkAvailable?.invoke()
|
||||
requestPlaylistRefresh()
|
||||
}
|
||||
}
|
||||
}
|
||||
cm.registerDefaultNetworkCallback(cb)
|
||||
netCallback = cb
|
||||
|
|
|
|||
|
|
@ -130,6 +130,31 @@ class DownloadCoordinatorTest {
|
|||
assertEquals("after backoff elapses, exactly one more attempt", after1 + 1, requests.get())
|
||||
}
|
||||
|
||||
// ===== #170: resetBackoff retries a stuck item NOW (genuine reassignment / toggle-back) =====
|
||||
@Test fun `resetBackoff re-attempts a stuck item before the backoff elapses`() {
|
||||
serve(null, null, status("404 Not Found"))
|
||||
coord.ensure("R", "v.bin"); waitAck("R:failed")
|
||||
val after1 = requests.get()
|
||||
repeat(3) { coord.ensure("R", "v.bin") }; Thread.sleep(200) // clock unchanged -> in backoff, skipped
|
||||
assertEquals("still in backoff -> no new attempt", after1, requests.get())
|
||||
coord.resetBackoff("R") // a genuine (re)assignment clears backoff
|
||||
coord.ensure("R", "v.bin"); Thread.sleep(300)
|
||||
assertEquals("resetBackoff -> retries NOW despite the clock not advancing", after1 + 1, requests.get())
|
||||
}
|
||||
|
||||
// ===== #170: resetAllBackoff clears every item (network just (re)connected) =====
|
||||
@Test fun `resetAllBackoff clears backoff for all items`() {
|
||||
serve(null, null, status("404 Not Found"))
|
||||
coord.ensure("A", "v.bin"); waitAck("A:failed")
|
||||
coord.ensure("B", "v.bin"); waitAck("B:failed")
|
||||
val after = requests.get()
|
||||
coord.ensure("A", "v.bin"); coord.ensure("B", "v.bin"); Thread.sleep(200)
|
||||
assertEquals("both in backoff -> no new attempts", after, requests.get())
|
||||
coord.resetAllBackoff()
|
||||
coord.ensure("A", "v.bin"); coord.ensure("B", "v.bin"); Thread.sleep(300)
|
||||
assertEquals("both re-attempt after resetAllBackoff", after + 2, requests.get())
|
||||
}
|
||||
|
||||
// ===== socket-down DEFER (ownership: watchdog owns recovery) =====
|
||||
@Test fun `socket down defers the download to the reconnect, then downloads when back`() {
|
||||
serve(null, null, full("data"))
|
||||
|
|
|
|||
Loading…
Reference in a new issue