Skip to content

Commit

Permalink
Fix collection load failure on Android
Browse files Browse the repository at this point in the history 
Loading a larger collection with V16 enabled was failing in openCollection
with error 6410, similar to simolus3/drift#876
  • Loading branch information
@dae
dae committed Jun 10, 2022
1 parent 940f1de commit f15e294
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions rslib/src/storage/sqlite.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ fn open_or_create_collection_db(path: &Path) -> Result<Connection> {
db.pragma_update(None, "cache_size", &(-40 * 1024))?;
db.pragma_update(None, "legacy_file_format", &false)?;
db.pragma_update(None, "journal_mode", &"wal")?;
// Android has no /tmp folder, and fails in the default config.
#[cfg(target_os = "android")]
db.pragma_update(None, "temp_store", &"memory")?;

This comment has been minimized.

Sign in to view

Copy link
@mikehardy

mikehardy Jun 10, 2022

Contributor

I read the linked issue - this looks like the "initial" fix the reporter tried, which worked until data was even larger, and then memory overflows right? doesn't this need a real temp directory

It appears there is an analog to the flutter-specific method getTemporaryDirectory method in the linked issue, and it is in std::env ? https://doc.rust-lang.org/std/env/fn.temp_dir.html#platform-specific-behavior - Android-specific discussion makes me think that might be the best way to make sure we are limited by storage as opposed to memory ?

Or I could be misunderstanding everything

This comment has been minimized.

Sign in to view

Copy link
@mikehardy

mikehardy Jun 10, 2022

Contributor

it occurs to me I'm maybe shouting in to the void by just dropping a comment on a commit, so for cross-linking this is from ankidroid/Anki-Android-Backend#193 and I'm tagging @dae - apologies if unnecessary

This comment has been minimized.

Sign in to view

Copy link
@dae

dae Jun 10, 2022

Author Member

AnkiDroid was actually already doing this in Storage.java:

            db.execute("PRAGMA temp_store = memory");

So this change simply moves the call earlier to prevent issues during collection load, and shouldn't be introducing any new memory pressure issues.

If you'd prefer to use a file, it's something you can do directly in AnkiDroid's code prior to initializing the backend:

        val dir = AnkiDroidApp.getInstance().applicationContext.cacheDir
        Os.setenv("TMPDIR", dir.path, true);

And the line above will need to be removed.

db.set_prepared_statement_cache_capacity(50);

Expand Down

0 comments on commit f15e294

Please sign in to comment.