Skip to content

Rescue low-level errors during migration testing

Simon Tomlinson requested to merge db-testing-catch-system-exceptions into master

What does this MR do and why?

Describe in detail what your merge request does and why.

Resolves gitlab-org/database-team/gitlab-com-database-testing#48 (closed).

During migration testing, if a migration to be tested threw an error not caught by StandardError (such as a stack overflow or a syntax error), we did not run cleanup for our observer code before re-raising the error. The observers then wrote truncated json files, and the notifier code that reads them would throw an error.

Fix by catching all types of errors, running finalization code, then re-throwing.

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

~/Documents/programming/gitlab-development-kit/gitlab db-testing-catch-system-exceptions ≡
❯ rm -r tmp/migration-testing/

~/Documents/programming/gitlab-development-kit/gitlab db-testing-catch-system-exceptions ≡
❯ bundle exec rake gitlab:db:migration_testing:up
WARNING: This version of GitLab depends on gitlab-shell 13.23.0, but you're running 13.22.1. Please update gitlab-shell.
rake aborted!
SyntaxError: /Users/simon/Documents/programming/gitlab-development-kit/gitlab/db/migrate/20220128164503_throws_bad_error.rb:8: syntax error, unexpected '='
    = # syntax error
    ^
/Users/simon/Documents/programming/gitlab-development-kit/gitlab/lib/gitlab/database/migrations/lock_retry_mixin.rb:25:in `ddl_transaction'
/Users/simon/Documents/programming/gitlab-development-kit/gitlab/lib/gitlab/database/migrations/runner.rb:73:in `block (2 levels) in run'
/Users/simon/Documents/programming/gitlab-development-kit/gitlab/lib/gitlab/database/migrations/instrumentation.rb:28:in `block in observe'
/Users/simon/Documents/programming/gitlab-development-kit/gitlab/lib/gitlab/database/migrations/instrumentation.rb:27:in `observe'
/Users/simon/Documents/programming/gitlab-development-kit/gitlab/lib/gitlab/database/migrations/runner.rb:72:in `block in run'
/Users/simon/Documents/programming/gitlab-development-kit/gitlab/lib/gitlab/database/migrations/runner.rb:71:in `each'
/Users/simon/Documents/programming/gitlab-development-kit/gitlab/lib/gitlab/database/migrations/runner.rb:71:in `run'
/Users/simon/Documents/programming/gitlab-development-kit/gitlab/lib/tasks/gitlab/db.rake:247:in `block (4 levels) in <main>'
/Users/simon/.asdf/installs/ruby/2.7.5/bin/bundle:23:in `load'
/Users/simon/.asdf/installs/ruby/2.7.5/bin/bundle:23:in `<main>'
Tasks: TOP => gitlab:db:migration_testing:up
(See full trace by running task with --trace)

~/Documents/programming/gitlab-development-kit/gitlab db-testing-catch-system-exceptions ≡ 13s
❯ jq . tmp/migration-testing/up/*ThrowsBadError-query-details.json > /dev/null && echo 'json was valid'
json was valid

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. Create a local migration that would throw a low-level error, such as a stack overflow or syntax error. I used
class ThrowsBadError < Gitlab::Database::Migration[1.0]
  def change
    = # syntax error
  end
end
  1. Clear previous migration testing output if you have any: rm -r tmp/migration-testing
  2. Run migration testing locally: bundle exec rake gitlab:db:migration_testing:up. You should see your syntax error.
  3. Check that the json file created is valid, for example with jq: jq . tmp/migration-testing/up/*ThrowsBadError-query-details.json > /dev/null && echo 'json was valid' should print json was valid

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Simon Tomlinson

Merge request reports