Skip to content
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

SQLITE_ERROR: cannot start a transaction within a transaction #215

Closed
CosminCraciun opened this issue Feb 1, 2022 · 14 comments
Closed

SQLITE_ERROR: cannot start a transaction within a transaction #215

CosminCraciun opened this issue Feb 1, 2022 · 14 comments

Comments

@CosminCraciun
Copy link

Describe the bug
If 2 inserts/updates are run in parallel the SQLITE_ERROR: cannot start a transaction within a transaction is thrown.
It may happen if 2 operations are run asynchronously and not awaited or within a Promise.all. It's extremely susceptible in electron, but I have noticed it in web as well.

The following errors occur:

  1. Error: Error invoking remote method 'CapacitorSQLite-execute': Execute failed: ExecuteSQL: ExecuteSQL: BeginTransaction: SQLITE_ERROR: cannot start a transaction within a transaction
  2. Error: Error invoking remote method 'CapacitorSQLite-execute': Execute failed: ExecuteSQL: ExecuteSQL: CommitTransaction: SQLITE_ERROR: cannot commit - no transaction is active : RollbackTransaction: SQLITE_ERROR: cannot rollback - no transaction is active

To Reproduce
Have multiple database operations run under a await Promise.all([ ... ]) in electron.

Expected behavior
The package should handle waiting a transaction to finish before another one starts

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: windows 10
@jepiqueau
Copy link
Collaborator

@CosminCraciun you right multiple transactions is not implemented. Can you share an example with await Promise.all()

@jepiqueau
Copy link
Collaborator

@CosminCraciun The sqlite transactions are going so fast especially with Electron that you can manage to do them sequentially. Sqlite does not provide a queuing transactions manager and anyhow if it was existing it will run transaction one by one.

@CosminCraciun
Copy link
Author

@jepiqueau I managed to provide an example on the web version as well.

I understand, but the issue of colliding transactions will still exist, even unintentional. If an user event triggers a transaction, in the same time as some other service does some background work (for example some sort of synching might be done in the app).

I've created a repo with a simple example https://github.com/CosminCraciun/parallel-transactions

@jepiqueau
Copy link
Collaborator

@CosminCraciun i will have a look to your repo

@jepiqueau
Copy link
Collaborator

@CosminCraciun if you do this

        await Promise.all([
            await this.test('table1'),await this.test('table2'),
        ])

and

         await Promise.all([
          console.log(`t1 count: ${await this.getCount(this.t1)}`), console.log(`t2 count: ${await this.getCount(this.t2)}`)
         ])

where getCount

    private async getCount(tableName: string) {
      return (await this._db!.query(`Select Count(*) as count from ${tableName};`)).values![0].count;
    }

it works now is it going faster than

        await this.test(this.t1)
        await this.test(this.t2)
        const res1 = await this._db!.query(`Select Count(*) as count from ${this.t1};`),
        const res2 = await this._db!.query(`Select Count(*) as count from ${this.t1};`)

        console.log(res1.values![0].count);
        console.log(res2.values![0].count);

i did not test i you test it with 10000 items and measure the execution time revert to me the result

@CosminCraciun
Copy link
Author

The problem with

 await Promise.all([
            await this.test('table1'),await this.test('table2'),
        ])

It defeats the purpose of the Promise.all, as the await forces it to run sequentially. The Promise.all won't get a list of Promises anymore.

I have run some tests as you suggested, but haven't seen any differences in times. Which makes sense, as both run sequentially.
The only difference I saw is in some kind of db warmup... as if I run in the same run 2 tests, the second one is faster, no matter what the second test is.

Test results for 10 000 inserts:

  • Promise.all with await inside: 5606 ms
  • Pure sequential: 5742
    But repeating the tests they seem to converge, towards an average.

Are there any foreseeable plans of implementing a queuing transactions manager?

@jepiqueau
Copy link
Collaborator

@CosminCraciun i have been googling again and see that we can use the Write-Ahead Logging (WAL). so i implemented in angular-parallel-access. In fact Android is handling it automatically not the others platforms so for these ones i use PRAGMA journal_mode=WAL; and run the db.execute and db.run commands without transaction enable. Seems to work. Can you test it more and confirm to me if it works as you expect.

@CosminCraciun
Copy link
Author

Yes! It now it works as expected.
Thank you for your help.

Will this setting be included in a future version of the package?

@jepiqueau
Copy link
Collaborator

@CosminCraciun What the good things about PRAGMA is that it is a SQLite command so nothing to do in the plugin itself it is there already

@CosminCraciun
Copy link
Author

Great! I was thinking more that maybe the plugin could activate it automatically during the init phase, so you have more or less the same interaction without caring if you are on android or not.

Thanks a lot for your help. Ant thanks for maintaining this library!

@jepiqueau
Copy link
Collaborator

@CosminCraciun thanks for your comments and to bring this to my attention. I do not think i will do the initialisation in the plugin as some developers will prefer the old and secure transaction way. Since the plugin exists you are the first and the only one asking for this. What is nice is you can do parrallel access if you whish without impacting the code. You just have to know about the particularity of Android and manage it in your app code.

@mtnair
Copy link

mtnair commented Mar 28, 2022

Had the exact same issue in a work project where log messages were inserted into the database at the same time as other queries were run.

Just to be clear: Has this method without transactions enabled any downside? From your comments in the issue here I can't see anything related to this question.

And could you include the solution somewhere in the documentation so others will find this easier? I was googling way too long to find this issue.

@jepiqueau
Copy link
Collaborator

@comniemeer feel free to make a PR to improve the doc where it is needed i will merge the PR. I am just alone and do this on my free time. Hope you will understand

@folsze
Copy link
Contributor

folsze commented Apr 25, 2023

Had the exact same issue in a work project where log messages were inserted into the database at the same time as other queries were run.

Just to be clear: Has this method without transactions enabled any downside? From your comments in the issue here I can't see anything related to this question.

And could you include the solution somewhere in the documentation so others will find this easier? I was googling way too long to find this issue.

Exact same thing for me. I could create a PR. I just don't know where this should be added. Maybe here:
image

? I think this should be as visible as possible. I am sure I am not the only one with @mtnair that searched very long until we found this. The repository which shows how to use WAL is great though! I could add a section and then add some explanations for what to add and quote the repository.

I think placing inside API.md would not be where users would be searching for something like this? But maybe it's just me as a rather unadvanced programmer

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

No branches or pull requests

4 participants