From af429fa5d820b22f7420dfb078c577c828d77630 Mon Sep 17 00:00:00 2001 From: DM Date: Sat, 23 Nov 2024 23:35:24 -0800 Subject: [PATCH] Use suffixed index names by default to prevent alias creation conflicts Updated index creation logic to use a suffixed index name (e.g., `users_`) by default. Previously, during index reset, the old index (e.g., `users`) was deleted, and a new suffixed index (e.g., `users_`) was created and aliased to the old index name. This caused a race condition, where another job could recreate the unsuffixed `users` index before the alias was applied, leading to a conflict when trying to create the alias. --- CHANGELOG.md | 2 ++ lib/chewy/index.rb | 10 ++++++++++ lib/chewy/index/actions.rb | 7 ++++--- lib/chewy/index/import/routine.rb | 5 ++++- lib/chewy/rake_helper.rb | 6 ++++-- lib/chewy/stash.rb | 15 +++++++++------ spec/chewy/index/actions_spec.rb | 12 ++++++++++++ spec/chewy/index/import_spec.rb | 29 +++++++++++++++++++++++++++-- spec/chewy/index_spec.rb | 10 ++++++++++ spec/chewy/search/request_spec.rb | 11 ++++++----- spec/chewy/stash_spec.rb | 4 ++++ 11 files changed, 92 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7963c22f..4b1c12644 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ ### Changes +* [#973](https://github.com/toptal/chewy/pull/973): Use suffixed index names by default to prevent alias creation conflicts. ([@dmeremyanin][]) + ### Bugs Fixed * [#964](https://github.com/toptal/chewy/pull/964): Fix `delayed_sidekiq` worker to handle UUID primary keys correctly. diff --git a/lib/chewy/index.rb b/lib/chewy/index.rb index 4c63cd8e4..dd6bca733 100644 --- a/lib/chewy/index.rb +++ b/lib/chewy/index.rb @@ -197,6 +197,16 @@ def scopes public_methods - Chewy::Index.public_methods end + # Generates an automatic suffix for index names. By default, the method + # returns a timestamp-based suffix, using the current time in milliseconds (e.g., 1638947285000). + # + # It is used to differentiate indexes for zero-downtime deployments. + # + # @return [Object] an object that can be cast to a string (e.g., integer, timestamp, etc.) + def auto_suffix + (Time.now.to_f * 1000).round + end + def settings_hash _settings.to_hash end diff --git a/lib/chewy/index/actions.rb b/lib/chewy/index/actions.rb index a146f47cc..eee5a565a 100644 --- a/lib/chewy/index/actions.rb +++ b/lib/chewy/index/actions.rb @@ -7,12 +7,13 @@ module Actions extend ActiveSupport::Concern module ClassMethods - # Checks index existance. Returns true or false + # Checks index existance. Supports suffixes. Returns true or false # # UsersIndex.exists? #=> true + # UsersIndex.exists?('11-2024') #=> false # - def exists? - client.indices.exists(index: index_name) + def exists?(suffix = nil) + client.indices.exists(index: index_name(suffix: suffix)) end # Creates index and applies mappings and settings. diff --git a/lib/chewy/index/import/routine.rb b/lib/chewy/index/import/routine.rb index 61004955a..0bed8a362 100644 --- a/lib/chewy/index/import/routine.rb +++ b/lib/chewy/index/import/routine.rb @@ -67,7 +67,10 @@ def create_indexes! Chewy::Stash::Journal.create if @options[:journal] && !Chewy.configuration[:skip_journal_creation_on_import] return if Chewy.configuration[:skip_index_creation_on_import] - @index.create!(**@bulk_options.slice(:suffix)) unless @index.exists? + suffix = @bulk_options[:suffix] || @index.auto_suffix + index_exists = @bulk_options[:suffix] ? @index.exists?(suffix) : @index.exists? + + @index.create!(suffix) unless index_exists end # The main process method. Converts passed objects to the bulk request body, diff --git a/lib/chewy/rake_helper.rb b/lib/chewy/rake_helper.rb index 32de31f6e..bc5212e32 100644 --- a/lib/chewy/rake_helper.rb +++ b/lib/chewy/rake_helper.rb @@ -279,7 +279,7 @@ def create_missing_indexes!(output: $stdout, env: ENV) next end - index.create! + index.create!(index.auto_suffix) output.puts "#{index.name} index successfully created" end @@ -336,7 +336,9 @@ def human_duration(seconds) def reset_one(index, output, parallel: false) output.puts "Resetting #{index}" - index.reset!((Time.now.to_f * 1000).round, parallel: parallel, apply_journal: journal_exists?) + + suffix = index.auto_suffix + index.reset!(suffix, parallel: parallel, apply_journal: journal_exists?) end def warn_missing_index(output) diff --git a/lib/chewy/stash.rb b/lib/chewy/stash.rb index d49957d50..d1e5d09a5 100644 --- a/lib/chewy/stash.rb +++ b/lib/chewy/stash.rb @@ -6,15 +6,20 @@ module Chewy # # @see Chewy::Index::Specification module Stash - class Specification < Chewy::Index - index_name 'chewy_specifications' - + class Index < Chewy::Index default_import_options journal: false + # Disable automatic suffixes for specification and journal indexes + def self.auto_suffix; end + end + + class Specification < Index + index_name 'chewy_specifications' + field :specification, type: 'binary' end - class Journal < Chewy::Index + class Journal < Index index_name 'chewy_journal' # Loads all entries since the specified time. @@ -51,8 +56,6 @@ def self.for(*something) scope end - default_import_options journal: false - field :index_name, type: 'keyword' field :action, type: 'keyword' field :references, type: 'binary' diff --git a/spec/chewy/index/actions_spec.rb b/spec/chewy/index/actions_spec.rb index 27d5682b4..3b52f21cb 100644 --- a/spec/chewy/index/actions_spec.rb +++ b/spec/chewy/index/actions_spec.rb @@ -15,6 +15,18 @@ before { DummiesIndex.create } specify { expect(DummiesIndex.exists?).to eq(true) } end + + context do + before { DummiesIndex.create('2024') } + specify { expect(DummiesIndex.exists?).to eq(true) } + specify { expect(DummiesIndex.exists?('2024')).to eq(true) } + end + + context do + before { DummiesIndex.create('2024', alias: false) } + specify { expect(DummiesIndex.exists?).to eq(false) } + specify { expect(DummiesIndex.exists?('2024')).to eq(true) } + end end describe '.create' do diff --git a/spec/chewy/index/import_spec.rb b/spec/chewy/index/import_spec.rb index b200a4599..ae2f17047 100644 --- a/spec/chewy/index/import_spec.rb +++ b/spec/chewy/index/import_spec.rb @@ -32,13 +32,38 @@ def subscribe_notification describe 'index creation on import' do let(:dummy_city) { City.create } + let(:dummy_auto_suffix) { (Time.now.to_f * 1000).round } + + before do + allow(Chewy::Index).to receive(:auto_suffix).and_return(dummy_auto_suffix) + end specify 'lazy (default)' do - expect(CitiesIndex).to receive(:exists?).and_call_original - expect(CitiesIndex).to receive(:create!).and_call_original + expect(CitiesIndex).to receive(:exists?).with(no_args).and_call_original + expect(CitiesIndex).to receive(:create!).with(dummy_auto_suffix).and_call_original + CitiesIndex.import(dummy_city) + end + + specify 'lazy when index is already created' do + CitiesIndex.create! + expect(CitiesIndex).to receive(:exists?).with(no_args).and_call_original + expect(CitiesIndex).not_to receive(:create!) CitiesIndex.import(dummy_city) end + specify 'lazy when index is already created and suffix is given' do + CitiesIndex.create!(dummy_auto_suffix) + expect(CitiesIndex).to receive(:exists?).with(dummy_auto_suffix).and_call_original + expect(CitiesIndex).not_to receive(:create!) + CitiesIndex.import(dummy_city, suffix: dummy_auto_suffix) + end + + specify 'lazy when suffix is given' do + expect(CitiesIndex).to receive(:exists?).with(dummy_auto_suffix).and_call_original + expect(CitiesIndex).to receive(:create!).with(dummy_auto_suffix).and_call_original + CitiesIndex.import(dummy_city, suffix: dummy_auto_suffix) + end + specify 'lazy without objects' do expect(CitiesIndex).not_to receive(:exists?) expect(CitiesIndex).not_to receive(:create!) diff --git a/spec/chewy/index_spec.rb b/spec/chewy/index_spec.rb index d96d1bf6f..ffe02923c 100644 --- a/spec/chewy/index_spec.rb +++ b/spec/chewy/index_spec.rb @@ -246,6 +246,16 @@ def self.by_id; end specify { expect(subject.specification).to equal(subject.specification) } end + describe '.auto_suffix' do + subject { stub_index(:documents) } + + around do |spec| + Timecop.freeze { spec.run } + end + + specify { expect(subject.auto_suffix).to eq((Time.now.to_f * 1000).round) } + end + context 'index call inside index', :orm do before do stub_index(:cities) do diff --git a/spec/chewy/search/request_spec.rb b/spec/chewy/search/request_spec.rb index b73b99161..7ef29f577 100644 --- a/spec/chewy/search/request_spec.rb +++ b/spec/chewy/search/request_spec.rb @@ -751,12 +751,13 @@ expect(subject.limit(5).source(:name).pluck(:id, :age)).to eq([[1, 10], [2, 20], [3, 30], [4, 40], [5, 50]]) end specify do + index_name = ProductsIndex.indexes[0] expect(subject.limit(5).pluck(:_index, :name)).to eq([ - %w[products Name1], - %w[products Name2], - %w[products Name3], - %w[products Name4], - %w[products Name5] + [index_name, 'Name1'], + [index_name, 'Name2'], + [index_name, 'Name3'], + [index_name, 'Name4'], + [index_name, 'Name5'] ]) end diff --git a/spec/chewy/stash_spec.rb b/spec/chewy/stash_spec.rb index fc0df7487..cc0dfb589 100644 --- a/spec/chewy/stash_spec.rb +++ b/spec/chewy/stash_spec.rb @@ -82,4 +82,8 @@ def fetch_deleted_number(response) expect(described_class.for(CitiesIndex, UsersIndex).map(&:index_name)).to contain_exactly('cities', 'users') end end + + describe '.auto_suffix' do + specify { expect(described_class.auto_suffix).to be_nil } + end end