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
39 changes: 28 additions & 11 deletions core/lib/workarea/elasticsearch/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,40 @@ def exists?
Workarea.elasticsearch.indices.exists?(index: name)
end

# Create the index, optionally deleting it first when +force+ is true.
#
# The previous implementation used an +unless exists?+ guard which
# introduced a TOCTOU (time-of-check/time-of-use) race condition: the
# cluster state visible to +exists?+ could differ from the state seen by
# the subsequent +create+ call, causing intermittent
# +resource_already_exists_exception+ (400) errors in CI and during rapid
# sequential test runs. When +create!+ raised that exception the index
# remained deleted (because +delete!+ had already run), which caused the
# cascade +index_not_found_exception+ (404) seen in later test operations.
#
# The fix removes the guard and instead rescues the 400 BadRequest that
# Elasticsearch raises when the index already exists. This makes the
# operation atomic from the caller's perspective: if the index was already
# present we simply skip creation, just as the old +unless exists?+ path
# intended – but without the window for a concurrent creation to slip in.
#
def create!(force: false)
delete! if force

unless exists?
Workarea.elasticsearch.indices.create(
index: name,
body: {
settings: Search::Settings.current.elasticsearch_settings,
mappings: mappings,
aliases: aliases
}
)
end
Workarea.elasticsearch.indices.create(
index: name,
body: {
settings: Search::Settings.current.elasticsearch_settings,
mappings: mappings,
aliases: aliases
}
)
rescue ::Elasticsearch::Transport::Transport::Errors::BadRequest => e
raise unless e.message.include?('resource_already_exists_exception')
end

def delete!
Workarea.elasticsearch.indices.delete(index: name, ignore: 404)
Workarea.elasticsearch.indices.delete(index: name, ignore: [404])
end

def while_closed
Expand Down
42 changes: 42 additions & 0 deletions core/test/lib/workarea/elasticsearch/document_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,48 @@ def test_scroll
ensure
Foo.clear_scroll(results['_scroll_id'])
end

# Regression: Index#create! must be idempotent.
#
# The old implementation used `unless exists?` before calling
# `indices.create`. This introduced a TOCTOU race: between the `exists?`
# check and the actual `create` call another request could have created
# (or re-created) the same index, causing a
# `resource_already_exists_exception` (HTTP 400). When that exception
# propagated the index was left in a deleted state (because `delete!` had
# already run), which then caused `index_not_found_exception` (404) for
# any search performed by the same test.
#
# The fix removes the guard and rescues the 400 BadRequest instead, making
# the operation atomic.
def test_create_idempotent_when_index_already_exists
index = Foo.current_index

# First call – index exists from setup_indexes; should not raise.
assert_nothing_raised { index.create! }
end

def test_create_force_recreates_index_and_remains_searchable
Foo.save(id: 'before')
Foo.current_index.wait_for_health

# force: true must delete-and-recreate; previously saved documents are gone
Foo.reset_indexes!

assert_equal(0, Foo.count, 'expected empty index after force-recreate')

Foo.save(id: 'after')
assert_equal(1, Foo.count, 'expected new document after force-recreate')
end

def test_create_force_idempotent_called_twice
# Calling reset_indexes! back-to-back must not raise
# resource_already_exists_exception on the second call.
assert_nothing_raised do
Foo.reset_indexes!
Foo.reset_indexes!
end
end
end
end
end
Loading