Skip to content
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

Validate tsdb's routing_path #79384

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -168,3 +168,45 @@ routing required:
mappings:
_routing:
required: true

---
bad routing_path:
- skip:
version: " - 7.99.99"
reason: introduced in 8.0.0

- do:
catch: /All fields that match routing_path must be keyword time_series_dimensions but \[@timestamp\] was \[date\]/
indices.create:
index: test_index
body:
settings:
index:
mode: time_series
routing_path: [metricset, k8s.pod.uid, "@timestamp"]
number_of_replicas: 0
number_of_shards: 2
mappings:
properties:
"@timestamp":
type: date
metricset:
type: keyword
time_series_dimension: true
k8s:
properties:
pod:
properties:
uid:
type: keyword
time_series_dimension: true
name:
type: keyword
ip:
type: ip
network:
properties:
tx:
type: long
rx:
type: long
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
add time series mappings:
ecs style:
- skip:
version: " - 7.99.99"
reason: introduced in 8.0.0
Expand Down Expand Up @@ -49,3 +49,58 @@ add time series mappings:
latency:
type: double
time_series_metric: gauge

---
top level dim object:
- skip:
version: " - 7.99.99"
reason: introduced in 8.0.0

- do:
indices.create:
index: tsdb_index
body:
settings:
index:
mode: time_series
routing_path: [dim.*]
number_of_replicas: 0
number_of_shards: 2
mappings:
properties:
"@timestamp":
type: date
dim:
properties:
metricset:
type: keyword
time_series_dimension: true
uid:
type: keyword
time_series_dimension: true
k8s:
properties:
pod:
properties:
availability_zone:
type: short
time_series_dimension: true
name:
type: keyword
ip:
type: ip
time_series_dimension: true
network:
properties:
tx:
type: long
time_series_metric: counter
rx:
type: integer
time_series_metric: gauge
packets_dropped:
type: long
time_series_metric: gauge
latency:
type: double
time_series_metric: gauge
2 changes: 2 additions & 0 deletions server/src/main/java/org/elasticsearch/index/IndexMode.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ void validateWithOtherSettings(Map<Setting<?>, Object> settings) {
}
}

@Override
public void validateMapping(MappingLookup lookup) {};

@Override
Expand Down Expand Up @@ -66,6 +67,7 @@ private String error(Setting<?> unsupported) {
return tsdbMode() + " is incompatible with [" + unsupported.getKey() + "]";
}

@Override
public void validateMapping(MappingLookup lookup) {
if (((RoutingFieldMapper) lookup.getMapper(RoutingFieldMapper.NAME)).required()) {
throw new IllegalArgumentException(routingRequiredBad());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.xcontent.support.filtering.FilterPathBasedFilter;

import java.util.List;
import java.util.Set;

public class DocumentMapper {
private final String type;
Expand Down Expand Up @@ -87,6 +91,10 @@ public void validate(IndexSettings settings, boolean checkLimits) {
if (settings.getIndexSortConfig().hasIndexSort() && mappers().hasNested()) {
throw new IllegalArgumentException("cannot have nested fields when index sort is activated");
}
List<String> routingPaths = settings.getIndexMetadata().getRoutingPaths();
if (false == routingPaths.isEmpty()) {
mappingLookup.getMapping().getRoot().validateRoutingPath(new FilterPathBasedFilter(Set.copyOf(routingPaths), true));
}
if (checkLimits) {
this.mappingLookup.checkLimits(settings);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,4 +584,14 @@ public FieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), indexAnalyzers, scriptCompiler).dimension(dimension).init(this);
}

@Override
protected void validateMatchedRoutingPath() {
if (false == fieldType().isDimension()) {
throw new IllegalArgumentException(
"All fields that match routing_path must be keyword time_series_dimensions but ["
+ name()
+ "] was not a time_series_dimension"
);
}
}
}
28 changes: 28 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/Mapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

package org.elasticsearch.index.mapper;

import com.fasterxml.jackson.core.filter.TokenFilter;

import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.xcontent.ToXContentFragment;

import java.util.Map;
Expand Down Expand Up @@ -66,4 +69,29 @@ public final String simpleName() {
*/
public abstract void validate(MappingLookup mappers);

/**
* Validate a {@link TokenFilter} made from {@link IndexMetadata#INDEX_ROUTING_PATH}.
*/
public final void validateRoutingPath(TokenFilter filter) {
nik9000 marked this conversation as resolved.
Show resolved Hide resolved
if (filter == TokenFilter.INCLUDE_ALL) {
validateMatchedRoutingPath();
}
for (Mapper m : this) {
TokenFilter next = filter.includeProperty(m.simpleName());
if (next == null) {
// null means "do not include"
continue;
}
m.validateRoutingPath(next);
}
}

/**
* Validate that this field can be the target of {@link IndexMetadata#INDEX_ROUTING_PATH}.
*/
protected void validateMatchedRoutingPath() {
throw new IllegalArgumentException(
"All fields that match routing_path must be keyword time_series_dimensions but [" + name() + "] was [" + typeName() + "]"
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.mapper.MapperServiceTestCase;

import java.io.IOException;

import static org.hamcrest.Matchers.equalTo;

public class TimeSeriesModeTests extends MapperServiceTestCase {
Expand Down Expand Up @@ -103,4 +105,76 @@ public void testValidateAliasWithSearchRouting() {
assertThat(e.getMessage(), equalTo("routing is forbidden on CRUD operations that target indices in [index.mode=time_series]"));
}

public void testRoutingPathMatchesObject() {
Settings s = Settings.builder()
.put(IndexSettings.MODE.getKey(), "time_series")
.put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), randomBoolean() ? "dim.o" : "dim.*")
.build();
Exception e = expectThrows(IllegalArgumentException.class, () -> createMapperService(s, mapping(b -> {
b.startObject("dim").startObject("properties");
{
b.startObject("o").startObject("properties");
b.startObject("inner_dim").field("type", "keyword").field("time_series_dimension", true).endObject();
b.endObject().endObject();
}
b.startObject("dim").field("type", "keyword").field("time_series_dimension", true).endObject();
b.endObject().endObject();
})));
assertThat(
e.getMessage(),
equalTo("All fields that match routing_path must be keyword time_series_dimensions but [dim.o] was [object]")
);
}

public void testRoutingPathMatchesNonDimensionKeyword() {
Settings s = Settings.builder()
.put(IndexSettings.MODE.getKey(), "time_series")
.put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), randomBoolean() ? "dim.non_dim" : "dim.*")
.build();
Exception e = expectThrows(IllegalArgumentException.class, () -> createMapperService(s, mapping(b -> {
b.startObject("dim").startObject("properties");
b.startObject("non_dim").field("type", "keyword").endObject();
b.startObject("dim").field("type", "keyword").field("time_series_dimension", true).endObject();
b.endObject().endObject();
})));
assertThat(
e.getMessage(),
equalTo(
"All fields that match routing_path must be keyword time_series_dimensions but "
+ "[dim.non_dim] was not a time_series_dimension"
)
);
}

public void testRoutingPathMatchesNonKeyword() {
Settings s = Settings.builder()
.put(IndexSettings.MODE.getKey(), "time_series")
.put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), randomBoolean() ? "dim.non_kwd" : "dim.*")
.build();
Exception e = expectThrows(IllegalArgumentException.class, () -> createMapperService(s, mapping(b -> {
b.startObject("dim").startObject("properties");
b.startObject("non_kwd").field("type", "integer").field("time_series_dimension", true).endObject();
b.startObject("dim").field("type", "keyword").field("time_series_dimension", true).endObject();
b.endObject().endObject();
})));
assertThat(
e.getMessage(),
equalTo("All fields that match routing_path must be keyword time_series_dimensions but [dim.non_kwd] was [integer]")
);
}

public void testRoutingPathMatchesOnlyKeywordDimensions() throws IOException {
Settings s = Settings.builder()
.put(IndexSettings.MODE.getKey(), "time_series")
.put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), randomBoolean() ? "dim.metric_type,dim.server,dim.species,dim.uuid" : "dim.*")
.build();
createMapperService(s, mapping(b -> {
b.startObject("dim").startObject("properties");
b.startObject("metric_type").field("type", "keyword").field("time_series_dimension", true).endObject();
b.startObject("server").field("type", "keyword").field("time_series_dimension", true).endObject();
b.startObject("species").field("type", "keyword").field("time_series_dimension", true).endObject();
b.startObject("uuid").field("type", "keyword").field("time_series_dimension", true).endObject();
b.endObject().endObject();
})); // doesn't throw
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.elasticsearch.index.mapper;

import com.fasterxml.jackson.core.filter.TokenFilter;

import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.MockLowerCaseFilter;
import org.apache.lucene.analysis.MockTokenizer;
Expand Down Expand Up @@ -579,4 +581,16 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
protected boolean dedupAfterFetch() {
return true;
}

@Override
protected String minimalIsInvalidRoutingPathErrorMessage(Mapper mapper) {
return "All fields that match routing_path must be keyword time_series_dimensions but [field] was not a time_series_dimension";
}

public void testDimensionInRoutingPath() throws IOException {
Mapper mapper = createMapperService(fieldMapping(b -> b.field("type", "keyword").field("time_series_dimension", true)))
.mappingLookup()
.getMapper("field");
mapper.validateRoutingPath(TokenFilter.INCLUDE_ALL); // Doesn't throw
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.elasticsearch.index.mapper;

import com.fasterxml.jackson.core.filter.TokenFilter;

import org.apache.lucene.index.DocValuesType;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexableField;
Expand Down Expand Up @@ -750,4 +752,14 @@ public final void testNullInput() throws Exception {
protected boolean allowsNullValues() {
return true;
}

public final void testMinimalIsInvalidInRoutingPath() throws IOException {
Mapper mapper = createMapperService(fieldMapping(this::minimalMapping)).mappingLookup().getMapper("field");
Exception e = expectThrows(IllegalArgumentException.class, () -> mapper.validateRoutingPath(TokenFilter.INCLUDE_ALL));
assertThat(e.getMessage(), equalTo(minimalIsInvalidRoutingPathErrorMessage(mapper)));
}

protected String minimalIsInvalidRoutingPathErrorMessage(Mapper mapper) {
return "All fields that match routing_path must be keyword time_series_dimensions but [field] was [" + mapper.typeName() + "]";
}
}