Reuben Pereira authored 5 years ago and Andreas Brandl committed 5 years ago

- Move code for creating a new column from old into a function so that
it can be reused.
- Also add comments above the methods.

- Also add helper to undo rename_column_concurrently.

- When renaming a column concurrently, drop any existing trigger before
attempting to create a new one.

When running migration specs multiple times (as it happens during
local development), the down method of previous migrations are called.
If any of the called methods contains a call to
rename_column_concurrently, a trigger will be created and not removed.
So, the next time a migration spec is run, if the same down method is
executed again, it will cause an error when attempting to create the
trigger (since it already exists). Dropping the trigger if it already
exists will prevent this problem.

We dropped MySQL support and a lot of mysql specific code has been
removed in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29608.

This comes in from the other direction and removes any `if postgresql?`
branches.

None of this code can be reached any more, so it can all be removed

Alexis Kalderimis authored 5 years ago and Mayra Cabrera committed 5 years ago

This improves the `add_timestamps_with_timezone` helper by allowing the
column names to be configured. This has the advantage that unnecessary
columns can be avoided, saving space.

A helper for removing the columns is also provided, to be used in the
`down` method of migrations.

Suggests to use a JSON structured log instead

Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/54102

So funny story, true story. I tried to run the test locally, but
didn't make it past setting up Gitaly.

Here's what I tried:

First attempt:

`git clone gitlab-ce`
`cd gitlab-ce && bundle install`
`be rspec`

This didn't work because I was missing the config/database.yml, I
didn't see a `script/bootstrap` so I looked in the readme which
redirected me to a webpage which redirected me to the
gitlab-development-kit.

Second attempt:

`gem install gitlab-development-kit`
cd gitlab-development-kit
gdk init
gdk isntall

This broke somwhere along the way because it couldn't install Gitaly
because my go version was too low. But it did clone the gitlab repo
again and this time it did have a config/database.yml.

So I tried to cd into it and `be rspec
spec/lib/gitlab/database/migration_helpers_spec.rb` which complained
about the database not being configured so I:

- Changed the socket to localhost (in the config/database.yml)
- `createdb <dev_db>` `createdb test_db`
- `be rake db:test:prepare`

Great success, it was doing things! But then failed when it came at
the Gitaly step.

Since I only want to change these three lines, at the point I gave up
and entrusted the pipeline to do its thing.

What I would have liked:

- A 'It's a Rails system, I know this' readme/docs (It's in there
  somewhere just couldn't find it)
- A way to run tests without having to use Gitaly
- Not having too install all the things for a small fix (I get why'd
  you want this, but to me it's overkill)

The rather cryptic:

      "fk_#{Digest::SHA256.hexdigest("#{table}_#{column}_fk").first(10)}"

Was too much for emacs too handle*, since it was coming from the Rails
codebase I took their way of doing the same thing and applied it here.

I think it's easier to read and it also makes emacs render the
migration helpers pretty again.

* not true, emacs can handle anything, leave emacs alone!

This is necessary for backporting the EE schema to ensure backported
foreign keys use the same key names.

This was incorrectly set by a bug in:
https://gitlab.com/gitlab-org/gitlab-ce/issues/54924

Also adds a `batch_size` option to `update_column_in_batches`

Patch 1 that migrates issues/merge requests states from integer to string.
On this commit we are only adding the state_id column and syncing it with a backgroud migration.

On Patch 2 the code to use the new integer column will be deployed and the old column will be
removed.

Enables frozens string for the following:

* lib/gitlab/conflict/**/*.rb
* lib/gitlab/cross_project_access/**/*.rb
* lib/gitlab/cycle_analytics/**/*.rb
* lib/gitlab/data_builder/**/*.rb
* lib/gitlab/database/**/*.rb
* lib/gitlab/dependency_linker/**/*.rb
* lib/gitlab/diff/**/*.rb
* lib/gitlab/downtime_check/**/*.rb
* lib/gitlab/email/**/*.rb
* lib/gitlab/etag_caching/**/*.rb

Partially addresses gitlab-org/gitlab-ce#47424.

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

it will decide the method for disable statement_timeout upon
per transaction or per session, based on how it's called.

When calling with a block, block will be executed and it will use
session based statement_timeout, otherwise will default to existing
behavior.

By default statement_timeout will only be enabled during transaction
lifetime, therefore not leaking outside of it.

With `transaction: false` it will set for entire session, but requires
a block to passed. It yields control and cleans up session after block
finishes, also preventing leaking outside of it.

This changes the BackgroundMigration worker so it checks for the health
of the DB before performing a background migration. This in turn allows
us to reduce the minimum interval, without having to worry about blowing
things up if we schedule too many migrations.

In this setup, the BackgroundMigration worker will reschedule jobs as
long as the database is considered to be in an unhealthy state. Once the
database has recovered, the migration can be performed.

To determine if the database is in a healthy state, we look at the
replication lag of any replication slots defined on the primary. If the
lag is deemed to great (100 MB by default) for too many slots, the
migration is rescheduled for a later point in time.

The health checking code is hidden behind a feature flag, allowing us to
disable it if necessary.

This works the same way as change_column_type_using_background_migration, but
for renaming a column. It takes a table, not a relation, to match its concurrent
counterpart.

Also, generalise the cleanup migrations to reduce code duplication.

In Arel 7.0.0 (Arel 7.1.4 is used in Rails 5.0) the `engine` parameter
of `Arel::UpdateManager#initializer` was removed.

This commit makes the gitlab database helpers work both in rails 4 and
rails 5.
Fixes errors like this one:

```
1) Gitlab::Database::MigrationHelpers#update_column_in_batches when running outside of a transaction updates all the rows in a table
    Failure/Error:
      update_arel = Arel::UpdateManager.new(ActiveRecord::Base)
        .table(table)
        .set([[table[column], value]])
        .where(table[:id].gteq(start_id))

    ArgumentError:
      wrong number of arguments (given 1, expected 0)
    # ./lib/gitlab/database/migration_helpers.rb:317:in `new'
    # ./lib/gitlab/database/migration_helpers.rb:317:in `block in update_column_in_batches'
    # ./lib/gitlab/database/migration_helpers.rb:307:in `loop'
    # ./lib/gitlab/database/migration_helpers.rb:307:in `update_column_in_batches'
    # ./spec/lib/gitlab/database/migration_helpers_spec.rb:367:in `block (4 levels) in <top (required)>'
```