Skip to content

[8.19] Backport data stream settings #129213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Jun 17, 2025

Conversation

joegallo
Copy link
Contributor

This is a single PR to backport the whole feature all at once. That is, it's backporting #126947, #127282, #127417, #127515, #127858, #128269, #128375, #128535, and #128594.

Warning

I did not do the transport version dance, the version that I introduced on this branch is bogus and can't be used as is.

@joegallo joegallo added WIP backport :Data Management/Data streams Data streams and their lifecycles Team:Data Management Meta label for data/management team v8.19.0 labels Jun 10, 2025
@joegallo

This comment was marked as resolved.

@lukewhiting lukewhiting requested a review from Copilot June 11, 2025 09:28
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR backports the entire data streams settings and mappings feature to the 8.19 branch, introducing new APIs and ensuring compatibility in tests.

  • Adds GET/PUT _data_stream/{name}/_settings endpoints and their transport implementations to view and update data stream settings.
  • Integrates settings support into existing data stream actions (e.g., GetDataStreamAction) and bumps the transport version.
  • Covers the new feature with unit and integration tests.

Reviewed Changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
server/src/main/java/org/elasticsearch/action/datastreams/UpdateDataStreamSettingsAction.java New action/request/response classes for updating data stream settings
modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/TransportUpdateDataStreamSettingsAction.java Master-node logic to apply settings to data streams and backing indices
modules/data-streams/src/main/java/org/elasticsearch/datastreams/rest/RestUpdateDataStreamSettingsAction.java / RestGetDataStreamSettingsAction.java REST handlers for the new settings API
modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamsPlugin.java Registers the new actions and REST handlers
modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamSettingsIT.java Integration tests for the settings API
server/src/main/java/org/elasticsearch/TransportVersions.java Adds a new transport version constant for this feature

UpdateDataStreamSettingsAction.Request,
UpdateDataStreamSettingsAction.Response> {
private static final Logger logger = LogManager.getLogger(TransportUpdateDataStreamSettingsAction.class);
private static final Set<String> APPLY_TO_BACKING_INDICES = Set.of("index.lifecycle.name", IndexSettings.PREFER_ILM);
Copy link
Preview

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

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

APPLY_TO_BACKING_INDICES mixes a String with a Setting instance. Use IndexSettings.PREFER_ILM.getKey() so that settingName comparisons work correctly.

Suggested change
private static final Set<String> APPLY_TO_BACKING_INDICES = Set.of("index.lifecycle.name", IndexSettings.PREFER_ILM);
private static final Set<String> APPLY_TO_BACKING_INDICES = Set.of("index.lifecycle.name", IndexSettings.PREFER_ILM.getKey());

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know what Copilot is talking about -- Index>settings.PREFER_ILM is a string.

@@ -238,6 +238,8 @@ static TransportVersion def(int id) {
public static final TransportVersion ESQL_QUERY_PLANNING_DURATION_8_19 = def(8_841_0_45);
public static final TransportVersion SEARCH_SOURCE_EXCLUDE_VECTORS_PARAM_8_19 = def(8_841_0_46);
public static final TransportVersion ML_INFERENCE_MISTRAL_CHAT_COMPLETION_ADDED_8_19 = def(8_841_0_47);
public static final TransportVersion SETTINGS_IN_DATA_STREAMS_8_19 = def(8_841_0_48); // TODO NO NO NO NO NO
Copy link
Preview

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove or refine the placeholder comment 'TODO NO NO NO NO NO' to maintain professional code quality and clarity.

Suggested change
public static final TransportVersion SETTINGS_IN_DATA_STREAMS_8_19 = def(8_841_0_48); // TODO NO NO NO NO NO
public static final TransportVersion SETTINGS_IN_DATA_STREAMS_8_19 = def(8_841_0_48); // TODO: Verify the implementation and ensure compatibility with data stream settings.

Copilot uses AI. Check for mistakes.

.getDataStreamSettingsResponses();
assertThat(dataStreamSettingsResponses.size(), equalTo(testDataStreamNames.size()));
for (int i = 0; i < testDataStreamNames.size(); i++) {
UpdateDataStreamSettingsAction.DataStreamSettingsResponse dataStreamSettingsResponse = dataStreamSettingsResponses.get(0);
Copy link
Preview

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

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

Inside the loop over testDataStreamNames, you always fetch index 0. It should be dataStreamSettingsResponses.get(i) to validate each response correctly.

Suggested change
UpdateDataStreamSettingsAction.DataStreamSettingsResponse dataStreamSettingsResponse = dataStreamSettingsResponses.get(0);
UpdateDataStreamSettingsAction.DataStreamSettingsResponse dataStreamSettingsResponse = dataStreamSettingsResponses.get(i);

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, I think it's right.

@joegallo joegallo force-pushed the datastreams-settings-and-mappings-overrides-8.19 branch from a2d0422 to 8708721 Compare June 11, 2025 14:00
@joegallo
Copy link
Contributor Author

b3c559e could be its own PR targeting 8.19. It makes it so that the splash damage from a single test failure (in that yaml file) doesn't result in like 60+ spurious test failures. The remaining docs test failures are real, and I think they're pretty trivial to fix.

@joegallo joegallo marked this pull request as ready for review June 12, 2025 15:07
@masseyke masseyke changed the title [8.19] Backport datastreams settings and mappings [8.19] Backport data stream settings Jun 16, 2025
@masseyke masseyke merged commit 5b51ddc into 8.19 Jun 17, 2025
23 checks passed
@masseyke masseyke deleted the datastreams-settings-and-mappings-overrides-8.19 branch June 17, 2025 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport :Data Management/Data streams Data streams and their lifecycles Team:Data Management Meta label for data/management team v8.19.0 WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants