Ericsson / codechecker

CodeChecker is an analyzer tooling, defect database and viewer extension for the Clang Static Analyzer and Clang Tidy
https://codechecker.readthedocs.io
Apache License 2.0
2.15k stars 357 forks source link

[db][server] Failing `db_cleanup` when started together with a schema migration should not result in the migration's `ROLLBACK` #4232

Open whisperity opened 2 months ago

whisperity commented 2 months ago

Describe the bug If a CodeChecker server is started with schema migration and database clean-up at the same time (not providing --skip-db-cleanup and answering y to all questions about migration), and the cleanup of a product fails, then the migration is also rolled back.

CodeChecker version 6.23.0, 6.24.0-rc1

To Reproduce Steps to reproduce the behaviour:

  1. Patch the CodeChecker package such that some db_cleanup routine randomly (or deterministically based on product name) fails by throwing an Exception.
  2. Have a schema migration built into the package. An empty script is alright, we just need a different value to be added to alembic_version in the database.
  3. Have at least TWO products, one's cleanup should not fail.
  4. Start the server, answer y to the migration queries.
  5. See in the logs that the server upgraded the schema.
  6. db_cleanup starts automatically after schema migration
  7. db_cleanup should "crash".
  8. SELECT * FROM alembic_version; on both products. One of them (the one where db_cleanup did not crash) will have the new schema ID, while the other will have the old, showing that a failing db cleanup effectively ROLLBACKed the migration as well.

Expected behaviour A successful migration should be COMMITted before any cleanup operation is done. This would be especially important if the migration affected rows that the cleanup is touching, because PostgreSQL in certain configurations could reject and error out from a COMMIT if there were ALTER TABLE statements together with data modification.