Repository navigation
fix(migration): stream export manifest into zip to bound memory (COLUMBA-DS) - #1189
Merged
Merged
Conversation
…MBA-DS) Creating the export ZIP serialized the whole MigrationBundle with json.encodeToString(bundle), buffering the full manifest as a String plus a second byte array on top of the in-memory bundle. A ~150 MB manifest OOM'd the device in JsonToStringWriter (291 MB buffer allocation, COLUMBA-DS). Write the manifest with json.encodeToStream(serializer, bundle, zipOut), which streams the JSON into the zip entry through a bounded buffer - the same shape the import side already uses (json.decodeFromStream). Manifest bytes are unchanged: a new unit test pins encodeToStream output equal to encodeToString output for a fully-populated bundle. Verified: full migration unit-test suite green, including the production exportData -> import round-trip tests; ktlint + detekt clean. Fixes COLUMBA-DS
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…BA-DS) Forks a 256MB-heap child JVM that runs the real createExportZip on a ~100 MB manifest bundle and verifies the result through the import-side decodeFromStream. Pre-fix the child dies with OutOfMemoryError in JsonToStringWriter.ensureAdditionalCapacity - the same stack as the production COLUMBA-DS event; post-fix it completes with bounded memory. Verified both directions before/after the fix on this branch.
Owner
Author
|
@greptile review |
- Pass the test task's full runtime classpath to the forked child (columba.test.runtimeClasspath, set in doFirst) instead of relying on the Gradle worker's java.class.path, which does not include test classes. Verified: with the java.class.path fallback removed, the test still passes on the exposed classpath alone. - Run the manifest equivalence test with the exporter's production Json settings (prettyPrint + ignoreUnknownKeys) so it pins the bytes the real export writes. - Delete the child's temp work dir (and export archive) in a finally block so repeated runs leave nothing on disk.
Owner
Author
|
@greptile review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
COLUMBA-DS: fatal
OutOfMemoryErrorduring export on a Galaxy S25 Ultra (build 2.3.0-beta+20300002, 2026-10-01). The crash happens after the crypto OOM fix (PR #1128) — this is the next unbounded allocation in the same flow:createExportZipserialized the entireMigrationBundlewithjson.encodeToString(bundle).toByteArray()— buffering the full manifest text plus a second full byte array on top of the in-memory bundle. The crashing user's manifest was ~150 MB of JSON.Fix
Write the manifest with
json.encodeToStream(MigrationBundle.serializer(), bundle, zipOut), which streams the JSON into the zip entry through a bounded buffer — the same shape the import side already uses (json.decodeFromStream). Manifest bytes are unchanged.New unit test pins the contract:
encodeToStreamoutput is byte-identical toencodeToStringoutput for a fully-populated bundle.Verification
MigrationCallHistoryRoundTripTestproductionexportData-> import round-trip tests (these caught an intermediate wrong-overload resolution during development)Note: the in-memory
MigrationBundledata model itself (all DAO data collected into lists) remains the residual memory ceiling for very large datasets; this PR removes the serialization copy that actually crashed the device.Fixes COLUMBA-DS