Skip to content

Commit

Permalink
Update IndexSettingProvider#getAdditionalIndexSettings() signature (#…
Browse files Browse the repository at this point in the history
…114150)

With logsdb another index mode is available, the isTimeSeries parameter is limiting. Instead, we should just push down the index mode from template to index settings provider.

Follow up from #113451
Relates to #113583
  • Loading branch information
martijnvg authored and davidkyle committed Oct 15, 2024
1 parent 9159abb commit cb116ef
Show file tree
Hide file tree
Showing 15 changed files with 138 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public class DataStreamIndexSettingsProvider implements IndexSettingProvider {
public Settings getAdditionalIndexSettings(
String indexName,
@Nullable String dataStreamName,
boolean isTimeSeries,
@Nullable IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings indexTemplateAndCreateRequestSettings,
Expand All @@ -70,15 +70,16 @@ public Settings getAdditionalIndexSettings(
// First backing index is created and then data stream is rolled over (in a single cluster state update).
// So at this point we can't check index_mode==time_series,
// so checking that index_mode==null|standard and templateIndexMode == TIME_SERIES
boolean isMigratingToTimeSeries = templateIndexMode == IndexMode.TIME_SERIES;
boolean migrating = dataStream != null
&& (dataStream.getIndexMode() == null || dataStream.getIndexMode() == IndexMode.STANDARD)
&& isTimeSeries;
&& isMigratingToTimeSeries;
IndexMode indexMode;
if (migrating) {
indexMode = IndexMode.TIME_SERIES;
} else if (dataStream != null) {
indexMode = isTimeSeries ? dataStream.getIndexMode() : null;
} else if (isTimeSeries) {
indexMode = isMigratingToTimeSeries ? dataStream.getIndexMode() : null;
} else if (isMigratingToTimeSeries) {
indexMode = IndexMode.TIME_SERIES;
} else {
indexMode = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void testGetAdditionalIndexSettings() throws Exception {
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand Down Expand Up @@ -123,7 +123,7 @@ public void testGetAdditionalIndexSettingsIndexRoutingPathAlreadyDefined() throw
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand Down Expand Up @@ -193,7 +193,7 @@ public void testGetAdditionalIndexSettingsMappingsMerging() throws Exception {
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand All @@ -218,7 +218,7 @@ public void testGetAdditionalIndexSettingsNoMappings() {
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand All @@ -243,7 +243,7 @@ public void testGetAdditionalIndexSettingsLookAheadTime() throws Exception {
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand All @@ -268,7 +268,7 @@ public void testGetAdditionalIndexSettingsLookBackTime() throws Exception {
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand Down Expand Up @@ -299,7 +299,7 @@ public void testGetAdditionalIndexSettingsDataStreamAlreadyCreated() throws Exce
var result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand Down Expand Up @@ -336,7 +336,7 @@ public void testGetAdditionalIndexSettingsDataStreamAlreadyCreatedTimeSettingsMi
() -> provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand All @@ -362,7 +362,7 @@ public void testGetAdditionalIndexSettingsNonTsdbTemplate() {
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
false,
null,
metadata,
Instant.ofEpochMilli(1L),
settings,
Expand All @@ -382,7 +382,7 @@ public void testGetAdditionalIndexSettingsMigrateToTsdb() {
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 2),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand Down Expand Up @@ -415,7 +415,7 @@ public void testGetAdditionalIndexSettingsDowngradeFromTsdb() {
Settings result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 2),
dataStreamName,
false,
null,
metadata,
Instant.ofEpochMilli(1L),
settings,
Expand Down Expand Up @@ -694,7 +694,7 @@ private Settings generateTsdbSettings(String mapping, Instant now) throws IOExce
var result = provider.getAdditionalIndexSettings(
DataStream.getDefaultBackingIndexName(dataStreamName, 1),
dataStreamName,
true,
IndexMode.TIME_SERIES,
metadata,
now,
settings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ public static Template resolveTemplate(
Settings result = provider.getAdditionalIndexSettings(
indexName,
template.getDataStreamTemplate() != null ? indexName : null,
template.getDataStreamTemplate() != null && metadata.isTimeSeriesTemplate(template),
metadata.retrieveIndexModeFromTemplate(template),
simulatedState.getMetadata(),
now,
templateSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1316,23 +1316,6 @@ public Map<String, ComposableIndexTemplate> templatesV2() {
.orElse(Collections.emptyMap());
}

// TODO: remove this method:
public boolean isTimeSeriesTemplate(ComposableIndexTemplate indexTemplate) {
var indexModeFromTemplate = retrieveIndexModeFromTemplate(indexTemplate);
if (indexModeFromTemplate == IndexMode.TIME_SERIES) {
// No need to check for the existence of index.routing_path here, because index.mode=time_series can't be specified without it.
// Setting validation takes care of this.
// Also no need to validate that the fields defined in index.routing_path are keyword fields with time_series_dimension
// attribute enabled. This is validated elsewhere (DocumentMapper).
return true;
}

// in a followup change: check the existence of keyword fields of type keyword and time_series_dimension attribute enabled in
// the template. In this case the index.routing_path setting can be generated from the mapping.

return false;
}

public IndexMode retrieveIndexModeFromTemplate(ComposableIndexTemplate indexTemplate) {
if (indexTemplate.getDataStreamTemplate() == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -982,10 +982,10 @@ static Settings aggregateIndexSettings(
if (sourceMetadata == null) {
final Settings templateAndRequestSettings = Settings.builder().put(combinedTemplateSettings).put(request.settings()).build();

final boolean timeSeriesTemplate = Optional.of(request)
final IndexMode templateIndexMode = Optional.of(request)
.map(CreateIndexClusterStateUpdateRequest::matchingTemplate)
.map(metadata::isTimeSeriesTemplate)
.orElse(false);
.map(metadata::retrieveIndexModeFromTemplate)
.orElse(null);

// Loop through all the explicit index setting providers, adding them to the
// additionalIndexSettings map
Expand All @@ -995,7 +995,7 @@ static Settings aggregateIndexSettings(
var newAdditionalSettings = provider.getAdditionalIndexSettings(
request.index(),
request.dataStreamName(),
timeSeriesTemplate,
templateIndexMode,
currentState.getMetadata(),
resolvedAt,
templateAndRequestSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ private void validateIndexTemplateV2(String name, ComposableIndexTemplate indexT
var newAdditionalSettings = provider.getAdditionalIndexSettings(
"validate-index-name",
indexTemplate.getDataStreamTemplate() != null ? "validate-data-stream-name" : null,
indexTemplate.getDataStreamTemplate() != null && metadata.isTimeSeriesTemplate(indexTemplate),
metadata.retrieveIndexModeFromTemplate(indexTemplate),
currentState.getMetadata(),
now,
combinedSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.IndexMode;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.IndexSettingProvider;
import org.elasticsearch.snapshots.SearchableSnapshotsSettings;
Expand Down Expand Up @@ -226,7 +227,7 @@ public static class DefaultHotAllocationSettingProvider implements IndexSettingP
public Settings getAdditionalIndexSettings(
String indexName,
@Nullable String dataStreamName,
boolean isTimeSeries,
IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings indexTemplateAndCreateRequestSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,21 @@ public interface IndexSettingProvider {
* Returns explicitly set default index {@link Settings} for the given index. This should not
* return null.
*
* @param indexName The name of the new index being created
* @param dataStreamName The name of the data stream if the index being created is part of a data stream otherwise
* <code>null</code>
* @param isTimeSeries Whether the template is in time series mode.
* @param metadata The current metadata instance that doesn't yet contain the index to be created
* @param resolvedAt The time the request to create this new index was accepted.
* @param indexTemplateAndCreateRequestSettings All the settings resolved from the template that matches and any settings
* defined on the create index request
* @param combinedTemplateMappings All the mappings resolved from the template that matches
* @param indexName The name of the new index being created
* @param dataStreamName The name of the data stream if the index being created is part of a data stream
* otherwise <code>null</code>
* @param templateIndexMode The index mode defined in template if template creates data streams,
* otherwise <code>null</code> is returned.
* @param metadata The current metadata instance that doesn't yet contain the index to be created
* @param resolvedAt The time the request to create this new index was accepted.
* @param indexTemplateAndCreateRequestSettings All the settings resolved from the template that matches and any settings
* defined on the create index request
* @param combinedTemplateMappings All the mappings resolved from the template that matches
*/
Settings getAdditionalIndexSettings(
String indexName,
@Nullable String dataStreamName,
boolean isTimeSeries,
@Nullable IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings indexTemplateAndCreateRequestSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.cluster.metadata.Template;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexMode;
import org.elasticsearch.index.IndexSettingProvider;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.indices.SystemIndices;
Expand Down Expand Up @@ -69,7 +70,7 @@ public void testSettingsProviderIsOverridden() throws Exception {
public Settings getAdditionalIndexSettings(
String indexName,
String dataStreamName,
boolean timeSeries,
IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings allSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexMode;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.IndexVersion;
Expand Down Expand Up @@ -2412,30 +2413,87 @@ public void testEnsureMetadataFieldCheckedForGlobalStateChanges() {
assertThat(unclassifiedFields, empty());
}

public void testIsTimeSeriesTemplate() throws IOException {
var template = new Template(Settings.builder().put("index.mode", "time_series").build(), new CompressedXContent("{}"), null);
public void testRetrieveIndexModeFromTemplateTsdb() throws IOException {
// tsdb:
var tsdbTemplate = new Template(Settings.builder().put("index.mode", "time_series").build(), new CompressedXContent("{}"), null);
// Settings in component template:
{
var componentTemplate = new ComponentTemplate(template, null, null);
var componentTemplate = new ComponentTemplate(tsdbTemplate, null, null);
var indexTemplate = ComposableIndexTemplate.builder()
.indexPatterns(List.of("test-*"))
.componentTemplates(List.of("component_template_1"))
.dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
.build();
Metadata m = Metadata.builder().put("component_template_1", componentTemplate).put("index_template_1", indexTemplate).build();
assertThat(m.isTimeSeriesTemplate(indexTemplate), is(true));
assertThat(m.retrieveIndexModeFromTemplate(indexTemplate), is(IndexMode.TIME_SERIES));
}
// Settings in composable index template:
{
var componentTemplate = new ComponentTemplate(new Template(null, null, null), null, null);
var indexTemplate = ComposableIndexTemplate.builder()
.indexPatterns(List.of("test-*"))
.template(template)
.template(tsdbTemplate)
.componentTemplates(List.of("component_template_1"))
.dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
.build();
Metadata m = Metadata.builder().put("component_template_1", componentTemplate).put("index_template_1", indexTemplate).build();
assertThat(m.isTimeSeriesTemplate(indexTemplate), is(true));
assertThat(m.retrieveIndexModeFromTemplate(indexTemplate), is(IndexMode.TIME_SERIES));
}
}

public void testRetrieveIndexModeFromTemplateLogsdb() throws IOException {
// logsdb:
var logsdbTemplate = new Template(Settings.builder().put("index.mode", "logsdb").build(), new CompressedXContent("{}"), null);
// Settings in component template:
{
var componentTemplate = new ComponentTemplate(logsdbTemplate, null, null);
var indexTemplate = ComposableIndexTemplate.builder()
.indexPatterns(List.of("test-*"))
.componentTemplates(List.of("component_template_1"))
.dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
.build();
Metadata m = Metadata.builder().put("component_template_1", componentTemplate).put("index_template_1", indexTemplate).build();
assertThat(m.retrieveIndexModeFromTemplate(indexTemplate), is(IndexMode.LOGSDB));
}
// Settings in composable index template:
{
var componentTemplate = new ComponentTemplate(new Template(null, null, null), null, null);
var indexTemplate = ComposableIndexTemplate.builder()
.indexPatterns(List.of("test-*"))
.template(logsdbTemplate)
.componentTemplates(List.of("component_template_1"))
.dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
.build();
Metadata m = Metadata.builder().put("component_template_1", componentTemplate).put("index_template_1", indexTemplate).build();
assertThat(m.retrieveIndexModeFromTemplate(indexTemplate), is(IndexMode.LOGSDB));
}
}

public void testRetrieveIndexModeFromTemplateEmpty() throws IOException {
// no index mode:
var emptyTemplate = new Template(Settings.EMPTY, new CompressedXContent("{}"), null);
// Settings in component template:
{
var componentTemplate = new ComponentTemplate(emptyTemplate, null, null);
var indexTemplate = ComposableIndexTemplate.builder()
.indexPatterns(List.of("test-*"))
.componentTemplates(List.of("component_template_1"))
.dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
.build();
Metadata m = Metadata.builder().put("component_template_1", componentTemplate).put("index_template_1", indexTemplate).build();
assertThat(m.retrieveIndexModeFromTemplate(indexTemplate), nullValue());
}
// Settings in composable index template:
{
var componentTemplate = new ComponentTemplate(new Template(null, null, null), null, null);
var indexTemplate = ComposableIndexTemplate.builder()
.indexPatterns(List.of("test-*"))
.template(emptyTemplate)
.componentTemplates(List.of("component_template_1"))
.dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
.build();
Metadata m = Metadata.builder().put("component_template_1", componentTemplate).put("index_template_1", indexTemplate).build();
assertThat(m.retrieveIndexModeFromTemplate(indexTemplate), nullValue());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ static class TestIndexSettingsProvider implements IndexSettingProvider {
public Settings getAdditionalIndexSettings(
String indexName,
String dataStreamName,
boolean isTimeSeries,
IndexMode templateIndexMode,
Metadata metadata,
Instant resolvedAt,
Settings indexTemplateAndCreateRequestSettings,
Expand Down
Loading

0 comments on commit cb116ef

Please sign in to comment.