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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# frozen_string_literal: true

require "elastic_graph/admin/cluster_configurator/action_reporter"
require "elastic_graph/admin/index_definition_configurator/mapping_update"
require "elastic_graph/datastore_core/index_config_normalizer"
require "elastic_graph/indexer/hash_differ"
require "elastic_graph/support/hash_util"
Expand All @@ -32,9 +33,8 @@ def initialize(datastore_client, index, env_agnostic_index_config, output)
# and the state of the index in the datastore, does one of the following:
#
# - If the index did not already exist: creates the index with the desired mappings and settings.
# - If the desired mapping has fewer fields than what is in the index: raises an exception,
# because the datastore provides no way to remove fields from a mapping and it would be confusing
# for this method to silently ignore the issue.
# - If the desired mapping has fewer fields than what is in the index: leaves the existing fields
# alone, because the datastore provides no way to remove fields from a mapping.
# - If the settings have desired changes: updates the settings, restoring any setting that
# no longer has a desired value to its default.
# - If the mapping has desired changes: updates the mappings.
Expand Down Expand Up @@ -75,14 +75,9 @@ def create_new_index
end

def update_mapping
@datastore_client.put_index_mapping(index: @index.name, body: desired_mapping)
@datastore_client.put_index_mapping(index: @index.name, body: desired_mapping_for_update)
action_description = "Updated mappings for index `#{@index.name}`:\n#{mapping_diff}"

if mapping_removals.any?
action_description += "\n\nNote: the extra fields listed here will not actually get removed. " \
"Mapping removals are unsupported (but ElasticGraph will leave them alone and they'll cause no problems)."
end

report_action action_description
end

Expand All @@ -100,10 +95,6 @@ def index_exists?
!current_config.empty?
end

def mapping_removals
@mapping_removals ||= mapping_fields_from(current_mapping) - mapping_fields_from(desired_mapping)
end

def mapping_type_changes
@mapping_type_changes ||= begin
flattened_current = Support::HashUtil.flatten_and_stringify_keys(current_mapping)
Expand All @@ -116,7 +107,7 @@ def mapping_type_changes
end

def has_mapping_updates?
current_mapping != desired_mapping
current_mapping != desired_mapping_for_update
end

def settings_updates
Expand All @@ -127,15 +118,8 @@ def settings_updates
end
end

def mapping_fields_from(mapping_hash, prefix = "")
(mapping_hash["properties"] || []).flat_map do |key, params|
field = prefix + key
if params.key?("properties")
[field] + mapping_fields_from(params, "#{field}.")
else
[field]
end
end
def desired_mapping_for_update
@desired_mapping_for_update ||= MappingUpdate.merge_existing_fields_into(desired_mapping, current_mapping)
end

def desired_mapping
Expand Down Expand Up @@ -178,7 +162,7 @@ def current_config
end

def mapping_diff
@mapping_diff ||= Indexer::HashDiffer.diff(current_mapping, desired_mapping) || "(no diff)"
@mapping_diff ||= Indexer::HashDiffer.diff(current_mapping, desired_mapping_for_update) || "(no diff)"
end

def settings_diff
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ def initialize(datastore_client, index_template, env_agnostic_index_config_paren
# and the state of the index in the datastore, does one of the following:
#
# - If the index did not already exist: creates the index with the desired mappings and settings.
# - If the desired mapping has fewer fields than what is in the index: raises an exception,
# because the datastore provides no way to remove fields from a mapping and it would be confusing
# for this method to silently ignore the issue.
# - If the desired mapping has fewer fields than what is in the index template: updates the template
# to drop those fields. Related concrete indices preserve their existing fields because the datastore
# provides no way to remove fields from index mappings.
# - If the settings have desired changes: updates the settings, restoring any setting that
# no longer has a desired value to its default.
# - If the mapping has desired changes: updates the mappings.
Expand Down Expand Up @@ -67,19 +67,13 @@ def validate
private

def put_index_template
desired_template_config_payload = Support::HashUtil.deep_merge(
desired_config_parent,
{"template" => {"mappings" => merge_properties(desired_mapping, current_mapping)}}
)

action_description = "Updated index template: `#{@index_template.name}`:\n#{config_diff}"

if mapping_removals.any?
action_description += "\n\nNote: the extra fields listed here will not actually get removed. " \
"Mapping removals are unsupported (but ElasticGraph will leave them alone and they'll cause no problems)."
action_description = if index_template_exists?
"Updated index template: `#{@index_template.name}`:\n#{config_diff}"
else
"Created index template: `#{@index_template.name}`"
end

@datastore_client.put_index_template(name: @index_template.name, body: desired_template_config_payload)
@datastore_client.put_index_template(name: @index_template.name, body: desired_config_parent)
report_action action_description
end

Expand All @@ -92,10 +86,6 @@ def index_template_exists?
!current_config_parent.empty?
end

def mapping_removals
@mapping_removals ||= mapping_fields_from(current_mapping) - mapping_fields_from(desired_mapping)
end

def mapping_type_changes
@mapping_type_changes ||= begin
flattened_current = Support::HashUtil.flatten_and_stringify_keys(current_mapping)
Expand All @@ -119,17 +109,6 @@ def settings_updates
end
end

def mapping_fields_from(mapping_hash, prefix = "")
(mapping_hash["properties"] || []).flat_map do |key, params|
field = prefix + key
if params.key?("properties")
[field] + mapping_fields_from(params, "#{field}.")
else
[field]
end
end
end

def desired_mapping
desired_config_parent.fetch("template").fetch("mappings")
end
Expand Down Expand Up @@ -185,36 +164,6 @@ def report_action(message)
@reporter.report_action(message)
end

# Helper method used to merge properties between a _desired_ configuration and a _current_ configuration.
# This is used when we are figuring out how to update an index template. We do not want to delete existing
# fields from a template--while the datastore would allow it, our schema evolution strategy depends upon
# us not dropping old unused fields. The datastore doesn't allow it on indices, anyway (though it does allow
# it on index templates). We've ran into trouble (a near SEV) when allowing the logic here to delete an unused
# field from an index template. The indexer "mapping completeness" check started failing because an old version
# of the code (from back when the field in question was still used) noticed the expected field was missing and
# started failing on every event.
#
# This helps us avoid that problem by retaining any currently existing fields.
#
# Long term, if we want to support fully "garbage collecting" these old fields on templates, we will need
# to have them get dropped in a follow up step. We could have our `update_datastore_config` script notice that
# the deployed prod indexers are at a version that will tolerate the fields being dropped, or support it
# via an opt-in flag or something.
def merge_properties(desired_object, current_object)
desired_properties = desired_object.fetch("properties") { _ = {} }
current_properties = current_object.fetch("properties") { _ = {} }

merged_properties = desired_properties.merge(current_properties) do |key, desired, current|
if current.is_a?(::Hash) && current.key?("properties") && desired.key?("properties")
merge_properties(desired, current)
else
desired
end
end

desired_object.merge("properties" => merged_properties)
end

def related_index_configurators
# Here we fan out and get a configurator for each related index. These are generally concrete
# index that are based on a template, either via being specified in our config YAML, or via
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Copyright 2024 - 2026 Block, Inc.
#
# Use of this source code is governed by an MIT-style
# license that can be found in the LICENSE file or at
# https://opensource.org/licenses/MIT.
#
# frozen_string_literal: true

module ElasticGraph
class Admin
module IndexDefinitionConfigurator
module MappingUpdate
empty_properties = {} # : ::Hash[::String, untyped]
EMPTY_PROPERTIES = empty_properties.freeze

# Elasticsearch/OpenSearch do not support removing mapping fields from an index. Preserve current
# fields when building concrete index update payloads and diffs, while still allowing updates to
# existing field parameters and additions of new fields.
def self.merge_existing_fields_into(desired_object, current_object)
desired_properties = desired_object.fetch("properties", EMPTY_PROPERTIES)
current_properties = current_object.fetch("properties", EMPTY_PROPERTIES)

merged_properties = desired_properties.merge(current_properties) do |_key, desired, current|
if current.is_a?(::Hash) && current.key?("properties") && desired.key?("properties")
merge_existing_fields_into(desired, current)
else
desired
end
end

desired_object.merge("properties" => merged_properties)
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ module ElasticGraph
def cannot_modify_mapping_field_type_error: () -> ::String
def index_exists?: () -> bool

@mapping_removals: ::Array[::String]?
def mapping_removals: () -> ::Array[::String]

@mapping_type_changes: ::Array[::String]?
def mapping_type_changes: () -> ::Array[::String]

Expand All @@ -39,7 +36,8 @@ module ElasticGraph
@settings_updates: DatastoreCore::indexSettingsHash?
def settings_updates: () -> DatastoreCore::indexSettingsHash

def mapping_fields_from: (DatastoreCore::indexMappingHash, ?::String) -> ::Array[::String]
@desired_mapping_for_update: DatastoreCore::indexMappingHash?
def desired_mapping_for_update: () -> DatastoreCore::indexMappingHash

def desired_mapping: () -> DatastoreCore::indexMappingHash

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ module ElasticGraph
def cannot_modify_mapping_field_type_error: () -> ::String
def index_template_exists?: () -> bool

@mapping_removals: ::Array[::String]?
def mapping_removals: () -> ::Array[::String]

@mapping_type_changes: ::Array[::String]?
def mapping_type_changes: () -> ::Array[::String]

Expand All @@ -40,8 +37,6 @@ module ElasticGraph
@settings_updates: DatastoreCore::indexSettingsHash?
def settings_updates: () -> DatastoreCore::indexSettingsHash

def mapping_fields_from: (DatastoreCore::indexMappingHash, ?::String) -> ::Array[::String]

def desired_mapping: () -> DatastoreCore::indexMappingHash

@desired_settings: DatastoreCore::indexSettingsHash?
Expand All @@ -62,7 +57,6 @@ module ElasticGraph
def config_diff: () -> ::String

def report_action: (::String) -> void
def merge_properties: (::Hash[::String, untyped], ::Hash[::String, untyped]) -> ::Hash[::String, untyped]

@related_index_configurators: ::Array[ForIndex]?
def related_index_configurators: () -> ::Array[ForIndex]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module ElasticGraph
class Admin
module IndexDefinitionConfigurator
module MappingUpdate
EMPTY_PROPERTIES: ::Hash[::String, untyped]

def self.merge_existing_fields_into: (::Hash[::String, untyped], ::Hash[::String, untyped]) -> ::Hash[::String, untyped]
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,25 @@ module IndexDefinitionConfigurator
}.to make_no_datastore_write_calls("main")
end

it "is a no-op when attempting to drop a mapping field because the datastore does not support it" do
configure_index_definition(schema_def)
output_io.string = +"" # use `+` so it is not a frozen string literal.

expect {
# Here we remove the `name` field and the `options.size` field to verify it works for both root and nested fields.
configure_index_definition(schema_def(
avoid_defining_widget_fields: %w[name],
avoid_defining_widget_options_fields: %w[size]
))
}.to maintain {
props = get_index_definition_configuration(unique_index_name).dig("mappings", "properties")
[props.keys.sort, props.dig("options", "properties").keys.sort]
}.from([[*index_meta_fields, "created_at", "id", "name", "options"], ["color", "size"]])
.and make_no_datastore_write_calls("main")

expect(output_io.string).to exclude("Updated mappings", "properties.name", "properties.options.properties.size")
end

def make_datastore_calls_to_configure_index_def(index_name, subresource = nil)
make_datastore_write_calls("main", "PUT #{put_index_definition_url(index_name, subresource)}")
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,30 @@ def fetch_artifact_configuration(schema_artifacts, index_def_name)
.and make_datastore_calls_to_configure_index_def(unique_index_name, :settings)
end

it "removes mapping fields from the index template while leaving concrete indices unchanged" do
configure_index_definition(schema_def)
output_io.string = +"" # use `+` so it is not a frozen string literal.

expect {
# Here we remove the `name` field and the `options.size` field to verify it works for both root and nested fields.
configure_index_definition(schema_def(
avoid_defining_widget_fields: %w[name],
avoid_defining_widget_options_fields: %w[size]
))
}.to change {
props = get_index_template_definition_configuration(unique_index_name).dig("mappings", "properties")
[props.keys.sort, props.dig("options", "properties").keys.sort]
}.from([[*index_meta_fields, "created_at", "id", "name", "options"], ["color", "size"]])
.to([[*index_meta_fields, "created_at", "id", "options"], ["color"]])
.and maintain {
props = main_datastore_client.get_index(concrete_index_name_for_now(unique_index_name)).dig("mappings", "properties")
[props.keys.sort, props.dig("options", "properties").keys.sort]
}.from([[*index_meta_fields, "created_at", "id", "name", "options"], ["color", "size"]])
.and make_datastore_write_calls("main", "PUT #{put_index_template_definition_url(unique_index_name)}")

expect(output_io.string).to include("Updated index template: `#{unique_index_name}`", "properties.name", "properties.options.properties.size")
end

it "creates concrete indices based on `setting_overrides_by_timestamp` configuration, and avoids creating an extra index for 'now'" do
jan_2020_index_name = unique_index_name + "_rollover__2020-01"

Expand Down
Loading