Skip to content

Conversation

@alumni
Copy link
Collaborator

@alumni alumni commented Jan 25, 2025

Description of change

Investigating SQLite flaky tests.

Currently identified issue is possibly related to #11260:

  1) github issues > #10209
       should not fail to run multiple nested transactions in parallel:
     QueryFailedError: SQLITE_ERROR: no such savepoint: typeorm_2
      at Statement.handler (src/driver/sqlite/SqliteQueryRunner.ts:17:65)
      at Statement.replacement (node_modules/sqlite3/lib/trace.js:25:27)
      at Statement.replacement (node_modules/sqlite3/lib/trace.js:25:27)

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run format to apply prettier formatting
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@alumni alumni force-pushed the fix/sqlite-flaky-test branch 2 times, most recently from 4e44463 to 6504a72 Compare January 25, 2025 19:49
@coveralls
Copy link

coveralls commented Jan 25, 2025

Coverage Status

coverage: 72.345% (-0.02%) from 72.369%
when pulling 2460100 on alumni:fix/sqlite-flaky-test
into 79960e1 on typeorm:master.

@alumni alumni force-pushed the fix/sqlite-flaky-test branch 2 times, most recently from 51baf3d to aabd20c Compare January 26, 2025 00:16
@OSA413
Copy link
Collaborator

OSA413 commented Jan 27, 2025

i think i can help you with creating a test that can reproduce the error.

@alumni
Copy link
Collaborator Author

alumni commented Jan 27, 2025

The issue I referenced might not be affecting this test.

I think I know the error for the flaky tests (it's the one in the PR description and I can reproduce it easily), it's caused by how we work with savepoints in the test - we can't have parallel savepoints, we can only parallelize the top transaction, not the lower level ones like the test was doing before. And this should be true in general in SQL, regardless of the dialect.

I actually don't know how the test worked for other db drivers - this is giving me a bit of doubt, so some confirmation that what I stated above is true or false would be helpful.

I didn't have time to look further, that's why I didn't update this anymore. The last issue I had after the changes in the test is that I couldn't parallelize those transactions (they go into deadlock for some reason). I'll come back to it later this week, but feel free to suggest anything or even take over if you'd like.

@alumni
Copy link
Collaborator Author

alumni commented Feb 7, 2025

Declining this because I'm reverting the PR that is causing our issues in #11264 and wrote an additional test there.

@alumni alumni closed this Feb 7, 2025
@alumni alumni deleted the fix/sqlite-flaky-test branch May 2, 2025 10:08
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.

3 participants