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

Replace SQLite with PostgreSQL for server database to prevent concurrency issues #190

Closed
33 tasks done
lanedirt opened this issue Aug 30, 2024 · 0 comments · Fixed by #492
Closed
33 tasks done

Replace SQLite with PostgreSQL for server database to prevent concurrency issues #190

lanedirt opened this issue Aug 30, 2024 · 0 comments · Fixed by #492
Labels
⚡️ enhancement New feature or request

Comments

@lanedirt
Copy link
Owner

lanedirt commented Aug 30, 2024

The current SQLite implementation doesn't play well when concurrent requests are received by webapi. This can be tested by making the security settings page in client do parallel/concurrent requests.

Switching to a proper database provider should hopefully fix this.

Todo:

  • Add docker container for PostgreSQL setup.
  • Add db engine specific migrations: keep current for sqlite and create new migration set for postgresql with initial migration. Follow instructions in https://learn.microsoft.com/en-us/ef/core/managing-schemas/migrations/providers?tabs=dotnet-core-cli.
  • Make admin project compile via new dynamic dbcontext factory for sqlite.
  • Note: a lot of fields in the current migrations are set to "TEXT" type because of limitations in SQLite... having text types all over the place is not the most optimal database structure, especially for other database engines. Research what to do about this in the scenario of supporting multiple db engines?
  • Make all projects compile with new db structure.
  • Review postgresql migrations and ensure correct data types are used.
  • Update developer docs for creating new migrations so migrations are always created for both db engines. Create migrations for both engines to make both up-to-date.
  • Update E2E tests to use PostgreSQL database connections
  • If possible and feasible, perhaps it's a good idea to keep SQLite as default for local testing and not having to setup the database server for simple setups. Perhaps we can add PostgreSQL as an optional option in install.sh where user can choose the dbprovider they want? Determine what will be the best way strategy going forward taking into account: self-hosting minimum memory required, stability, EF core migrations with SQLite limitations (e.g. TEXT fields vs. specific more optimized field types in postgresql). Being able to switch between SQLite and PostgreSQL... is this feasible at all to support both db engines? It's feasible... but what we see until now is that SQLite is not entirely stable for AliasVault's purposes, not even for local development. Check if we can make E2E tests work using PostgreSQL docker container?
  • Make db engine switch work for all projects via appsettings.json to postgresql.
  • Add option to switch between SQLite and PostgreSQL in appsettings for all projects.
  • Refactor admin tests to make them pass
  • Refactor IntegrationTests to make them pass
  • Add postgresql password generation to install.sh depending if sqlite or postgresql is used by default.
  • Add install.sh command or update docs for only starting postgresql container focused on local development only with static password?
  • Fix admin http 400 errors
  • Test clean install with new branch to ensure all services properly start with no errors or warnings in the logs
  • Refactor WebApplicationApiFactoryFixture so it works correct in creating a new temp db and in exposing the temp db to the tests.
  • Refactor WebApplicationAdminFactoryFixture
  • Update GitHub Action workflow tests so they all run with Postgresql docker container up eliminating need for Sqlite
  • Add instructions / helper tool for migrating data from Sqlite to Postgresql, this is only required once from updating from < 0.9.4 to 0.10.0.
  • Update migration script so it can be called from Docker and all required params are passed in correctly
  • Test with clean install with existing SQLite database that migration works correctly.
  • Run all tests again to ensure everything works with new PostgreSQL datetime col settings
  • Run E2E tests in interactive mode and check for client side errors that appear but do not seem to interfere with primary test path
  • Refactor previous bug to verify if issues are fixed: by making the security settings page in client do parallel/concurrent requests.
  • DbSyncClientMergeCredentialPropertiesTest is flaky, fix it (Shard2/DbSync)
  • Test admin password after migrate
  • Check admin redirect bug to client absolute/relative issue
  • Duplicate key violates PK_AuthLog unique constraint.
  • Write docs for how to upgrade from 0.9.x to 0.10.0.
  • Test upgrade on staging on copy folder.
  • Test upgrade on Azure VM with enough specs for build and test email SMTP
@lanedirt lanedirt converted this from a draft issue Aug 30, 2024
@lanedirt lanedirt added the ⚡️ enhancement New feature or request label Aug 30, 2024
@lanedirt lanedirt added this to the Usability / UX polishing milestone Aug 30, 2024
@lanedirt lanedirt removed this from the Usability / UX polishing milestone Oct 6, 2024
@lanedirt lanedirt changed the title Add PostgreSQL as database support to prevent concurrency locking Add PostgreSQL as database option to prevent concurrency locking due to SQLite limitations Nov 27, 2024
@lanedirt lanedirt moved this from Ready to In progress in AliasVault Dec 21, 2024
lanedirt added a commit that referenced this issue Dec 21, 2024
lanedirt added a commit that referenced this issue Dec 21, 2024
lanedirt added a commit that referenced this issue Dec 23, 2024
lanedirt added a commit that referenced this issue Dec 23, 2024
lanedirt added a commit that referenced this issue Dec 23, 2024
lanedirt added a commit that referenced this issue Dec 23, 2024
lanedirt added a commit that referenced this issue Dec 25, 2024
lanedirt added a commit that referenced this issue Dec 25, 2024
lanedirt added a commit that referenced this issue Dec 25, 2024
lanedirt added a commit that referenced this issue Dec 25, 2024
lanedirt added a commit that referenced this issue Dec 25, 2024
lanedirt added a commit that referenced this issue Dec 25, 2024
lanedirt added a commit that referenced this issue Dec 25, 2024
lanedirt added a commit that referenced this issue Dec 26, 2024
lanedirt added a commit that referenced this issue Dec 26, 2024
lanedirt added a commit that referenced this issue Dec 28, 2024
lanedirt added a commit that referenced this issue Dec 28, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in AliasVault Dec 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡️ enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant