Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions app/events/resync_event.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# frozen_string_literal: true

class ResyncEvent < BaseEventStoreEvent
def self.create(provider_id:, service_id: nil)
zync = service_id ? { service_id: } : {}

new(
metadata: {
provider_id:,
zync:
}
)
end
end
60 changes: 45 additions & 15 deletions lib/tasks/zync.rake
Original file line number Diff line number Diff line change
@@ -1,44 +1,74 @@
# frozen_string_literal: true

BATCH_SIZE = 100

namespace :zync do
namespace :resync do
def each_with_progress(scope)
def each_with_progress(label, scope)
puts "== Resyncing #{label} =="
total_count = scope.count
batch_size = 100
index = 0

step = [(total_count / 10), 1].max
progress = -> do
break unless (index % batch_size) == 0
break unless (index % step).zero?

percent = (index / total_count.to_f) * 100.0
puts "#{percent.round(2)}% completed"
end

scope.find_each(batch_size: batch_size) do |object|
scope.find_each(batch_size: BATCH_SIZE) do |object|
index += 1
yield object
progress.call
end
end

desc 'Resync provider domains with zync'
task provider_domains: :environment do
def active_providers
accounts = Account.providers_with_master
if (provider_id = ENV["PROVIDER_ID"])
accounts = accounts.where(id: provider_id)
accounts.where(id: provider_id)
else
accounts.without_suspended.without_deleted
end
each_with_progress(accounts) { |account| Domains::ProviderDomainsChangedEvent.create_and_publish!(account) }
end

def publish_zync_event(model, provider_id:, service_id: nil)
event = ResyncEvent.create(provider_id:, service_id:)
ZyncEvent.create_and_publish!(event, model)
end

desc 'Resync provider domains with zync'
task providers: :environment do
each_with_progress('providers', active_providers) { |account| publish_zync_event(account, provider_id: account.id) }
end

task provider_domains: :providers

desc 'Resync services with zync'
task services: :environment do
services = Service.joins(:account).merge(active_providers)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, so in the past we didn't filter out inactive providers for this task? Wasteful in some environments.

Previously we had .includes(:proxy), does it help avoid individual queries? Idk if you checked how many queries actually run. Also I'm not sure whether .includes(:account) will also help because likely we will query the account of the service anyway. But maybe joins already includes the data for the account. I haven't checked.

Suggested change
services = Service.joins(:account).merge(active_providers)
services = Service.includes(:account).joins(:account).merge(active_providers)

btw if we loop over services, because that query already has the provider accounts, that may really reduce the SQL loop if we have a custom action like before, but I don't ask you to make a custom action 👼 just a FYI, don't think the marginal benefit is worth the additional code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, this is a new task. Bob said:

The zync:resync:services task is redundant. The old proxy_domains resync │
│ already synced services through the dependency mechanism in ZyncWorker.

So we can have it if one needs to run independently but maybe redundant to run with the others? Idk, I was just asking 😬

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware of this dependency mechanism. I think we don't need to touch anything because it's not really redundant, it depends on the service existing or not. The mechanism is only triggered when zync returns 422, that wouldn't happen when running our :full task because it runs :services before :proxies.

If we run :proxies directly, then yes, it will sync services, but only after each failed attempt to sync a proxy, so that implies a couple of extra round trips which is suboptimal. I don't think we should remove the :services task, since it's the way to avoid the suboptimal path.

It also works for applications, that is, if you try to sync an application, it will sync the service and proxy first if they don't exist. But again, suboptimal.

def dependencies
return non_persisted_dependencies unless record.persisted?
case record
when Cinstance
[ service = record.service, service.proxy ]
when Proxy
[ record.service ]
when Service
NONE
else
NONE
end
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, but you split out proxies I see now 🤔 ok, we go to the other extreme :) alright.

each_with_progress('services', services) { |service| publish_zync_event(service, provider_id: service.account_id) }
end

desc 'Resync proxy domains with zync'
task proxy_domains: :environment do
services = Service.includes(:proxy)
if (provider_id = ENV["PROVIDER_ID"])
services = services.where(account_id: provider_id)
end
each_with_progress(services) { |service| Domains::ProxyDomainsChangedEvent.create_and_publish!(service.proxy) }
task proxies: :environment do
proxies = Proxy.eager_load(service: :account).merge(active_providers)
each_with_progress('proxies', proxies) { |proxy| publish_zync_event(proxy, provider_id: proxy.service.account_id, service_id: proxy.service_id) }
end

task proxy_domains: :proxies

desc 'Resync applications with zync'
task applications: :environment do
cinstances = Cinstance.eager_load(service: :account).merge(active_providers)
each_with_progress('applications', cinstances) { |cinstance| publish_zync_event(cinstance, provider_id: cinstance.service.account_id, service_id: cinstance.service_id) }
end

desc 'Resync all domains with zync'
task domains: [:provider_domains, :proxy_domains]
task domains: %i[provider_domains proxy_domains]

desc 'Full resync'
task full: %i[providers services proxies applications]
end
end
88 changes: 82 additions & 6 deletions test/unit/tasks/zync_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,90 @@ class ZyncTest < ActiveSupport::TestCase
FactoryBot.create(:provider_account)
end

test 'resync provider domains' do
Account.providers_with_master.each { |account| Domains::ProviderDomainsChangedEvent.expects(:create_and_publish!).with(account) }
execute_rake_task 'zync.rake', 'zync:resync:provider_domains'
class DomainsSyncTest < ZyncTest
test 'resync provider domains' do
active_providers.each { |account| ZyncEvent.expects(:create_and_publish!).with(instance_of(ResyncEvent), account) }
execute_rake_task 'zync.rake', 'zync:resync:provider_domains'
end

test 'resync proxy domains' do
active_proxies.each { |proxy| ZyncEvent.expects(:create_and_publish!).with(instance_of(ResyncEvent), proxy) }
execute_rake_task 'zync.rake', 'zync:resync:proxy_domains'
end

private

def active_providers
Account.providers_with_master.without_suspended.without_deleted
end

def active_proxies
Proxy.eager_load(service: :account).merge(active_providers)
end
end

test 'resync proxy domains' do
Service.all.each { |service| Domains::ProxyDomainsChangedEvent.expects(:create_and_publish!).with(service.proxy) }
execute_rake_task 'zync.rake', 'zync:resync:proxy_domains'
class FullSyncTest < ZyncTest
setup do
@providers = FactoryBot.create_list(:provider_account, 3)

@providers.each do |provider|
services = FactoryBot.create_list(:service, 2, account: provider)
services.each do |service|
plan = FactoryBot.create(:application_plan, issuer: service)
FactoryBot.create_list(:cinstance, 3, plan: plan, service: service)
end
end
end

test 'full resync' do
load_collections
expect_resync_events
execute_rake_task 'zync.rake', 'zync:resync:full'
end

test 'full resync excludes suspended providers' do
@providers.first.suspend!
load_collections
expect_resync_events
execute_rake_task 'zync.rake', 'zync:resync:full'
end

test 'full resync excludes scheduled for deletion providers' do
@providers.first.schedule_for_deletion!
load_collections
expect_resync_events
execute_rake_task 'zync.rake', 'zync:resync:full'
end

test 'full resync with PROVIDER_ID' do
ENV['PROVIDER_ID'] = @providers.first.id.to_s
load_collections
expect_resync_events
execute_rake_task 'zync.rake', 'zync:resync:full'
ensure
ENV.delete('PROVIDER_ID')
end

private

def load_collections
active = Account.providers_with_master
if (provider_id = ENV['PROVIDER_ID'])
active = active.where(id: provider_id)
else
active = active.without_suspended.without_deleted
end
@all_accounts = active.to_a
@all_services = Service.joins(:account).merge(active).to_a
@all_proxies = Proxy.eager_load(service: :account).merge(active).to_a
@all_cinstances = Cinstance.eager_load(service: :account).merge(active).to_a
end

def expect_resync_events
(@all_accounts + @all_services + @all_proxies + @all_cinstances).each do |model|
ZyncEvent.expects(:create_and_publish!).with(instance_of(ResyncEvent), model)
end
end
end
end
end