[ML] Limit ML filter items to 10K (#31731)
Add hard limit to the number of items a filter may have. This serves to protect from excessive overhead due to the filters taking too much memory or lookups becoming too expensive.
This commit is contained in:
parent
e9f8442bee
commit
896317fe36
|
@ -29,6 +29,15 @@ import java.util.TreeSet;
|
|||
|
||||
public class MlFilter implements ToXContentObject, Writeable {
|
||||
|
||||
/**
|
||||
* The max number of items allowed per filter.
|
||||
* Limiting the number of items protects users
|
||||
* from running into excessive overhead due to
|
||||
* filters using too much memory and lookups
|
||||
* becoming too expensive.
|
||||
*/
|
||||
private static final int MAX_ITEMS = 10000;
|
||||
|
||||
public static final String DOCUMENT_ID_PREFIX = "filter_";
|
||||
|
||||
public static final String FILTER_TYPE = "filter";
|
||||
|
@ -62,7 +71,7 @@ public class MlFilter implements ToXContentObject, Writeable {
|
|||
private MlFilter(String id, String description, SortedSet<String> items) {
|
||||
this.id = Objects.requireNonNull(id);
|
||||
this.description = description;
|
||||
this.items = Objects.requireNonNull(items, ITEMS.getPreferredName() + " must not be null");
|
||||
this.items = Objects.requireNonNull(items);
|
||||
}
|
||||
|
||||
public MlFilter(StreamInput in) throws IOException {
|
||||
|
@ -182,9 +191,13 @@ public class MlFilter implements ToXContentObject, Writeable {
|
|||
|
||||
public MlFilter build() {
|
||||
ExceptionsHelper.requireNonNull(id, MlFilter.ID.getPreferredName());
|
||||
ExceptionsHelper.requireNonNull(items, MlFilter.ITEMS.getPreferredName());
|
||||
if (!MlStrings.isValidId(id)) {
|
||||
throw ExceptionsHelper.badRequestException(Messages.getMessage(Messages.INVALID_ID, ID.getPreferredName(), id));
|
||||
}
|
||||
if (items.size() > MAX_ITEMS) {
|
||||
throw ExceptionsHelper.badRequestException(Messages.getMessage(Messages.FILTER_CONTAINS_TOO_MANY_ITEMS, id, MAX_ITEMS));
|
||||
}
|
||||
return new MlFilter(id, description, items);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -43,6 +43,7 @@ public final class Messages {
|
|||
"Datafeed frequency [{0}] must be a multiple of the aggregation interval [{1}]";
|
||||
|
||||
public static final String FILTER_NOT_FOUND = "No filter with id [{0}] exists";
|
||||
public static final String FILTER_CONTAINS_TOO_MANY_ITEMS = "Filter [{0}] contains too many items; up to [{1}] items are allowed";
|
||||
|
||||
public static final String INCONSISTENT_ID =
|
||||
"Inconsistent {0}; ''{1}'' specified in the body differs from ''{2}'' specified as a URL argument";
|
||||
|
|
|
@ -13,6 +13,8 @@ import org.elasticsearch.common.xcontent.json.JsonXContent;
|
|||
import org.elasticsearch.test.AbstractSerializingTestCase;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.SortedSet;
|
||||
import java.util.TreeSet;
|
||||
|
||||
|
@ -71,9 +73,9 @@ public class MlFilterTests extends AbstractSerializingTestCase<MlFilter> {
|
|||
}
|
||||
|
||||
public void testNullItems() {
|
||||
NullPointerException ex = expectThrows(NullPointerException.class,
|
||||
Exception ex = expectThrows(IllegalArgumentException.class,
|
||||
() -> MlFilter.builder(randomValidFilterId()).setItems((SortedSet<String>) null).build());
|
||||
assertEquals(MlFilter.ITEMS.getPreferredName() + " must not be null", ex.getMessage());
|
||||
assertEquals("[items] must not be null.", ex.getMessage());
|
||||
}
|
||||
|
||||
public void testDocumentId() {
|
||||
|
@ -102,6 +104,27 @@ public class MlFilterTests extends AbstractSerializingTestCase<MlFilter> {
|
|||
assertThat(e.getMessage(), startsWith("Invalid filter_id; 'Invalid id' can contain lowercase"));
|
||||
}
|
||||
|
||||
public void testTooManyItems() {
|
||||
List<String> items = new ArrayList<>(10001);
|
||||
for (int i = 0; i < 10001; ++i) {
|
||||
items.add("item_" + i);
|
||||
}
|
||||
ElasticsearchStatusException e = expectThrows(ElasticsearchStatusException.class,
|
||||
() -> MlFilter.builder("huge").setItems(items).build());
|
||||
assertThat(e.getMessage(), startsWith("Filter [huge] contains too many items"));
|
||||
}
|
||||
|
||||
public void testGivenItemsAreMaxAllowed() {
|
||||
List<String> items = new ArrayList<>(10000);
|
||||
for (int i = 0; i < 10000; ++i) {
|
||||
items.add("item_" + i);
|
||||
}
|
||||
|
||||
MlFilter hugeFilter = MlFilter.builder("huge").setItems(items).build();
|
||||
|
||||
assertThat(hugeFilter.getItems().size(), equalTo(items.size()));
|
||||
}
|
||||
|
||||
public void testItemsAreSorted() {
|
||||
MlFilter filter = MlFilter.builder("foo").setItems("c", "b", "a").build();
|
||||
assertThat(filter.getItems(), contains("a", "b", "c"));
|
||||
|
|
Loading…
Reference in New Issue