Skip to content

Add missing index.mapping.total_fields.limit setting to the target index#89875

Merged
salvatore-campagna merged 5 commits intoelastic:mainfrom
salvatore-campagna:fix/downsampling-fields-limit-setting
Sep 8, 2022
Merged

Add missing index.mapping.total_fields.limit setting to the target index#89875
salvatore-campagna merged 5 commits intoelastic:mainfrom
salvatore-campagna:fix/downsampling-fields-limit-setting

Conversation

@salvatore-campagna
Copy link
Copy Markdown
Contributor

@salvatore-campagna salvatore-campagna commented Sep 7, 2022

When the source index has a non-default value for the total number of fields in the mapping we need to copy that value and apply the same setting to the target index. This is necessary because later we apply the source index mappings to the target index. As a result, if the source index has a non-default limit on the number of fields in the mapping we need that same limit on the target index to avoid errors when applying the mappings.

…et index

When the source index has a non-default value for the total number of fields in
the mapping we need to copy that value and apply the same setting to the target
index.
.put(
MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(),
sourceIndexMetadata.getSettings().get(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey())
)
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 will update tests to check for this setting too.

@elasticsearchmachine elasticsearchmachine added v8.5.0 needs:triage Requires assignment of a team area label labels Sep 7, 2022
@salvatore-campagna salvatore-campagna added :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data >non-issue and removed needs:triage Requires assignment of a team area label labels Sep 7, 2022
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Sep 7, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@salvatore-campagna
Copy link
Copy Markdown
Contributor Author

@elasticsearchmachine test this please

@salvatore-campagna
Copy link
Copy Markdown
Contributor Author

@elasticmachine run elasticsearch-ci/bwc

Copy link
Copy Markdown
Contributor

@csoulios csoulios left a comment

Choose a reason for hiding this comment

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

LGTM!

I only left a comment if we should set the index setting when it's not set in source index.

Comment on lines +561 to +564
.put(
MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(),
sourceIndexMetadata.getSettings().get(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey())
)
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.

Will this work if MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING is null at the source index?
Do we know what happens in this case?

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 can add a test for this maybe?

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.

Or better I can make the value random...

@salvatore-campagna salvatore-campagna self-assigned this Sep 8, 2022
@salvatore-campagna salvatore-campagna merged commit 397df7d into elastic:main Sep 8, 2022
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Sep 9, 2022
* main: (34 commits)
  Make sure ivy repo directory exists before downloading artifacts
  Use 'file://' scheme for local repository URL
  Use DRA artifacts for release build CI jobs
  Log unsuccessful attempts to get credentials from web identity tokens (elastic#88241)
  Script: Write Field API path manipulation (elastic#89889)
  Fetch health info action (elastic#89820)
  Fix memory leak in TransportDeleteExpiredDataAction (elastic#89935)
  [ML] Performance improvements for categorization jobs (elastic#89824)
  [DOCS] Revert changes for ES_JAVA_OPTS (elastic#89931)
  Fix deadlock bug exposed by a test (elastic#89934)
  [Downsampling] Remove `FieldValueFetcher` validator (elastic#89497)
  Fix segment stats in tsdb (elastic#89754)
  Synthetic _source: support dense_vector (elastic#89840)
  REST tests fetching fields with synthetic _source (elastic#89888)
  Do not deserialize back BytesTransportRequest to clone a request in MockTransportService (elastic#89926)
  Add SDK request logging to debug failures of S3BlobStoreRepositoryTests#testRequestStats (elastic#89912)
  Fix SnapshotStatusApisIT.testGetSnapshotsWithSnapshotInProgress (elastic#89925)
  Document synthetic source for text and keyword (elastic#89893)
  Fix CloneSnapshotIT.testRemoveFailedCloneFromCSWithQueuedSnapshotInProgress (elastic#89914)
  Add missing index.mapping.total_fields.limit setting to the target index (elastic#89875)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants