Skip to content

Commit 7cb2bd5

Browse files
seanzatzdevCopilotelasticsearchmachine
authored
Adjust sampling max size limit to depend on heap size (#137482)
* Adjust sampling max size limit to depend on heap size * add helper method for max size * [CI] Auto commit changes from spotless * update tests with lower limits * separate out default and max size limit --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: elasticsearchmachine <[email protected]>
1 parent 83330cd commit 7cb2bd5

File tree

15 files changed

+103
-59
lines changed

15 files changed

+103
-59
lines changed

‎rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.delete_sample_configuration/10_basic.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ teardown:
2828
body:
2929
rate: 0.5
3030
max_samples: 100
31-
max_size: "10mb"
31+
max_size: "100kb"
3232
time_to_live: "1h"
3333

3434
- match: { acknowledged: true }
@@ -186,7 +186,7 @@ teardown:
186186
body:
187187
rate: 0.9
188188
max_samples: 90
189-
max_size: "20mb"
189+
max_size: "100kb"
190190

191191
- match: { acknowledged: true }
192192

‎rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.get_all_sample_configuration/10_basic.yml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ teardown:
4040
body:
4141
rate: 0.5
4242
max_samples: 100
43-
max_size: "10mb"
43+
max_size: "100kb"
4444
time_to_live: "1h"
4545

4646
- match: { acknowledged: true }
@@ -53,7 +53,7 @@ teardown:
5353
- match: { configurations.0.index: "test-single-config-index" }
5454
- match: { configurations.0.configuration.rate: 0.5 }
5555
- match: { configurations.0.configuration.max_samples: 100 }
56-
- match: { configurations.0.configuration.max_size: "10mb" }
56+
- match: { configurations.0.configuration.max_size: "100kb" }
5757
- match: { configurations.0.configuration.time_to_live: "1h" }
5858

5959
---
@@ -85,7 +85,7 @@ teardown:
8585
body:
8686
rate: 0.7
8787
max_samples: 200
88-
max_size: "20mb"
88+
max_size: "100kb"
8989

9090
- match: { acknowledged: true }
9191

@@ -186,7 +186,7 @@ teardown:
186186
body:
187187
rate: 0.9
188188
max_samples: 150
189-
max_size: "15mb"
189+
max_size: "100kb"
190190

191191
- match: { acknowledged: true }
192192

@@ -199,7 +199,7 @@ teardown:
199199
- match: { configurations.0.index: "test-update-all-index" }
200200
- match: { configurations.0.configuration.rate: 0.9 }
201201
- match: { configurations.0.configuration.max_samples: 150 }
202-
- match: { configurations.0.configuration.max_size: "15mb" }
202+
- match: { configurations.0.configuration.max_size: "100kb" }
203203

204204
---
205205
"Get all sampling configurations after deletion":

‎rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.get_sample_configuration/10_basic.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ teardown:
2727
body:
2828
rate: 0.5
2929
max_samples: 100
30-
max_size: "10mb"
30+
max_size: "100kb"
3131
time_to_live: "1h"
3232

3333
- match: { acknowledged: true }
@@ -39,7 +39,7 @@ teardown:
3939

4040
- match: { configuration.rate: 0.5 }
4141
- match: { configuration.max_samples: 100 }
42-
- match: { configuration.max_size: "10mb" }
42+
- match: { configuration.max_size: "100kb" }
4343
- match: { configuration.time_to_live: "1h" }
4444

4545
---
@@ -140,7 +140,7 @@ teardown:
140140
body:
141141
rate: 0.8
142142
max_samples: 75
143-
max_size: "5mb"
143+
max_size: "100kb"
144144

145145
- match: { acknowledged: true }
146146

@@ -152,7 +152,7 @@ teardown:
152152

153153
- match: { configuration.rate: 0.8 }
154154
- match: { configuration.max_samples: 75 }
155-
- match: { configuration.max_size: "5mb" }
155+
- match: { configuration.max_size: "100kb" }
156156

157157
---
158158
"Get sampling configuration with master timeout":

‎rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.put_sample_configuration/10_basic.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ teardown:
2727
body:
2828
rate: 0.5
2929
max_samples: 100
30-
max_size: "10mb"
30+
max_size: "100kb"
3131
time_to_live: "1h"
3232

3333
- match: { acknowledged: true }
@@ -85,7 +85,7 @@ teardown:
8585
body:
8686
rate: 0.8
8787
max_samples: 75
88-
max_size: "5mb"
88+
max_size: "100kb"
8989

9090
- match: { acknowledged: true }
9191

@@ -155,7 +155,7 @@ teardown:
155155
body:
156156
rate: ".05"
157157
max_samples: 1000
158-
max_size: "10mb"
158+
max_size: "100kb"
159159
time_to_live: "1d"
160160

161161
- match: { acknowledged: true }

‎server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/sampling/GetAllSampleConfigurationActionIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ private SamplingConfiguration randomSamplingConfiguration() {
215215
return new SamplingConfiguration(
216216
randomDoubleBetween(0.1, 1.0, true),
217217
randomBoolean() ? randomIntBetween(1, SamplingConfiguration.MAX_SAMPLES_LIMIT) : null,
218-
randomBoolean() ? ByteSizeValue.ofGb(randomLongBetween(1, SamplingConfiguration.MAX_SIZE_LIMIT_GIGABYTES)) : null,
218+
randomBoolean() ? ByteSizeValue.ofMb(randomLongBetween(1, 100)) : null,
219219
randomBoolean() ? randomValidTimeValue() : null,
220220
randomBoolean() ? randomAlphaOfLengthBetween(5, 30) : null
221221
);

‎server/src/main/java/org/elasticsearch/action/admin/indices/sampling/SamplingConfiguration.java‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.common.io.stream.StreamOutput;
1616
import org.elasticsearch.common.unit.ByteSizeValue;
1717
import org.elasticsearch.core.TimeValue;
18+
import org.elasticsearch.monitor.jvm.JvmInfo;
1819
import org.elasticsearch.xcontent.ConstructingObjectParser;
1920
import org.elasticsearch.xcontent.ObjectParser;
2021
import org.elasticsearch.xcontent.ParseField;
@@ -57,18 +58,23 @@ public record SamplingConfiguration(
5758

5859
// Constants for validation and defaults
5960
public static final int MAX_SAMPLES_LIMIT = 10_000;
60-
public static final long MAX_SIZE_LIMIT_GIGABYTES = 5;
61+
public static final double DEFAULT_MAX_SIZE_HEAP_PERCENTAGE = 0.01;
62+
public static final double MAX_SIZE_HEAP_PERCENTAGE_LIMIT = 0.05;
63+
public static final ByteSizeValue DEFAULT_MAX_SIZE_FLOOR = ByteSizeValue.ofKb(100);
6164
public static final long MAX_TIME_TO_LIVE_DAYS = 30;
6265
public static final int DEFAULT_MAX_SAMPLES = 100;
63-
public static final long DEFAULT_MAX_SIZE_GIGABYTES = 1;
6466
public static final long DEFAULT_TIME_TO_LIVE_DAYS = 10;
6567

6668
// Error messages
6769
public static final String INVALID_RATE_MESSAGE = "rate must be greater than 0 and less than or equal to 1";
6870
public static final String INVALID_MAX_SAMPLES_MIN_MESSAGE = "maxSamples must be greater than 0";
6971
public static final String INVALID_MAX_SAMPLES_MAX_MESSAGE = "maxSamples must be less than or equal to " + MAX_SAMPLES_LIMIT;
7072
public static final String INVALID_MAX_SIZE_MIN_MESSAGE = "maxSize must be greater than 0";
71-
public static final String INVALID_MAX_SIZE_MAX_MESSAGE = "maxSize must be less than or equal to " + MAX_SIZE_LIMIT_GIGABYTES + "GB";
73+
public static final String INVALID_MAX_SIZE_MAX_MESSAGE = "maxSize must be less than or equal to "
74+
+ (int) (MAX_SIZE_HEAP_PERCENTAGE_LIMIT * 100)
75+
+ "% of heap size ("
76+
+ calculateMaxSizeLimit().toString()
77+
+ ")";
7278
public static final String INVALID_TIME_TO_LIVE_MIN_MESSAGE = "timeToLive must be greater than 0";
7379
public static final String INVALID_TIME_TO_LIVE_MAX_MESSAGE = "timeToLive must be less than or equal to "
7480
+ MAX_TIME_TO_LIVE_DAYS
@@ -152,7 +158,7 @@ public record SamplingConfiguration(
152158
*
153159
* @param rate The fraction of documents to sample (must be between 0 and 1)
154160
* @param maxSamples The maximum number of documents to sample (optional, defaults to {@link #DEFAULT_MAX_SAMPLES})
155-
* @param maxSize The maximum total size of sampled documents (optional, defaults to {@link #DEFAULT_MAX_SIZE_GIGABYTES} GB)
161+
* @param maxSize The maximum total size of sampled documents (optional, defaults to {@link #DEFAULT_MAX_SIZE_HEAP_PERCENTAGE} of heap)
156162
* @param timeToLive The duration for which the sampled documents
157163
* should be retained (optional, defaults to {@link #DEFAULT_TIME_TO_LIVE_DAYS} days)
158164
* @param condition An optional condition script that sampled documents must satisfy (optional, can be null)
@@ -168,12 +174,32 @@ public SamplingConfiguration(
168174
) {
169175
this.rate = rate;
170176
this.maxSamples = maxSamples == null ? DEFAULT_MAX_SAMPLES : maxSamples;
171-
this.maxSize = maxSize == null ? ByteSizeValue.ofGb(DEFAULT_MAX_SIZE_GIGABYTES) : maxSize;
177+
this.maxSize = maxSize == null ? calculateDefaultMaxSize() : maxSize;
172178
this.timeToLive = timeToLive == null ? TimeValue.timeValueDays(DEFAULT_TIME_TO_LIVE_DAYS) : timeToLive;
173179
this.condition = condition;
174180
this.creationTime = creationTime == null ? Instant.now().toEpochMilli() : creationTime;
175181
}
176182

183+
/**
184+
* Calculates the default max size as a percentage of the configured heap size,
185+
* with a minimum floor value.
186+
*
187+
* @return The default max size value
188+
*/
189+
private static ByteSizeValue calculateDefaultMaxSize() {
190+
long heapBasedSize = (long) (DEFAULT_MAX_SIZE_HEAP_PERCENTAGE * JvmInfo.jvmInfo().getConfiguredMaxHeapSize());
191+
return ByteSizeValue.ofBytes(Math.max(heapBasedSize, DEFAULT_MAX_SIZE_FLOOR.getBytes()));
192+
}
193+
194+
/**
195+
* Calculates the max size limit as a percentage of the configured heap size.
196+
*
197+
* @return The max size limit value
198+
*/
199+
public static ByteSizeValue calculateMaxSizeLimit() {
200+
return ByteSizeValue.ofBytes((long) (MAX_SIZE_HEAP_PERCENTAGE_LIMIT * JvmInfo.jvmInfo().getConfiguredMaxHeapSize()));
201+
}
202+
177203
// Convenience constructor without creationTime
178204
public SamplingConfiguration(double rate, Integer maxSamples, ByteSizeValue maxSize, TimeValue timeToLive, String condition) {
179205
this(rate, maxSamples, maxSize, timeToLive, condition, null);
@@ -266,7 +292,7 @@ private static void validateInputs(double rate, Integer maxSamples, ByteSizeValu
266292
if (maxSize.compareTo(ByteSizeValue.ZERO) <= 0) {
267293
throw new IllegalArgumentException(INVALID_MAX_SIZE_MIN_MESSAGE);
268294
}
269-
ByteSizeValue maxLimit = ByteSizeValue.ofGb(MAX_SIZE_LIMIT_GIGABYTES);
295+
ByteSizeValue maxLimit = calculateMaxSizeLimit();
270296
if (maxSize.compareTo(maxLimit) > 0) {
271297
throw new IllegalArgumentException(INVALID_MAX_SIZE_MAX_MESSAGE);
272298
}

‎server/src/test/java/org/elasticsearch/action/admin/indices/sampling/GetAllSampleConfigurationActionResponseTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ private SamplingConfiguration createRandomSamplingConfiguration() {
8181
return new SamplingConfiguration(
8282
randomDoubleBetween(0.1, 1.0, true),
8383
randomBoolean() ? randomIntBetween(1, 1000) : null,
84-
randomBoolean() ? ByteSizeValue.ofMb(randomIntBetween(1, 100)) : null,
84+
randomBoolean() ? null : ByteSizeValue.ofKb(randomIntBetween(50, 100)),
8585
randomBoolean() ? TimeValue.timeValueMinutes(randomIntBetween(1, 60)) : null,
8686
randomBoolean() ? "ctx?.field == 'test'" : null
8787
);

‎server/src/test/java/org/elasticsearch/action/admin/indices/sampling/GetSampleConfigurationActionResponseTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ private SamplingConfiguration createRandomSamplingConfiguration() {
4747
return new SamplingConfiguration(
4848
randomDoubleBetween(0.1, 1.0, true),
4949
randomBoolean() ? randomIntBetween(1, 1000) : null,
50-
randomBoolean() ? ByteSizeValue.ofMb(randomIntBetween(1, 100)) : null,
50+
randomBoolean() ? null : ByteSizeValue.ofKb(randomIntBetween(50, 100)),
5151
randomBoolean() ? TimeValue.timeValueMinutes(randomIntBetween(1, 60)) : null,
5252
randomBoolean() ? "ctx?.field == 'test'" : null
5353
);

‎server/src/test/java/org/elasticsearch/action/admin/indices/sampling/PutSampleConfigurationActionTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ private static SamplingConfiguration createRandomSampleConfig() {
114114
return new SamplingConfiguration(
115115
randomDoubleBetween(0.0, 1.0, true),
116116
randomBoolean() ? null : randomIntBetween(1, 1000),
117-
randomBoolean() ? null : ByteSizeValue.ofGb(randomIntBetween(1, 5)),
117+
randomBoolean() ? null : ByteSizeValue.ofKb(randomIntBetween(50, 100)),
118118
randomBoolean() ? null : new TimeValue(randomIntBetween(1, 30), TimeUnit.DAYS),
119119
randomBoolean() ? randomAlphaOfLength(10) : null
120120
);

‎server/src/test/java/org/elasticsearch/action/admin/indices/sampling/SamplingConfigurationTests.java‎

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.common.io.stream.Writeable;
1212
import org.elasticsearch.common.unit.ByteSizeValue;
1313
import org.elasticsearch.core.TimeValue;
14+
import org.elasticsearch.monitor.jvm.JvmInfo;
1415
import org.elasticsearch.test.AbstractXContentSerializingTestCase;
1516
import org.elasticsearch.xcontent.ToXContent;
1617
import org.elasticsearch.xcontent.XContentParser;
@@ -40,17 +41,21 @@ protected Writeable.Reader<SamplingConfiguration> instanceReader() {
4041

4142
@Override
4243
protected SamplingConfiguration createTestInstance() {
44+
long maxHeap = JvmInfo.jvmInfo().getConfiguredMaxHeapSize();
45+
long maxSizeLimit = (long) (SamplingConfiguration.MAX_SIZE_HEAP_PERCENTAGE_LIMIT * maxHeap);
4346
return new SamplingConfiguration(
4447
randomDoubleBetween(0.0, 1.0, true),
4548
randomBoolean() ? null : randomIntBetween(1, SamplingConfiguration.MAX_SAMPLES_LIMIT),
46-
randomBoolean() ? null : ByteSizeValue.ofGb(randomLongBetween(1, SamplingConfiguration.MAX_SIZE_LIMIT_GIGABYTES)),
49+
randomBoolean() ? null : ByteSizeValue.ofBytes(randomLongBetween(1, maxSizeLimit)),
4750
randomBoolean() ? null : TimeValue.timeValueDays(randomLongBetween(1, SamplingConfiguration.MAX_TIME_TO_LIVE_DAYS)),
4851
randomBoolean() ? null : randomAlphaOfLength(10)
4952
);
5053
}
5154

5255
@Override
5356
protected SamplingConfiguration mutateInstance(SamplingConfiguration instance) {
57+
long maxHeap = JvmInfo.jvmInfo().getConfiguredMaxHeapSize();
58+
long maxSizeLimit = (long) (SamplingConfiguration.MAX_SIZE_HEAP_PERCENTAGE_LIMIT * maxHeap);
5459
return switch (randomIntBetween(0, 4)) {
5560
case 0 -> new SamplingConfiguration(
5661
randomValueOtherThan(instance.rate(), () -> randomDoubleBetween(0.0, 1.0, true)),
@@ -69,10 +74,7 @@ protected SamplingConfiguration mutateInstance(SamplingConfiguration instance) {
6974
case 2 -> new SamplingConfiguration(
7075
instance.rate(),
7176
instance.maxSamples(),
72-
randomValueOtherThan(
73-
instance.maxSize(),
74-
() -> ByteSizeValue.ofGb(randomLongBetween(1, SamplingConfiguration.MAX_SIZE_LIMIT_GIGABYTES))
75-
),
77+
randomValueOtherThan(instance.maxSize(), () -> ByteSizeValue.ofBytes(randomLongBetween(1, maxSizeLimit))),
7678
instance.timeToLive(),
7779
instance.condition()
7880
);
@@ -101,7 +103,11 @@ public void testDefaults() {
101103
SamplingConfiguration config = new SamplingConfiguration(0.5, null, null, null, null);
102104
assertThat(config.rate(), equalTo(0.5));
103105
assertThat(config.maxSamples(), equalTo(SamplingConfiguration.DEFAULT_MAX_SAMPLES));
104-
assertThat(config.maxSize(), equalTo(ByteSizeValue.ofGb(SamplingConfiguration.DEFAULT_MAX_SIZE_GIGABYTES)));
106+
long expectedDefaultMaxSize = Math.max(
107+
(long) (SamplingConfiguration.DEFAULT_MAX_SIZE_HEAP_PERCENTAGE * JvmInfo.jvmInfo().getConfiguredMaxHeapSize()),
108+
SamplingConfiguration.DEFAULT_MAX_SIZE_FLOOR.getBytes()
109+
);
110+
assertThat(config.maxSize(), equalTo(ByteSizeValue.ofBytes(expectedDefaultMaxSize)));
105111
assertThat(config.timeToLive(), equalTo(TimeValue.timeValueDays(SamplingConfiguration.DEFAULT_TIME_TO_LIVE_DAYS)));
106112
assertThat(config.condition(), nullValue());
107113
}
@@ -157,12 +163,15 @@ public void testValidation() throws IOException {
157163
}
158164
""", SamplingConfiguration.INVALID_MAX_SIZE_MIN_MESSAGE);
159165

166+
// Test max size exceeding heap-based limit
167+
long maxHeap = JvmInfo.jvmInfo().getConfiguredMaxHeapSize();
168+
long maxSizeLimit = (long) (SamplingConfiguration.MAX_SIZE_HEAP_PERCENTAGE_LIMIT * maxHeap);
160169
assertValidationError(String.format(Locale.ROOT, """
161170
{
162171
"rate": 0.5,
163-
"max_size": "%dgb"
172+
"max_size_in_bytes": %d
164173
}
165-
""", SamplingConfiguration.MAX_SIZE_LIMIT_GIGABYTES + 1), SamplingConfiguration.INVALID_MAX_SIZE_MAX_MESSAGE);
174+
""", maxSizeLimit + 1), SamplingConfiguration.INVALID_MAX_SIZE_MAX_MESSAGE);
166175

167176
// Test invalid timeToLive
168177
assertValidationError("""
@@ -234,24 +243,17 @@ public void testValidInputs() throws IOException {
234243
assertThat(config.maxSamples(), equalTo(1));
235244

236245
// Test boundary conditions - maximum values
237-
parser = createParser(
238-
JsonXContent.jsonXContent,
239-
String.format(
240-
Locale.ROOT,
241-
"""
242-
{
243-
"rate": 1.0,
244-
"max_samples": %d,
245-
"max_size": "%dgb",
246-
"time_to_live": "%dd",
247-
"if": "test_condition"
248-
}
249-
""",
250-
SamplingConfiguration.MAX_SAMPLES_LIMIT,
251-
SamplingConfiguration.MAX_SIZE_LIMIT_GIGABYTES,
252-
SamplingConfiguration.MAX_TIME_TO_LIVE_DAYS
253-
)
254-
);
246+
long maxHeap = JvmInfo.jvmInfo().getConfiguredMaxHeapSize();
247+
long maxSizeLimit = (long) (SamplingConfiguration.MAX_SIZE_HEAP_PERCENTAGE_LIMIT * maxHeap);
248+
parser = createParser(JsonXContent.jsonXContent, String.format(Locale.ROOT, """
249+
{
250+
"rate": 1.0,
251+
"max_samples": %d,
252+
"max_size_in_bytes": %d,
253+
"time_to_live": "%dd",
254+
"if": "test_condition"
255+
}
256+
""", SamplingConfiguration.MAX_SAMPLES_LIMIT, maxSizeLimit, SamplingConfiguration.MAX_TIME_TO_LIVE_DAYS));
255257
config = SamplingConfiguration.fromXContent(parser);
256258
assertThat(config.rate(), equalTo(1.0));
257259
assertThat(config.maxSamples(), equalTo(SamplingConfiguration.MAX_SAMPLES_LIMIT));
@@ -332,4 +334,20 @@ public void testCreationTimeUserDataRestrictionRaw() throws IOException {
332334
assertNotNull("Expected IllegalArgumentException with creation_time_in_millis message", cause);
333335
assertThat(cause.getMessage(), equalTo("Creation time cannot be set by user (field: creation_time_in_millis)"));
334336
}
337+
338+
public void testMinimumDefaultMaxSize() {
339+
// Test that the minimum default max size is enforced
340+
SamplingConfiguration config = new SamplingConfiguration(0.5, null, null, null, null);
341+
342+
// Calculate what the heap percentage would give us
343+
long heapBasedSize = (long) (SamplingConfiguration.DEFAULT_MAX_SIZE_HEAP_PERCENTAGE * JvmInfo.jvmInfo().getConfiguredMaxHeapSize());
344+
long minSize = SamplingConfiguration.DEFAULT_MAX_SIZE_FLOOR.getBytes();
345+
346+
// The actual default should be the larger of the two
347+
long expectedSize = Math.max(heapBasedSize, minSize);
348+
assertThat(config.maxSize().getBytes(), equalTo(expectedSize));
349+
350+
// Verify it's at least the minimum
351+
assertThat(config.maxSize().getBytes(), greaterThanOrEqualTo(minSize));
352+
}
335353
}

0 commit comments

Comments
 (0)