mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 15:29:15 +01:00
sync: batch the fallback getAlbum song fetches
The `search3`-less fallback emitted one batch per album, and `SyncService.writeSongs` opens a transaction per batch, so syncing a large library that way ran one transaction per album — thousands of fsyncs, and an invalidation round for every list observing `songs` with each. Accumulate what each `getAlbum` returns and flush at `PAGE_SIZE`, so the fallback writes the same ~500-row batches every other fetch already does. Albums are fetched concurrently and complete out of order, so the accumulator sits behind a mutex and the send happens outside it. Batches are in-flight memory, so the flush is per rows reached rather than per album page. Measured per `Song` (strings, boxed longs and genre list included): ~0.7 KiB, so a 500-row batch is ~334 KiB plus ~51 KiB of the `names`/`genres` structures `writeSongs` derives, where a whole album page (~5,500 rows) would be ~3.8 MiB. `albumSongs()` is also a bare `channelFlow`, whose channel holds 64 elements whatever their size, so 500-row batches would have made its worst case ~21 MiB; it now carries the same `.buffer(1)` that `entityBatches()` already uses on itself. The rest of the in-flight rows scale with `syncConcurrency`, so that leaves the default's batch in hand and a few MiB at the highest setting. `SubsonicSourceTest.albumSongsAreEmittedInBoundedBatches` asserts the emitted sizes, [500, 100] for 600 songs spread over 200 three-song albums, which pins the carried remainder as well as the flush: verified it fails on the pre-change code (200 batches of 3) and against a mutant that drops the remainder ([500, 99]). `docs/architecture.md`'s page cap was 20x out of date, so it is corrected alongside. `ktlintCheck`, `testDebugUnitTest` and `lintDebug` pass.
This commit is contained in:
@@ -25,7 +25,9 @@ import kotlinx.coroutines.flow.flow
|
|||||||
import kotlinx.coroutines.flow.flowOn
|
import kotlinx.coroutines.flow.flowOn
|
||||||
import kotlinx.coroutines.flow.map
|
import kotlinx.coroutines.flow.map
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
|
import kotlinx.coroutines.sync.Mutex
|
||||||
import kotlinx.coroutines.sync.Semaphore
|
import kotlinx.coroutines.sync.Semaphore
|
||||||
|
import kotlinx.coroutines.sync.withLock
|
||||||
import kotlinx.coroutines.sync.withPermit
|
import kotlinx.coroutines.sync.withPermit
|
||||||
import kotlinx.coroutines.withContext
|
import kotlinx.coroutines.withContext
|
||||||
import org.xml.sax.Attributes
|
import org.xml.sax.Attributes
|
||||||
@@ -224,6 +226,23 @@ class SubsonicSource(
|
|||||||
private fun albumSongs(): Flow<List<Song>> =
|
private fun albumSongs(): Flow<List<Song>> =
|
||||||
channelFlow {
|
channelFlow {
|
||||||
val permits = Semaphore(maxConcurrentFetches)
|
val permits = Semaphore(maxConcurrentFetches)
|
||||||
|
val lock = Mutex()
|
||||||
|
val pending = ArrayList<Song>(PAGE_SIZE)
|
||||||
|
|
||||||
|
suspend fun accept(songs: List<Song>) {
|
||||||
|
lock
|
||||||
|
.withLock {
|
||||||
|
pending.addAll(songs)
|
||||||
|
if (pending.size < PAGE_SIZE) {
|
||||||
|
null
|
||||||
|
} else {
|
||||||
|
val batch = ArrayList(pending.subList(0, PAGE_SIZE))
|
||||||
|
pending.subList(0, PAGE_SIZE).clear()
|
||||||
|
batch
|
||||||
|
}
|
||||||
|
}?.let { send(it) }
|
||||||
|
}
|
||||||
|
|
||||||
var offset = 0
|
var offset = 0
|
||||||
var pages = 0
|
var pages = 0
|
||||||
while (true) {
|
while (true) {
|
||||||
@@ -251,7 +270,7 @@ class SubsonicSource(
|
|||||||
onChild = { song, name, attrs -> if (name == "genres") song.addGenre(attrs) },
|
onChild = { song, name, attrs -> if (name == "genres") song.addGenre(attrs) },
|
||||||
).collect { batch ->
|
).collect { batch ->
|
||||||
val accepted = batch.map { it.toSong() }.filter { it.id.isNotEmpty() }
|
val accepted = batch.map { it.toSong() }.filter { it.id.isNotEmpty() }
|
||||||
if (accepted.isNotEmpty()) send(accepted)
|
accept(accepted)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -265,7 +284,14 @@ class SubsonicSource(
|
|||||||
if (pages > maxPages) throw pageCapExceeded()
|
if (pages > maxPages) throw pageCapExceeded()
|
||||||
offset += PAGE_SIZE
|
offset += PAGE_SIZE
|
||||||
}
|
}
|
||||||
}
|
val tail =
|
||||||
|
lock.withLock {
|
||||||
|
val held = pending.toList()
|
||||||
|
pending.clear()
|
||||||
|
held
|
||||||
|
}
|
||||||
|
if (tail.isNotEmpty()) send(tail)
|
||||||
|
}.buffer(1)
|
||||||
|
|
||||||
private fun supportsEmptyQuerySearch(): Boolean {
|
private fun supportsEmptyQuerySearch(): Boolean {
|
||||||
emptyQuerySearchSupported?.let { return it }
|
emptyQuerySearchSupported?.let { return it }
|
||||||
|
|||||||
@@ -608,6 +608,40 @@ class SubsonicSourceTest {
|
|||||||
assertTrue(maxInFlight.get() in 2..DEFAULT_FETCH_CONCURRENCY)
|
assertTrue(maxInFlight.get() in 2..DEFAULT_FETCH_CONCURRENCY)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun albumSongsAreEmittedInBoundedBatches() =
|
||||||
|
runBlocking {
|
||||||
|
val albumCount = 200
|
||||||
|
val songsPerAlbum = 3
|
||||||
|
server.dispatcher =
|
||||||
|
object : Dispatcher() {
|
||||||
|
override fun dispatch(request: RecordedRequest): MockResponse {
|
||||||
|
val url = request.requestUrl!!
|
||||||
|
return when (url.encodedPath) {
|
||||||
|
"/rest/search3.view" -> {
|
||||||
|
MockResponse().setBody(failed(10, "Required parameter is missing"))
|
||||||
|
}
|
||||||
|
|
||||||
|
"/rest/getAlbumList2.view" -> {
|
||||||
|
MockResponse().setBody(albumListOf(albumCount))
|
||||||
|
}
|
||||||
|
|
||||||
|
"/rest/getAlbum.view" -> {
|
||||||
|
MockResponse().setBody(albumWithSongs(url.queryParameter("id")!!, songsPerAlbum))
|
||||||
|
}
|
||||||
|
|
||||||
|
else -> {
|
||||||
|
MockResponse().setResponseCode(404)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
val batches = SubsonicSource(1, client()).songs().toList().map { it.size }
|
||||||
|
|
||||||
|
assertEquals(listOf(500, 100), batches)
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun playlistEntriesWithoutIdsAreSkipped() =
|
fun playlistEntriesWithoutIdsAreSkipped() =
|
||||||
runBlocking {
|
runBlocking {
|
||||||
@@ -705,6 +739,13 @@ class SubsonicSourceTest {
|
|||||||
"<song id=\"$albumId-s1\" title=\"One\"/>" +
|
"<song id=\"$albumId-s1\" title=\"One\"/>" +
|
||||||
"</album></subsonic-response>"
|
"</album></subsonic-response>"
|
||||||
|
|
||||||
|
private fun albumWithSongs(
|
||||||
|
albumId: String,
|
||||||
|
count: Int,
|
||||||
|
) = "<subsonic-response status=\"ok\"><album id=\"$albumId\">" +
|
||||||
|
(1..count).joinToString("") { "<song id=\"$albumId-s$it\" title=\"Title $it\"/>" } +
|
||||||
|
"</album></subsonic-response>"
|
||||||
|
|
||||||
private fun failed(
|
private fun failed(
|
||||||
code: Int,
|
code: Int,
|
||||||
message: String,
|
message: String,
|
||||||
|
|||||||
@@ -120,7 +120,7 @@ Images: grids and lists request a 256 px `getCoverArt` thumbnail; the 120 dp det
|
|||||||
|
|
||||||
A run is triggered by the user (pull-to-refresh or a Sync action) or by `AutoSync`, a foreground-only timer. While the app has a visible Activity `AutoSync` re-checks at most every 15 minutes (the configured interval, capped) whether `syncIntervalMinutes` has elapsed since the last attempt and, if the mode allows (`Off`, `Wi-Fi only` or `Always`, default `Wi-Fi only`), the app is online and a server is configured, it asks `SyncManager` for a silent sync; nothing runs while the app is backgrounded, and a missed interval catches up on the next open. Only the time of the last attempt is persisted, so a server that keeps failing is retried once per interval rather than on every poll. A silent sync still shows the progress notification and the Settings spinner, but never a toast.
|
A run is triggered by the user (pull-to-refresh or a Sync action) or by `AutoSync`, a foreground-only timer. While the app has a visible Activity `AutoSync` re-checks at most every 15 minutes (the configured interval, capped) whether `syncIntervalMinutes` has elapsed since the last attempt and, if the mode allows (`Off`, `Wi-Fi only` or `Always`, default `Wi-Fi only`), the app is online and a server is configured, it asks `SyncManager` for a silent sync; nothing runs while the app is backgrounded, and a missed interval catches up on the next open. Only the time of the last attempt is persisted, so a server that keeps failing is retried once per interval rather than on every poll. A silent sync still shows the progress notification and the Settings spinner, but never a toast.
|
||||||
|
|
||||||
Songs have a primary and a fallback fetch. The primary probes `search3` with an empty query once (memoized per source); if the server supports it, the library is paged through `search3` 500 songs at a time. Otherwise the fallback pages `getAlbumList2` and then issues one `getAlbum` per album — it works everywhere but costs a request per album instead of per 500 songs. Every paged loop stops on a short page and is capped at 1000 pages. Playlists are fetched whole, and their songs are stored by position so a playlist that got shorter (or was emptied) is trimmed at the end.
|
Songs have a primary and a fallback fetch. The primary probes `search3` with an empty query once (memoized per source); if the server supports it, the library is paged through `search3` 500 songs at a time. Otherwise the fallback pages `getAlbumList2` and then issues one `getAlbum` per album — it works everywhere but costs a request per album instead of per 500 songs, though it still coalesces the songs into 500-row batches before writing them. Every paged loop stops on a short page and is capped at 20,000 pages. Playlists are fetched whole, and their songs are stored by position so a playlist that got shorter (or was emptied) is trimmed at the end.
|
||||||
|
|
||||||
Cleanup is the "everything I had, minus everything the server just sent" step described above: artists, albums, songs and playlists delete the leftover IDs in 500-row chunks, disc titles are deleted per `(album, disc)` for albums that still exist, and playlist entries beyond the new count are removed along with entries for playlists that disappeared.
|
Cleanup is the "everything I had, minus everything the server just sent" step described above: artists, albums, songs and playlists delete the leftover IDs in 500-row chunks, disc titles are deleted per `(album, disc)` for albums that still exist, and playlist entries beyond the new count are removed along with entries for playlists that disappeared.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user