rake geo:db:migrate failed on geo testbed secondary

assigned to @to1ne

added Geo in GitLab.com / infrastructure label

Note this is probably not a new issue, I only added this rake geo:db:migrate yesterday so this is the first time it has run in awhile.

To me this looks like Rails is looking at the wrong database for the application_settings table.

Perhaps the Geo initializer needs to come earlier than the Rack::Attack one? Or do we just completely fail to set up the environment correctly in the geo:db:migrate task?

Here we go, https://gitlab.com/gitlab-org/gitlab-ee/blob/master/lib/tasks/geo.rake#L86

This task breaks Rails by modifying the configuration to make the Geo tracking database into the main one.

This is a bug in Geo in GitLab.com / infrastructure so I'll move this issue to the gitlab-ee tracker and we can try to resolve it there.

moved from gitlab-com/infrastructure#2856 (closed)

added bug label

assigned to @to1ne

Rails/AR already provides a method to reuse the database tasks, let try that: https://github.com/rails/rails/blob/5755f57f3e887df93e11c4b3efb8bf21226744cc/activerecord/lib/active_record/tasks/database_tasks.rb#L8-L36

That looks good! Thanks for digging into this @to1ne

Do we need to backport the fix into %10.0 ?

Do we need to backport the fix into %10.0 ?

Well, I was thinking the same. Although I do not know since when it is broken.

At the moment my attempt of using ActiveRecord::Tasks::DatabaseTasks results in mostly duplicating https://github.com/rails/rails/blob/5755f57f3e887df93e11c4b3efb8bf21226744cc/activerecord/lib/active_record/railties/databases.rake. Not sure if there is a better method.

https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2714 got into %10.0 so I think the fix for this should go in as well.

I expect it's broken at the moment and the only reason we're not seeing it among customers is because Rack::Attack is disabled by default.

changed milestone to %10.0

https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2925 is going to give us problems when picking into %10.0 though, I expect :/. Maybe pick that as well - it's informational-only, after all.

mentioned in merge request !3019 (merged)

@nick.thomas @to1ne Should not this bug related to https://gitlab.com/gitlab-org/gitlab-ee/blob/master/config/initializers/geo.rb#L8?

Maybe the proper fix should be something like:

if File.exist?(Rails.root.join('config/database_geo.yml'))
  Rails.application.configure do
    config.geo_database = config_for(:database_geo)
  end
else
  begin
    if Gitlab::Geo.primary?
      Gitlab::Geo.current_node&.update_clone_url!
    end
  rescue => e
    warn "WARNING: Unable to check/update clone_url_prefix for Geo: #{e}"
  end
end

@dbalexandre I don't think so.

The problem is that the geo:db:* tasks currently assume that no models referencing the main database will be used while they're running. In fact, some models (like ApplicationSetting) are used, so we need to point them at the right database.

mentioned in issue #3334

mentioned in commit 1345a461

mentioned in merge request !3052 (merged)

mentioned in commit 8b1f155e

closed via merge request !3019 (merged)

closed via commit 29de35a9

closed via commit 1345a461

rake geo:db:migrate failed on geo testbed secondary

Designs

Child items ...

Activity

Admin message

Admin message

rake geo:db:migrate failed on geo testbed secondary

Activity