Skip to content

Commit

Permalink
Use suffixed index names by default to prevent alias creation conflicts
Browse files Browse the repository at this point in the history
Updated index creation logic to use a suffixed index name (e.g., `users_<timestamp-suffix>`) by default.

Previously, during index reset, the old index (e.g., `users`) was deleted, and a new suffixed index (e.g., `users_<timestamp-suffix>`) 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.
  • Loading branch information
dmeremyanin committed Nov 24, 2024
1 parent 6a2176f commit 4de9519
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 19 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -823,6 +825,7 @@
[@davekaro]: https://github.com/davekaro
[@dck]: https://github.com/dck
[@dm1try]: https://github.com/dm1try
[@dmeremyanin]: https://github.com/dmeremyanin
[@dmitry]: https://github.com/dmitry
[@dnd]: https://github.com/dnd
[@DNNX]: https://github.com/DNNX
Expand Down
10 changes: 10 additions & 0 deletions lib/chewy/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions lib/chewy/index/actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion lib/chewy/index/import/routine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions lib/chewy/rake_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 9 additions & 6 deletions lib/chewy/stash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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'
Expand Down
12 changes: 12 additions & 0 deletions spec/chewy/index/actions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 27 additions & 2 deletions spec/chewy/index/import_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!)
Expand Down
10 changes: 10 additions & 0 deletions spec/chewy/index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions spec/chewy/search/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions spec/chewy/stash_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 4de9519

Please sign in to comment.