Skip to content

Get all Sqlite connections in the pool #101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

davidmartos96
Copy link
Contributor

Fixes #62

This is a proposal API to fix the linked issue. We have been using it succesfully on our project for the past months. The main goal is to be able to attach a database on all the active connections, read and write. Additionally, we have our own open factory, which knows which databases are attached at any given time, so it will also attach new opened connections.

We use it like this:

extension SqliteAsyncExtension on SqliteDatabase {
  Future<void> attachDbOnAllConnections(String path, String alias, {String password = ''}) async {
    await detachDbOnAllConnections(alias);

    // This is the only way we can do an attach in all connections
    //
    // When using sqlcipher, the KEY needs to be provided always, otherwise it "inherits" the main db config
    // so we wouldn't be able to attach to an uneccrypted db
    try {
      await _runOnAllConnections((conn) => conn("ATTACH DATABASE ? AS ? KEY ?", [path, alias, password]));
    } catch (_) {
      // detach the db if it failed to attach on any connection
      await detachDbOnAllConnections(alias);
      rethrow;
    }
  }

  Future<void> detachDbOnAllConnections(String alias) async {
    await _runOnAllConnections((conn) async {
      // We need to check the list of attached databases to make the "warm" the detach command
      // if we try to try-catch it there are cases were it will block/not capture the error in the isolate
      final dbListRes = await conn(
        '''
        SELECT *
        FROM pragma_database_list
        WHERE name = ?
        ''',
        [alias],
      );

      final isAttached = dbListRes.isNotEmpty;
      if (isAttached) {
        // logger.i("Detached db: $alias");
        await conn('DETACH DATABASE ?', [alias]);
      }
    });
  }

  Future<void> _runOnAllConnections(
    Future<void> Function(ConnFun connFun) callback,
  ) async {
    final connections = getAllConnections();
    for (final conn in connections) {
      await callback(conn.getAll);
    }
  }
}

typedef ConnFun = Future<ResultSet> Function(String sql, [List<Object?> parameters]);

Copy link
Contributor

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I'm wondering about is whether a snapshot of all connections enough - e.g. I could imagine a potential race condition between usages of this call and the pool growing.

Another API which should work as well but might be safer to use could be something like this:

/// Locks all underlying connections making up this database, and gives [block] access to all of them at once.
Future<void> withAllConnections(Future<void> Function(List<({SqliteWriteContext context, bool isReadOnlyConnection})> connections) block);

That also feels a bit clumsy though, so I'm not sure if just exposing all connections directly might be easier for this advanced use-case that realistically requires cooperation with a custom open factory anyway.

@davidmartos96
Copy link
Contributor Author

@simolus3 We are happy to discuss a different API. Our only use case is the snippet above, and it's only used for attaching/detaching databases.
We are currently solving the attaching dbs problem with a file that holds the current attached databases. We need to use a file because it's the only way we have at the moment to share data between the pool isolates. More information in the linked issue.
And even though we write to the file synchronously before we run the attach, it's true that there could be a race condition if the pool was growing at that exact moment. It won't be aware of the current attached databases.
Is there any way we can use an existing mutex/lock from the library, that works cross isolates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Obtain all active connections in the pool
3 participants