Status Update
No update yet.
Comments
ya...@gmail.com
yb...@google.com
ya...@gmail.com
yb...@gmail.com
ya...@gmail.com
However, more I look at the code, more I believe the problem is elsewhere. If the migration fails, there will be no `SQLiteDatabase` and no `getWrappedDb(db)`. According to the code of `SQLiteOpenHelper`, there should be only one query before migration. The list of queries should be:
PRAGMA user_version;
DROP TABLE IF EXISTS `tracking_events` <--- this fails due closed database
DROP TABLE IF EXISTS `items` <-- never happens
More findings caught by my eye:
- I tried to make sure all DAO queries happen on a single thread. No success.
- The crash happens on a small number of users, but users get many crashes. I am afraid the user is unable to launch the app.
- Most crashes come from Samsung devices. But `SQLiteOpenHelper` class were not changed. All lines of stacktrace matches.
- `SQLiteOpenHelper` was constructed only once for `vinted_database.db`. However, meminfo shows something I don't understand:
DATABASES
pgsz dbsz Lookaside(b) cache Dbname
4 512 61 104/135/8 /data/user/0/
4 8 0/0/0 (attached) temp
4 512 25 35/14/2 /data/user/0/
Looks like database was opened two times. There was opened more databases in my app, but no one appears in this list.
What else I could check?
yb...@google.com
See this patch and specifically the test in it:
When the DB is corrupted (outside your control, and very little to do really) then the DB is immediately closed in hopes of creating a new DB (and a new object). What ends up happening is the corrupted object is still used causing the first db operation to fail (in this case a migration or that DROP TABLE IF EXISTS).
da...@google.com
yb...@google.com
do you mean doing it in the copying helper? Maybe even we can let createFromAsset builder methods to receive a callback which includes onPopulated?
da...@google.com
Yeah, doing it on copying helper, it's the most accurate place where we know a pre-packaged DB was used / copied. The callback itself can also be receive in createFromAsset / createFromFile, or just a new callback in RoomDatabase.Callback, I still think the tricky part is giving a DB in the callback itself (onPopulated(SuppoertSqliteDatabase db)) which requires opening the DB in the middle of the 'opening sequence'.
yb...@google.com
thats why i thought about scoping it to createFromFile(file, callback) so that it would be responsibility of the createFromFile to open the database and call the callback and then close it.
now it makes me wonder if that callback needs to be full fledged open helper callback instead of a special on populated? What if we just let them pass the regular callback there and if that callback exists, they can do whatever.
Another problem is though, we don't know the version so how are we going to open it in the first place :/.
da...@google.com
Seems reasonable to scope it and not be a full fledge callback. However, I still think it needs to be lazy called (when the DB is opened for the first time, usually on the first query done) and not when building the DB (via the builder). The callback is really only useful is it receives the DB as a param so the user gets a chance to do something, so still think we need to open the DB and close it.
By the way, we can read the DB version without opening the DB:
ya...@gmail.com
But that means that we would be adding 2 new methods to the builder - createFromAsset/createFromFile but with additional parameter, right? So that we don't break backwards compatibility. I'll play with this ideas tomorrow.
But what about "transactionality"? Does putting this in copying helper make any difference? Do you think we still should use Room master table to detect if this new onPopulated callback should be called or not?
yb...@google.com
ugh thats a good point.
Danny knows better but my first instinct is to let SQLiteCopyOpenHelper take care of calling it.
It also needs to know that it is expected to open the database :/ so needs to keep track of that information as well not to open the database when it doesn't need to.
we might possibly go a bit loose on transactionality and just keep an additional file next to the database to keep this information.
the downside is that we'll be creating more junk that we can never delete.
ap...@google.com
Branch: androidx-master-dev
commit 52c2e0b99eb8e3638a7fb8645772a529d42653f8
Author: mzgreen <yairobbe@gmail.com>
Date: Fri Aug 07 20:22:02 2020
[GH] Add onOpenPrepackagedDatabase callback
This patch adds a new builder methods that accept new
PrepackagedCallback for creating a database. The new PrepackagedCallback
contains a single method onOpenPrepackagedDatabase(SupportSQLiteDatabase)
which is called right after creating a db from Asset/File/InputStream.
onOpenPrepackagedDatabase(SupportSQLiteDatabase) callback is transactional
which means that it'll be called only once after creating a db from
Asset/File/InputStream. If an exception is thrown inside onOpenPrepackagedDatabase
callback then the next time this database is opened, it should be created from
Asset/File/InputStream again and onOpenPrepackagedDatabase should also be called
again.
Relnote: "Add a onOpenPrepackagedDatabase callback for when a prepackaged
DB is copied."
Test: Added a unit tests to PrepackageTest.
Fixes:
This is an imported pull request from
Resolves #55
Github-Pr-Head-Sha: 330c068dda27ff3ee3edc5c9166a1f3218e6acf8
GitOrigin-RevId: 1771ccf4458fdd2c9318ba413e4a1576da3c0024
Change-Id: I1ba740f679565f5c97548f6683de40b0f40f9024
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/PrepackageTest.java
M room/runtime/api/current.txt
M room/runtime/api/public_plus_experimental_current.txt
M room/runtime/api/restricted_current.txt
M room/runtime/src/main/java/androidx/room/DatabaseConfiguration.java
M room/runtime/src/main/java/androidx/room/RoomDatabase.java
M room/runtime/src/main/java/androidx/room/SQLiteCopyOpenHelper.java
M room/testing/src/main/java/androidx/room/testing/MigrationTestHelper.java
ap...@google.com
Branch: androidx-master-dev
commit 52c2e0b99eb8e3638a7fb8645772a529d42653f8
Author: mzgreen <yairobbe@gmail.com>
Date: Fri Aug 07 20:22:02 2020
[GH] Add onOpenPrepackagedDatabase callback
This patch adds a new builder methods that accept new
PrepackagedCallback for creating a database. The new PrepackagedCallback
contains a single method onOpenPrepackagedDatabase(SupportSQLiteDatabase)
which is called right after creating a db from Asset/File/InputStream.
onOpenPrepackagedDatabase(SupportSQLiteDatabase) callback is transactional
which means that it'll be called only once after creating a db from
Asset/File/InputStream. If an exception is thrown inside onOpenPrepackagedDatabase
callback then the next time this database is opened, it should be created from
Asset/File/InputStream again and onOpenPrepackagedDatabase should also be called
again.
Relnote: "Add a onOpenPrepackagedDatabase callback for when a prepackaged
DB is copied."
Test: Added a unit tests to PrepackageTest.
Fixes:
This is an imported pull request from
Resolves #55
Github-Pr-Head-Sha: 330c068dda27ff3ee3edc5c9166a1f3218e6acf8
GitOrigin-RevId: 1771ccf4458fdd2c9318ba413e4a1576da3c0024
Change-Id: I1ba740f679565f5c97548f6683de40b0f40f9024
M room/runtime/api/current.txt
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/PrepackageTest.java
M room/runtime/api/public_plus_experimental_current.txt
M room/runtime/src/main/java/androidx/room/SQLiteCopyOpenHelper.java
M room/runtime/src/main/java/androidx/room/DatabaseConfiguration.java
M room/runtime/api/restricted_current.txt
M room/runtime/src/main/java/androidx/room/RoomDatabase.java
M room/testing/src/main/java/androidx/room/testing/MigrationTestHelper.java
Description
Version used: 2.2.3
Thanks a lot for long-awaited `createFromAssets\createFromFile` feature!
But since the copied database is not considered a new database, need the possibility do business logic when database actually copied from assets by subscription to special callback `RoomDatabase.Callback#onPopulated`, for instance.
Now, to workaround this we need to do a lot of things.