Cross Isolate issue

[absar]

Having a once in a few months issue, the DB version is updated to e.g. v5 but the new tables are not created. Since the upgrade happens in a transaction the full transaction should have completed or rolled back, as SQFlite is setting the version at the end of SqfliteDatabaseMixin.doOpen and I am not doing any exception handling in onUpgrade. The app is opening the DB from a background workmanager Isolate and of course from the front end as well when user opens the App, both background job and frontend app are using the same database open function on the same database file and the DB is never explicitly closed, using singleInstance: true in all places which are opening the DB.

To counter the issue I am planning to NOT run the upgrade process from the background isolate until the DB is upgraded when user opens the App.
These are the planned steps, is it the right way or there is a more safer way:

1. When the background task runs it will first open the DB in read only mode with singleInstance: false: openReadOnlyDatabase(dbPath, singleInstance: false)
2. With the read only DB get DB version
3. If the version is less than latest expected version then close the read only DB and terminate the background task
4. However if the version is latest, close the read only DB and open DB normally with singleInstance: true
5. The reason to use a read only DB is to not fire onCreate onUpgrade
6. However if the App is opened by user then this read only version check will not happen

Using SQFlite 2.2.8+4 with Flutter 3.10

class LocalDatabaseService {
  static LocalDatabaseService? _instance;
  static Database? _database;
  static const int currentDBVersion = 3;
  
  static Future<LocalDatabaseService?> getInstance(ProcessRunBy processRunBy) async {
    if (_instance == null) {
      final dbPath = join(await getDatabasesPath(), dataBaseName);

      if (processRunBy.isBackend) {
        final readonlyDb = await openReadOnlyDatabase(dbPath, singleInstance: false);
        try {
          final dbVersion = await readonlyDb.getVersion();
          if (dbVersion < currentDBVersion) {
            logger.i('DB version $dbVersion is not updated to latest version $currentDBVersion, will terminate backend process');
            return _instance;
          }
        } finally {
          await readonlyDb.close();
        }
      }

      _instance = LocalDatabaseService._internal();

      _database = await openDatabase(
        dbPath, version: currentDBVersion,

        onCreate: (db, version) async {
          final batch = db.batch();
          _createDatabaseLatestVersion(batch);
          await batch.commit(noResult: true);
        },

        onUpgrade: (db, oldVersion, newVersion) async {
          if (oldVersion == 1) {
            final batch = db.batch();
            _upgradeDatabaseToV2(batch);
            await batch.commit(noResult: true);
          }

          if (oldVersion < 3) {
            await _upgradeDatabaseToV3(db);
          }
        },
      );
    }
    return _instance!;
  }
}

[absar]

@alextekartik can you please give it a look when you have some time

[alextekartik]

My experiments with isolate are very limited but this looks fine to me and I took a similar approach in the past. I think I decided to open with singleInstance: false in the background isolate (I was doing a small action and closing the database - and like you was not performing any schema update from the isolage) but as long as you don't close the database in any isolate it is even better to use singleInstance: true like you do to prevent any memory leak - now sqflite transactions are cross isolate safe.

I understand testing is complex here (I was mainly using print for debugging in theses cases). Does it work fine so far and are you having any issue with your solution ?

[absar]

Thanks for the answer. Do you know what can be the scenarios in which the version will get updated without running the full upgrade, this needs to be investigated since it is a very risky behavior, the upgrade happens in a transaction the full transaction should have completed or rolled back including version update.

Well it's in the planning phase, haven't rolled out yet, however I added another check to know whether DB exists or not, which should be run before checking DB version using openReadOnlyDatabase, since this will throw if DB does not exist

[alextekartik]

Do you know what can be the scenarios in which the version will get updated without running the full upgrade

Ah yes that was the original issue you reported, sorry. As you mentioned since the whole upgrade process run in a transaction this cannot happen.
One possibility would be the somehow COMMIT was called from another isolate but here as well that should not happen (unless there is bug). What I did in my case when I was opening from the background isolate: I was simply throwing an exception if onCreate or onUpdate was called. If you want to share your _createDatabaseLatestVersion_upgradeDatabaseToV2_upgradeDatabaseToV3, I can also take a look but you seem expert enough.

[absar]

One possibility would be the somehow COMMIT was called from another isolate but here as well

I need to understand this part, why should it be a problem, since in other Isolate as well first the upgrade will run, there also is a transaction which should have atomicity committing at the end, which means even if other Isolate tried to update the DB, shouldn't that also either fully commit or rollback the upgrade.
This opens some more questions:

• Are there any scenarios in which an exception within a transaction is ignored and the version is updated, even if we are NOT using ConflictAlgorithm.ignore or continueOnError: true

• Does noResult e.g. batch.commit(noResult: true) ignore exceptions, my understanding is it should not ignore right?

Here is the pseudo code of the methods you mentioned:

• _createDatabaseLatestVersion create all tables with latest fields, and does inserts
• _upgradeDatabaseToV2 drops a table, creates two new table
• _upgradeDatabaseToV3 in a new batch creates a new table, adds new columns to an existing table, commits the batch, commit is necessary since we need to insert data below
  does db.update to fill values in newly created columns
  does db.rawQuery to get some data
  creates a new batch and inserts data into the new table, then commits this batch

[alextekartik]

I need to understand this part, why should it be a problem, since in other Isolate as well first the upgrade will run, there also is a transaction which should have atomicity committing at the end

Well it is a bit more complicated as statements are not ran in the isolate itself. SQLite transaction is based on threading and sqflite runs on a separate thread so SQL requests are sent from all isolates on this thread for execution. In the past it was possible to have one isolate running a statement while another isolate was running a transaction which was very bad. This should be solved now.

Are there any scenarios in which an exception within a transaction is ignored and the version is updated

No and throwing an exception from within a transaction is the best way to rollback a transaction.

Does noResult e.g. batch.commit(noResult: true) ignore exceptions, my understanding is it should not ignore right?

Correct.

Here is the pseudo code of the methods you mentioned:

As long as your properly await all you statements (that is what I wanted to check), it should be fine. Indeed don't use continueOnError: true

So basically all your assumptions are correct and unless there is a bug in sqflite or your code (and I have to admit that I have avoided background isolates in my own applications), what you experienced (version bumped while not having the expected schema) should not happen...sorry for now having a better response...

[absar]

SQLite transaction is based on threading and sqflite runs on a separate thread so SQL requests are sent from all isolates on this thread for execution

Interesting indeed, this is the missing puzzle I was looking for, I will try to play with Isolates and try to reproduce the issue.
Need some more info though, if two Isolates start almost at the same time, both have long multiple DDL and DML statements with batches and commits in the middle, how do the two Isolates handover these statements to the sqflite thread, does one Isolate wait till the other Isolate is completely done, e.g. until the transaction is committed. Is there a design document somewhere in the repo which has this info.

Thank you very much for the extended help, and sorry to bother you

[alextekartik]

does one Isolate wait till the other Isolate is completely done, e.g. until the transaction is committed

That's correct, the 'server' threads know from which isolate the transaction comes from and suspend other requests.

Is there a design document somewhere in the repo which has this info.
Thank you very much for the extended help, and sorry to bother you

Unfortunately I don't have much documentation but sometimes I use what I write in discussions (such as this one) and issues to write a small document. You are not bothering me (but sometimes I don't have time), you spent a fair amount of time writing/describing your issue and I know this cross isolate issue is something that needs more attention. For example I wonder what would happen if a background isolate is killed in the middle of a transaction...So far I have avoided using sqflite in a background isolate as much as possible and use workarounds to avoid potential issues but of course that depends of your needs...