Skip to content

Commit 5f9965e

Browse files
przemekwitekprzemekwitek
authored
Lower minimum model memory limit value from 1MB to 1kB. (#49227) (#49242)
1 parent f7d9e7b commit 5f9965e

File tree

7 files changed

+39
-22
lines changed

7 files changed

+39
-22
lines changed

‎client/rest-high-level/src/test/java/org/elasticsearch/client/MachineLearningIT.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@
177177
import static org.hamcrest.Matchers.contains;
178178
import static org.hamcrest.Matchers.containsInAnyOrder;
179179
import static org.hamcrest.Matchers.equalTo;
180-
import static org.hamcrest.Matchers.greaterThan;
181180
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
182181
import static org.hamcrest.Matchers.hasItem;
183182
import static org.hamcrest.Matchers.hasItems;
@@ -1989,8 +1988,8 @@ public void testEstimateMemoryUsage() throws IOException {
19891988
EstimateMemoryUsageResponse response1 =
19901989
execute(
19911990
estimateMemoryUsageRequest, machineLearningClient::estimateMemoryUsage, machineLearningClient::estimateMemoryUsageAsync);
1992-
assertThat(response1.getExpectedMemoryWithoutDisk(), allOf(greaterThan(lowerBound), lessThan(upperBound)));
1993-
assertThat(response1.getExpectedMemoryWithDisk(), allOf(greaterThan(lowerBound), lessThan(upperBound)));
1991+
assertThat(response1.getExpectedMemoryWithoutDisk(), allOf(greaterThanOrEqualTo(lowerBound), lessThan(upperBound)));
1992+
assertThat(response1.getExpectedMemoryWithDisk(), allOf(greaterThanOrEqualTo(lowerBound), lessThan(upperBound)));
19941993

19951994
BulkRequest bulk2 = new BulkRequest()
19961995
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
@@ -1999,13 +1998,16 @@ public void testEstimateMemoryUsage() throws IOException {
19991998
}
20001999
highLevelClient().bulk(bulk2, RequestOptions.DEFAULT);
20012000

2002-
// Data Frame now has 100 rows, expect that the returned estimates will be greater than the previous ones.
2001+
// Data Frame now has 100 rows, expect that the returned estimates will be greater than or equal to the previous ones.
20032002
EstimateMemoryUsageResponse response2 =
20042003
execute(
20052004
estimateMemoryUsageRequest, machineLearningClient::estimateMemoryUsage, machineLearningClient::estimateMemoryUsageAsync);
20062005
assertThat(
2007-
response2.getExpectedMemoryWithoutDisk(), allOf(greaterThan(response1.getExpectedMemoryWithoutDisk()), lessThan(upperBound)));
2008-
assertThat(response2.getExpectedMemoryWithDisk(), allOf(greaterThan(response1.getExpectedMemoryWithDisk()), lessThan(upperBound)));
2006+
response2.getExpectedMemoryWithoutDisk(),
2007+
allOf(greaterThanOrEqualTo(response1.getExpectedMemoryWithoutDisk()), lessThan(upperBound)));
2008+
assertThat(
2009+
response2.getExpectedMemoryWithDisk(),
2010+
allOf(greaterThanOrEqualTo(response1.getExpectedMemoryWithDisk()), lessThan(upperBound)));
20092011
}
20102012

20112013
public void testPutFilter() throws Exception {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsConfig.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public class DataFrameAnalyticsConfig implements ToXContentObject, Writeable {
4040
public static final String TYPE = "data_frame_analytics_config";
4141

4242
public static final ByteSizeValue DEFAULT_MODEL_MEMORY_LIMIT = new ByteSizeValue(1, ByteSizeUnit.GB);
43-
public static final ByteSizeValue MIN_MODEL_MEMORY_LIMIT = new ByteSizeValue(1, ByteSizeUnit.MB);
43+
public static final ByteSizeValue MIN_MODEL_MEMORY_LIMIT = new ByteSizeValue(1, ByteSizeUnit.KB);
4444
/**
4545
* This includes the overhead of thread stacks and data structures that the program might use that
4646
* are not instrumented. But it does NOT include the memory used by loading the executable code.
@@ -442,7 +442,8 @@ private void applyMaxModelMemoryLimit() {
442442
if (modelMemoryLimit.compareTo(MIN_MODEL_MEMORY_LIMIT) < 0) {
443443
// Explicit setting lower than minimum is an error
444444
throw ExceptionsHelper.badRequestException(
445-
Messages.getMessage(Messages.JOB_CONFIG_MODEL_MEMORY_LIMIT_TOO_LOW, modelMemoryLimit));
445+
Messages.getMessage(
446+
Messages.JOB_CONFIG_MODEL_MEMORY_LIMIT_TOO_LOW, modelMemoryLimit, MIN_MODEL_MEMORY_LIMIT.getStringRep()));
446447
}
447448
if (maxModelMemoryIsSet && modelMemoryLimit.compareTo(maxModelMemoryLimit) > 0) {
448449
// Explicit setting higher than limit is an error

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/AnalysisLimits.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public AnalysisLimits(Long categorizationExamplesLimit) {
9090

9191
public AnalysisLimits(Long modelMemoryLimit, Long categorizationExamplesLimit) {
9292
if (modelMemoryLimit != null && modelMemoryLimit < 1) {
93-
String msg = Messages.getMessage(Messages.JOB_CONFIG_MODEL_MEMORY_LIMIT_TOO_LOW, modelMemoryLimit);
93+
String msg = Messages.getMessage(Messages.JOB_CONFIG_MODEL_MEMORY_LIMIT_TOO_LOW, modelMemoryLimit, "1 MiB");
9494
throw ExceptionsHelper.badRequestException(msg);
9595
}
9696
if (categorizationExamplesLimit != null && categorizationExamplesLimit < 0) {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ public final class Messages {
143143
"Invalid detector rule: scope field ''{0}'' is invalid; select from {1}";
144144
public static final String JOB_CONFIG_FIELDNAME_INCOMPATIBLE_FUNCTION = "field_name cannot be used with function ''{0}''";
145145
public static final String JOB_CONFIG_FIELD_VALUE_TOO_LOW = "{0} cannot be less than {1,number}. Value = {2,number}";
146-
public static final String JOB_CONFIG_MODEL_MEMORY_LIMIT_TOO_LOW = "model_memory_limit must be at least 1 MiB. Value = {0}";
146+
public static final String JOB_CONFIG_MODEL_MEMORY_LIMIT_TOO_LOW = "model_memory_limit must be at least {1}. Value = {0}";
147147
public static final String JOB_CONFIG_MODEL_MEMORY_LIMIT_GREATER_THAN_MAX =
148148
"model_memory_limit [{0}] must be less than the value of the " +
149149
MachineLearningField.MAX_MODEL_MEMORY_LIMIT.getKey() +

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/EstimateMemoryUsageActionResponseTests.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.core.ml.action;
77

88
import org.elasticsearch.common.io.stream.Writeable;
9+
import org.elasticsearch.common.unit.ByteSizeUnit;
910
import org.elasticsearch.common.unit.ByteSizeValue;
1011
import org.elasticsearch.common.xcontent.XContentParser;
1112
import org.elasticsearch.test.AbstractSerializingTestCase;
@@ -39,9 +40,15 @@ public void testConstructor_NullValues() {
3940
assertThat(response.getExpectedMemoryWithDisk(), nullValue());
4041
}
4142

43+
public void testConstructor_SmallValues() {
44+
Response response = new Response(new ByteSizeValue(120, ByteSizeUnit.KB), new ByteSizeValue(30, ByteSizeUnit.KB));
45+
assertThat(response.getExpectedMemoryWithoutDisk(), equalTo(new ByteSizeValue(120, ByteSizeUnit.KB)));
46+
assertThat(response.getExpectedMemoryWithDisk(), equalTo(new ByteSizeValue(30, ByteSizeUnit.KB)));
47+
}
48+
4249
public void testConstructor() {
43-
Response response = new Response(new ByteSizeValue(2048), new ByteSizeValue(1024));
44-
assertThat(response.getExpectedMemoryWithoutDisk(), equalTo(new ByteSizeValue(2048)));
45-
assertThat(response.getExpectedMemoryWithDisk(), equalTo(new ByteSizeValue(1024)));
50+
Response response = new Response(new ByteSizeValue(20, ByteSizeUnit.MB), new ByteSizeValue(10, ByteSizeUnit.MB));
51+
assertThat(response.getExpectedMemoryWithoutDisk(), equalTo(new ByteSizeValue(20, ByteSizeUnit.MB)));
52+
assertThat(response.getExpectedMemoryWithDisk(), equalTo(new ByteSizeValue(10, ByteSizeUnit.MB)));
4653
}
4754
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsConfigTests.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -232,18 +232,16 @@ public void testInvalidModelMemoryLimits() {
232232
DataFrameAnalyticsConfig.Builder builder = new DataFrameAnalyticsConfig.Builder();
233233

234234
// All these are different ways of specifying a limit that is lower than the minimum
235-
assertTooSmall(expectThrows(ElasticsearchStatusException.class,
236-
() -> builder.setModelMemoryLimit(new ByteSizeValue(1048575, ByteSizeUnit.BYTES)).build()));
237-
assertTooSmall(expectThrows(ElasticsearchStatusException.class,
238-
() -> builder.setModelMemoryLimit(new ByteSizeValue(0, ByteSizeUnit.BYTES)).build()));
239235
assertTooSmall(expectThrows(ElasticsearchStatusException.class,
240236
() -> builder.setModelMemoryLimit(new ByteSizeValue(-1, ByteSizeUnit.BYTES)).build()));
241237
assertTooSmall(expectThrows(ElasticsearchStatusException.class,
242-
() -> builder.setModelMemoryLimit(new ByteSizeValue(1023, ByteSizeUnit.KB)).build()));
238+
() -> builder.setModelMemoryLimit(new ByteSizeValue(0, ByteSizeUnit.BYTES)).build()));
243239
assertTooSmall(expectThrows(ElasticsearchStatusException.class,
244240
() -> builder.setModelMemoryLimit(new ByteSizeValue(0, ByteSizeUnit.KB)).build()));
245241
assertTooSmall(expectThrows(ElasticsearchStatusException.class,
246242
() -> builder.setModelMemoryLimit(new ByteSizeValue(0, ByteSizeUnit.MB)).build()));
243+
assertTooSmall(expectThrows(ElasticsearchStatusException.class,
244+
() -> builder.setModelMemoryLimit(new ByteSizeValue(1023, ByteSizeUnit.BYTES)).build()));
247245
}
248246

249247
public void testNoMemoryCapping() {
@@ -342,6 +340,6 @@ public void testPreventVersionInjection() throws IOException {
342340
}
343341

344342
private static void assertTooSmall(ElasticsearchStatusException e) {
345-
assertThat(e.getMessage(), startsWith("model_memory_limit must be at least 1 MiB."));
343+
assertThat(e.getMessage(), startsWith("model_memory_limit must be at least 1kb."));
346344
}
347345
}

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/dataframe/process/results/MemoryUsageEstimationResultTests.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
package org.elasticsearch.xpack.ml.dataframe.process.results;
77

8+
import org.elasticsearch.common.unit.ByteSizeUnit;
89
import org.elasticsearch.common.unit.ByteSizeValue;
910
import org.elasticsearch.common.xcontent.XContentParser;
1011
import org.elasticsearch.test.AbstractXContentTestCase;
@@ -43,9 +44,17 @@ public void testConstructor_NullValues() {
4344
assertThat(result.getExpectedMemoryWithDisk(), nullValue());
4445
}
4546

47+
public void testConstructor_SmallValues() {
48+
MemoryUsageEstimationResult result =
49+
new MemoryUsageEstimationResult(new ByteSizeValue(120, ByteSizeUnit.KB), new ByteSizeValue(30, ByteSizeUnit.KB));
50+
assertThat(result.getExpectedMemoryWithoutDisk(), equalTo(new ByteSizeValue(120, ByteSizeUnit.KB)));
51+
assertThat(result.getExpectedMemoryWithDisk(), equalTo(new ByteSizeValue(30, ByteSizeUnit.KB)));
52+
}
53+
4654
public void testConstructor() {
47-
MemoryUsageEstimationResult result = new MemoryUsageEstimationResult(new ByteSizeValue(2048), new ByteSizeValue(1024));
48-
assertThat(result.getExpectedMemoryWithoutDisk(), equalTo(new ByteSizeValue(2048)));
49-
assertThat(result.getExpectedMemoryWithDisk(), equalTo(new ByteSizeValue(1024)));
55+
MemoryUsageEstimationResult result =
56+
new MemoryUsageEstimationResult(new ByteSizeValue(20, ByteSizeUnit.MB), new ByteSizeValue(10, ByteSizeUnit.MB));
57+
assertThat(result.getExpectedMemoryWithoutDisk(), equalTo(new ByteSizeValue(20, ByteSizeUnit.MB)));
58+
assertThat(result.getExpectedMemoryWithDisk(), equalTo(new ByteSizeValue(10, ByteSizeUnit.MB)));
5059
}
5160
}

0 commit comments

Comments
 (0)