mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 19:53:08 +00:00
ci: cap Gradle memory on the Android job; add AppPreferenceStores.release
Two small things. **The Android job's memory.** gradle.properties asks for -Xmx6g (Gradle) plus -Xmx8g and 2g metaspace (Kotlin daemon). That is roughly 16GB of ceiling on a 16GB ubuntu-latest runner, before the launcher JVM, Lint's fork and the KSP workers are counted, and no CI job overrides it. test-and-build-android is the only job that comes near that ceiling — two lint variants, two flavours of unit tests, assembleBenchmark — and it is the only job that has died: five times, each one mid-compile or mid-lint rather than at a random moment, reported as "the runner has received a shutdown signal". That is what the Linux OOM killer taking the runner agent looks like from the outside, and it is the same configuration, on the same 16GB size, that repeatedly killed the Gradle daemon in the container this branch was developed in. Capped to 4g apiece for this job only, so local builds on bigger machines keep their headroom. 4g was verified to complete lintFdroidBenchmark and both unit-test tasks; it trades some build time for a job that finishes. This corrects an earlier guess of mine, posted on the PR, that `concurrency: cancel-in-progress` was behind these. It is not: a concurrency cancel ends with conclusion `cancelled`, and these are `failure`. The cancelled runs on main are a separate and expected effect of merging quickly. **AppPreferenceStores.release.** There was no way to let go of a store, so "write the file, reopen it, check what is on disk" was impossible — which is why the migration-guard test had to be driven against the DataMigration directly rather than through a real reopen. Mirrors AccountPreferenceStores.removeAccount, including the join: cancel() only asks, and DataStore's registry entry survives until the owning job actually completes. That detail produced "there are multiple DataStores active for the same file" twice in this codebase already. Production has no reason to call it; these stores live as long as the process. Two tests now use it, and the second is the one that was missing: a migration runs when the file is first opened and does NOT run again when a later instance opens the same file — the copy-once guarantee the Cashu counter and UI settings copies both rest on, checked the way it actually happens at runtime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AXvKXakvup4inNFfAhhr4L
This commit is contained in:
@@ -380,9 +380,25 @@ jobs:
|
|||||||
# variants are compile-equivalent for unit-test purposes; running all six
|
# variants are compile-equivalent for unit-test purposes; running all six
|
||||||
# adds ~5× the kotlinc work without catching new defects on PRs. Push to
|
# adds ~5× the kotlinc work without catching new defects on PRs. Push to
|
||||||
# main still gets the full test matrix via the production-build path.
|
# main still gets the full test matrix via the production-build path.
|
||||||
|
# Memory caps, CI-only. gradle.properties asks for -Xmx6g (Gradle) plus
|
||||||
|
# -Xmx8g and 2g metaspace (Kotlin daemon), which is ~16GB of ceiling on a
|
||||||
|
# 16GB ubuntu-latest runner before the launcher JVM, Lint's fork and the
|
||||||
|
# KSP workers are counted. Every other job stays well under it; this one
|
||||||
|
# runs two lint variants, two flavours of unit tests and
|
||||||
|
# assembleBenchmark, and it is the only job that has died — five times,
|
||||||
|
# each mid-compile or mid-lint, reported as "the runner has received a
|
||||||
|
# shutdown signal", which is what the Linux OOM killer taking the runner
|
||||||
|
# agent looks like from the outside.
|
||||||
|
#
|
||||||
|
# Overridden here rather than in gradle.properties so local builds on
|
||||||
|
# bigger machines keep the headroom. 4g apiece was verified to complete
|
||||||
|
# lintFdroidBenchmark and both unit-test tasks; it trades some build time
|
||||||
|
# for a job that finishes.
|
||||||
- name: Test + Build Android (gradle)
|
- name: Test + Build Android (gradle)
|
||||||
run: |
|
run: |
|
||||||
./gradlew \
|
./gradlew \
|
||||||
|
-Dorg.gradle.jvmargs="-Xmx4g -Dfile.encoding=UTF-8" \
|
||||||
|
-Dkotlin.daemon.jvmargs="-Xmx4g -XX:MaxMetaspaceSize=1g" \
|
||||||
:amethyst:lintFdroidBenchmark \
|
:amethyst:lintFdroidBenchmark \
|
||||||
:amethyst:lintPlayBenchmark \
|
:amethyst:lintPlayBenchmark \
|
||||||
:quartz:jvmTest \
|
:quartz:jvmTest \
|
||||||
|
|||||||
+29
@@ -30,6 +30,8 @@ import kotlinx.coroutines.CoroutineScope
|
|||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.IO
|
import kotlinx.coroutines.IO
|
||||||
import kotlinx.coroutines.SupervisorJob
|
import kotlinx.coroutines.SupervisorJob
|
||||||
|
import kotlinx.coroutines.cancel
|
||||||
|
import kotlinx.coroutines.job
|
||||||
import okio.Path
|
import okio.Path
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -117,6 +119,33 @@ class AppPreferenceStores(
|
|||||||
/** The file UI, Tor, OTS, Namecoin and friends share. */
|
/** The file UI, Tor, OTS, Namecoin and friends share. */
|
||||||
fun sharedSettings(): DataStore<Preferences> = getDataStore(SHARED_SETTINGS)
|
fun sharedSettings(): DataStore<Preferences> = getDataStore(SHARED_SETTINGS)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Releases the store for [name], so the file can be opened again.
|
||||||
|
*
|
||||||
|
* DataStore keeps a process-wide registry keyed by path and refuses a second
|
||||||
|
* live instance, and `cancel()` only *asks* a scope to stop — the registry
|
||||||
|
* entry survives until the owning job actually completes, which is why this
|
||||||
|
* joins. Getting that wrong produced "there are multiple DataStores active
|
||||||
|
* for the same file" twice in this codebase already.
|
||||||
|
*
|
||||||
|
* Production has no reason to call this: these stores live as long as the
|
||||||
|
* process. It exists so a test can write a file, let go of it, and reopen it
|
||||||
|
* to check what is actually on disk — the one thing that was impossible
|
||||||
|
* before, and the reason the migration-guard test had to be driven against
|
||||||
|
* the DataMigration directly instead.
|
||||||
|
*
|
||||||
|
* Returns false if nothing was open under that name.
|
||||||
|
*/
|
||||||
|
suspend fun release(name: String): Boolean {
|
||||||
|
val entry = storeCache.get(name) ?: return false
|
||||||
|
|
||||||
|
entry.scope.cancel()
|
||||||
|
entry.scope.coroutineContext.job
|
||||||
|
.join()
|
||||||
|
storeCache.remove(name)
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The names of stores already on disk whose name starts with [prefix].
|
* The names of stores already on disk whose name starts with [prefix].
|
||||||
*
|
*
|
||||||
|
|||||||
+58
@@ -26,6 +26,7 @@ import kotlinx.coroutines.flow.first
|
|||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
import okio.Path.Companion.toOkioPath
|
import okio.Path.Companion.toOkioPath
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertFalse
|
||||||
import org.junit.Assert.assertNull
|
import org.junit.Assert.assertNull
|
||||||
import org.junit.Assert.assertSame
|
import org.junit.Assert.assertSame
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
@@ -121,4 +122,61 @@ class AppPreferenceStoresTest {
|
|||||||
|
|
||||||
assertTrue("cashu_npubA" in asked && "cashu_npubB" in asked && "shared_settings" in asked)
|
assertTrue("cashu_npubA" in asked && "cashu_npubB" in asked && "shared_settings" in asked)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Releasing a store lets the same file be opened again.
|
||||||
|
*
|
||||||
|
* DataStore's registry is keyed by path and `cancel()` only asks, so this is
|
||||||
|
* only safe because [AppPreferenceStores.release] joins the scope's job.
|
||||||
|
* Without the join this test is exactly the "multiple DataStores active for
|
||||||
|
* the same file" crash.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun aReleasedStoreCanBeReopenedAndStillHasItsData() =
|
||||||
|
runTest {
|
||||||
|
val key = stringPreferencesKey("k")
|
||||||
|
val subject = stores()
|
||||||
|
|
||||||
|
subject.getDataStore("reopen").edit { it[key] = "written once" }
|
||||||
|
|
||||||
|
assertTrue("something was open", subject.release("reopen"))
|
||||||
|
assertFalse("and now nothing is", subject.release("reopen"))
|
||||||
|
|
||||||
|
// a genuinely new instance over the same file
|
||||||
|
val reopened = subject.getDataStore("reopen")
|
||||||
|
assertEquals("written once", reopened.data.first()[key])
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The guard the Cashu and UI copies both rest on, now checked the way it
|
||||||
|
* actually happens at runtime: the migration runs when the file is first
|
||||||
|
* opened, and must not run again when a later instance opens the same file.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
fun aMigrationRunsOnceEvenAcrossAReopen() =
|
||||||
|
runTest {
|
||||||
|
val marker = stringPreferencesKey("copied")
|
||||||
|
var runs = 0
|
||||||
|
|
||||||
|
val subject =
|
||||||
|
AppPreferenceStores(
|
||||||
|
rootFilesDir = { folder.root.toOkioPath() },
|
||||||
|
migrations = {
|
||||||
|
listOf(
|
||||||
|
CopyOnceMigration("migrated.once") { out ->
|
||||||
|
runs++
|
||||||
|
out[marker] = "run $runs"
|
||||||
|
},
|
||||||
|
)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assertEquals("run 1", subject.getDataStore("once").data.first()[marker])
|
||||||
|
assertEquals(1, runs)
|
||||||
|
|
||||||
|
subject.release("once")
|
||||||
|
|
||||||
|
assertEquals("still the first copy", "run 1", subject.getDataStore("once").data.first()[marker])
|
||||||
|
assertEquals("the migration must not run a second time", 1, runs)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user