Disallow negative TimeValues (#53913)

This commit causes negative TimeValues, other than -1 which is sometimes used as
a sentinel value, to be rejected during parsing.

Also introduces a hack to allow ILM to load policies which were written to the
cluster state with a negative min_age, treating those values as 0, which should
match the behavior of prior versions.
This commit is contained in:
Gordon Brown 2020-03-26 13:30:35 -06:00 committed by GitHub
parent 14204f8381
commit 0d30b48613
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 124 additions and 75 deletions

View File

@ -109,7 +109,7 @@ public class CcrStatsResponseTests extends AbstractResponseTestCase<CcrStatsActi
randomNonNegativeLong(), randomNonNegativeLong(),
randomNonNegativeLong(), randomNonNegativeLong(),
Collections.emptyNavigableMap(), Collections.emptyNavigableMap(),
randomLong(), randomNonNegativeLong(),
randomBoolean() ? new ElasticsearchException("fatal error") : null); randomBoolean() ? new ElasticsearchException("fatal error") : null);
responses.add(new FollowStatsAction.StatsResponse(status)); responses.add(new FollowStatsAction.StatsResponse(status));
} }

View File

@ -87,7 +87,7 @@ public class StatsResponseTests extends AbstractResponseTestCase<EnrichStatsActi
String action = randomAlphaOfLength(5); String action = randomAlphaOfLength(5);
String description = randomAlphaOfLength(5); String description = randomAlphaOfLength(5);
long startTime = randomLong(); long startTime = randomLong();
long runningTimeNanos = randomLong(); long runningTimeNanos = randomNonNegativeLong();
boolean cancellable = randomBoolean(); boolean cancellable = randomBoolean();
TaskId parentTaskId = TaskId.EMPTY_TASK_ID; TaskId parentTaskId = TaskId.EMPTY_TASK_ID;
Map<String, String> headers = randomBoolean() ? Map<String, String> headers = randomBoolean() ?

View File

@ -57,7 +57,7 @@ public class IndexLifecycleExplainResponseTests extends AbstractXContentTestCase
boolean stepNull = randomBoolean(); boolean stepNull = randomBoolean();
return IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10), return IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10),
randomAlphaOfLength(10), randomAlphaOfLength(10),
randomBoolean() ? null : randomNonNegativeLong(), randomBoolean() ? null : randomLongBetween(0, System.currentTimeMillis()),
stepNull ? null : randomAlphaOfLength(10), stepNull ? null : randomAlphaOfLength(10),
stepNull ? null : randomAlphaOfLength(10), stepNull ? null : randomAlphaOfLength(10),
stepNull ? null : randomAlphaOfLength(10), stepNull ? null : randomAlphaOfLength(10),

View File

@ -47,6 +47,9 @@ public class TimeValue implements Comparable<TimeValue> {
} }
public TimeValue(long duration, TimeUnit timeUnit) { public TimeValue(long duration, TimeUnit timeUnit) {
if (duration < -1) {
throw new IllegalArgumentException("duration cannot be negative, was given [" + duration + "]");
}
this.duration = duration; this.duration = duration;
this.timeUnit = timeUnit; this.timeUnit = timeUnit;
} }
@ -201,7 +204,7 @@ public class TimeValue implements Comparable<TimeValue> {
* Returns a {@link String} representation of the current {@link TimeValue}. * Returns a {@link String} representation of the current {@link TimeValue}.
* *
* Note that this method might produce fractional time values (ex 1.6m) which cannot be * Note that this method might produce fractional time values (ex 1.6m) which cannot be
* parsed by method like {@link TimeValue#parse(String, String, String)}. * parsed by method like {@link TimeValue#parse(String, String, String, String)}.
* *
* Also note that the maximum string value that will be generated is * Also note that the maximum string value that will be generated is
* {@code 106751.9d} due to the way that values are internally converted * {@code 106751.9d} due to the way that values are internally converted
@ -216,7 +219,7 @@ public class TimeValue implements Comparable<TimeValue> {
* Returns a {@link String} representation of the current {@link TimeValue}. * Returns a {@link String} representation of the current {@link TimeValue}.
* *
* Note that this method might produce fractional time values (ex 1.6m) which cannot be * Note that this method might produce fractional time values (ex 1.6m) which cannot be
* parsed by method like {@link TimeValue#parse(String, String, String)}. The number of * parsed by method like {@link TimeValue#parse(String, String, String, String)}. The number of
* fractional decimals (up to 10 maximum) are truncated to the number of fraction pieces * fractional decimals (up to 10 maximum) are truncated to the number of fraction pieces
* specified. * specified.
* *
@ -358,20 +361,20 @@ public class TimeValue implements Comparable<TimeValue> {
} }
final String normalized = sValue.toLowerCase(Locale.ROOT).trim(); final String normalized = sValue.toLowerCase(Locale.ROOT).trim();
if (normalized.endsWith("nanos")) { if (normalized.endsWith("nanos")) {
return new TimeValue(parse(sValue, normalized, "nanos"), TimeUnit.NANOSECONDS); return new TimeValue(parse(sValue, normalized, "nanos", settingName), TimeUnit.NANOSECONDS);
} else if (normalized.endsWith("micros")) { } else if (normalized.endsWith("micros")) {
return new TimeValue(parse(sValue, normalized, "micros"), TimeUnit.MICROSECONDS); return new TimeValue(parse(sValue, normalized, "micros", settingName), TimeUnit.MICROSECONDS);
} else if (normalized.endsWith("ms")) { } else if (normalized.endsWith("ms")) {
return new TimeValue(parse(sValue, normalized, "ms"), TimeUnit.MILLISECONDS); return new TimeValue(parse(sValue, normalized, "ms", settingName), TimeUnit.MILLISECONDS);
} else if (normalized.endsWith("s")) { } else if (normalized.endsWith("s")) {
return new TimeValue(parse(sValue, normalized, "s"), TimeUnit.SECONDS); return new TimeValue(parse(sValue, normalized, "s", settingName), TimeUnit.SECONDS);
} else if (sValue.endsWith("m")) { } else if (sValue.endsWith("m")) {
// parsing minutes should be case-sensitive as 'M' means "months", not "minutes"; this is the only special case. // parsing minutes should be case-sensitive as 'M' means "months", not "minutes"; this is the only special case.
return new TimeValue(parse(sValue, normalized, "m"), TimeUnit.MINUTES); return new TimeValue(parse(sValue, normalized, "m", settingName), TimeUnit.MINUTES);
} else if (normalized.endsWith("h")) { } else if (normalized.endsWith("h")) {
return new TimeValue(parse(sValue, normalized, "h"), TimeUnit.HOURS); return new TimeValue(parse(sValue, normalized, "h", settingName), TimeUnit.HOURS);
} else if (normalized.endsWith("d")) { } else if (normalized.endsWith("d")) {
return new TimeValue(parse(sValue, normalized, "d"), TimeUnit.DAYS); return new TimeValue(parse(sValue, normalized, "d", settingName), TimeUnit.DAYS);
} else if (normalized.matches("-0*1")) { } else if (normalized.matches("-0*1")) {
return TimeValue.MINUS_ONE; return TimeValue.MINUS_ONE;
} else if (normalized.matches("0+")) { } else if (normalized.matches("0+")) {
@ -383,10 +386,16 @@ public class TimeValue implements Comparable<TimeValue> {
} }
} }
private static long parse(final String initialInput, final String normalized, final String suffix) { private static long parse(final String initialInput, final String normalized, final String suffix, String settingName) {
final String s = normalized.substring(0, normalized.length() - suffix.length()).trim(); final String s = normalized.substring(0, normalized.length() - suffix.length()).trim();
try { try {
return Long.parseLong(s); final long value = Long.parseLong(s);
if (value < -1) {
// -1 is magic, but reject any other negative values
throw new IllegalArgumentException("failed to parse setting [" + settingName + "] with value [" + initialInput +
"] as a time value: negative durations are not supported");
}
return value;
} catch (final NumberFormatException e) { } catch (final NumberFormatException e) {
try { try {
@SuppressWarnings("unused") final double ignored = Double.parseDouble(s); @SuppressWarnings("unused") final double ignored = Double.parseDouble(s);

View File

@ -238,4 +238,26 @@ public class TimeValueTests extends ESTestCase {
TimeValue secondValue = new TimeValue(firstValue.getSeconds(), TimeUnit.SECONDS); TimeValue secondValue = new TimeValue(firstValue.getSeconds(), TimeUnit.SECONDS);
assertEquals(firstValue.hashCode(), secondValue.hashCode()); assertEquals(firstValue.hashCode(), secondValue.hashCode());
} }
public void testRejectsNegativeValuesDuringParsing() {
final String settingName = "test-value";
final long negativeValue = randomLongBetween(Long.MIN_VALUE, -2);
final String negativeTimeValueString = Long.toString(negativeValue) + randomTimeUnit();
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
() -> TimeValue.parseTimeValue(negativeTimeValueString, settingName));
assertThat(ex.getMessage(),
equalTo("failed to parse setting [" + settingName + "] with value [" + negativeTimeValueString +
"] as a time value: negative durations are not supported"));
}
public void testRejectsNegativeValuesAtCreation() {
final long duration = randomLongBetween(Long.MIN_VALUE, -2);
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new TimeValue(duration, randomTimeUnitObject()));
assertThat(ex.getMessage(), containsString("duration cannot be negative"));
}
private TimeUnit randomTimeUnitObject() {
return randomFrom(TimeUnit.NANOSECONDS, TimeUnit.MICROSECONDS, TimeUnit.MILLISECONDS, TimeUnit.SECONDS,
TimeUnit.MINUTES, TimeUnit.HOURS, TimeUnit.DAYS);
}
} }

View File

@ -133,7 +133,9 @@ public class IndexingStats implements Writeable, ToXContentFragment {
/** /**
* The total amount of time spend on executing delete operations. * The total amount of time spend on executing delete operations.
*/ */
public TimeValue getDeleteTime() { return new TimeValue(deleteTimeInMillis); } public TimeValue getDeleteTime() {
return new TimeValue(deleteTimeInMillis);
}
/** /**
* Returns the currently in-flight delete operations * Returns the currently in-flight delete operations

View File

@ -28,8 +28,8 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.monitor.jvm.JvmStats.GarbageCollector; import org.elasticsearch.monitor.jvm.JvmStats.GarbageCollector;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.threadpool.Scheduler.Cancellable; import org.elasticsearch.threadpool.Scheduler.Cancellable;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.threadpool.ThreadPool.Names; import org.elasticsearch.threadpool.ThreadPool.Names;
import java.util.HashMap; import java.util.HashMap;
@ -165,13 +165,20 @@ public class JvmGcMonitorService extends AbstractLifecycleComponent {
} }
private static TimeValue getValidThreshold(Settings settings, String key, String level) { private static TimeValue getValidThreshold(Settings settings, String key, String level) {
TimeValue threshold = settings.getAsTime(level, null); final TimeValue threshold;
try {
threshold = settings.getAsTime(level, null);
} catch (RuntimeException ex) {
final String settingValue = settings.get(level);
throw new IllegalArgumentException("failed to parse setting [" + getThresholdName(key, level) + "] with value [" +
settingValue + "] as a time value", ex);
}
if (threshold == null) { if (threshold == null) {
throw new IllegalArgumentException("missing gc_threshold for [" + getThresholdName(key, level) + "]"); throw new IllegalArgumentException("missing gc_threshold for [" + getThresholdName(key, level) + "]");
} }
if (threshold.nanos() <= 0) {
throw new IllegalArgumentException("invalid gc_threshold [" + threshold + "] for [" + getThresholdName(key, level) + "]");
}
return threshold; return threshold;
} }

View File

@ -530,7 +530,7 @@ public final class SnapshotInfo implements Comparable<SnapshotInfo>, ToXContent,
if (verbose || endTime != 0) { if (verbose || endTime != 0) {
builder.field(END_TIME, DATE_TIME_FORMATTER.format(Instant.ofEpochMilli(endTime).atZone(ZoneOffset.UTC))); builder.field(END_TIME, DATE_TIME_FORMATTER.format(Instant.ofEpochMilli(endTime).atZone(ZoneOffset.UTC)));
builder.field(END_TIME_IN_MILLIS, endTime); builder.field(END_TIME_IN_MILLIS, endTime);
builder.humanReadableField(DURATION_IN_MILLIS, DURATION, new TimeValue(endTime - startTime)); builder.humanReadableField(DURATION_IN_MILLIS, DURATION, new TimeValue(Math.max(0L, endTime - startTime)));
} }
if (verbose || !shardFailures.isEmpty()) { if (verbose || !shardFailures.isEmpty()) {
builder.startArray(FAILURES); builder.startArray(FAILURES);

View File

@ -94,12 +94,12 @@ public class SearchResponseMergerTests extends ESTestCase {
} }
public void testMergeTookInMillis() throws InterruptedException { public void testMergeTookInMillis() throws InterruptedException {
long currentRelativeTime = randomLong(); long currentRelativeTime = randomNonNegativeLong();
SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime); SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime);
SearchResponseMerger merger = new SearchResponseMerger(randomIntBetween(0, 1000), randomIntBetween(0, 10000), SearchResponseMerger merger = new SearchResponseMerger(randomIntBetween(0, 1000), randomIntBetween(0, 10000),
SearchContext.TRACK_TOTAL_HITS_ACCURATE, timeProvider, emptyReduceContextBuilder()); SearchContext.TRACK_TOTAL_HITS_ACCURATE, timeProvider, emptyReduceContextBuilder());
for (int i = 0; i < numResponses; i++) { for (int i = 0; i < numResponses; i++) {
SearchResponse searchResponse = new SearchResponse(InternalSearchResponse.empty(), null, 1, 1, 0, randomLong(), SearchResponse searchResponse = new SearchResponse(InternalSearchResponse.empty(), null, 1, 1, 0, randomNonNegativeLong(),
ShardSearchFailure.EMPTY_ARRAY, SearchResponseTests.randomClusters()); ShardSearchFailure.EMPTY_ARRAY, SearchResponseTests.randomClusters());
addResponse(merger, searchResponse); addResponse(merger, searchResponse);
} }
@ -399,7 +399,7 @@ public class SearchResponseMergerTests extends ESTestCase {
} }
public void testMergeSearchHits() throws InterruptedException { public void testMergeSearchHits() throws InterruptedException {
final long currentRelativeTime = randomLong(); final long currentRelativeTime = randomNonNegativeLong();
final SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime); final SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime);
final int size = randomIntBetween(0, 100); final int size = randomIntBetween(0, 100);
final int from = size > 0 ? randomIntBetween(0, 100) : 0; final int from = size > 0 ? randomIntBetween(0, 100) : 0;
@ -557,7 +557,7 @@ public class SearchResponseMergerTests extends ESTestCase {
} }
public void testMergeNoResponsesAdded() { public void testMergeNoResponsesAdded() {
long currentRelativeTime = randomLong(); long currentRelativeTime = randomNonNegativeLong();
final SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime); final SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime);
SearchResponseMerger merger = new SearchResponseMerger(0, 10, Integer.MAX_VALUE, timeProvider, emptyReduceContextBuilder()); SearchResponseMerger merger = new SearchResponseMerger(0, 10, Integer.MAX_VALUE, timeProvider, emptyReduceContextBuilder());
SearchResponse.Clusters clusters = SearchResponseTests.randomClusters(); SearchResponse.Clusters clusters = SearchResponseTests.randomClusters();

View File

@ -129,8 +129,8 @@ public class RoundingTests extends ESTestCase {
Rounding tzRounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH) Rounding tzRounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH)
.timeZone(ZoneOffset.ofHours(timezoneOffset)).build(); .timeZone(ZoneOffset.ofHours(timezoneOffset)).build();
assertThat(tzRounding.round(0), equalTo(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis())); assertThat(tzRounding.round(0), equalTo(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()));
assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(0L - TimeValue assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(TimeValue
.timeValueHours(timezoneOffset).millis())); .timeValueHours(-timezoneOffset).millis()));
ZoneId tz = ZoneId.of("-08:00"); ZoneId tz = ZoneId.of("-08:00");
tzRounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); tzRounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build();

View File

@ -128,8 +128,8 @@ public class TimeZoneRoundingTests extends ESTestCase {
Rounding tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(DateTimeZone.forOffsetHours(timezoneOffset)) Rounding tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(DateTimeZone.forOffsetHours(timezoneOffset))
.build(); .build();
assertThat(tzRounding.round(0), equalTo(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis())); assertThat(tzRounding.round(0), equalTo(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()));
assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(0L - TimeValue assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(TimeValue
.timeValueHours(timezoneOffset).millis())); .timeValueHours(-timezoneOffset).millis()));
DateTimeZone tz = DateTimeZone.forID("-08:00"); DateTimeZone tz = DateTimeZone.forID("-08:00");
tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build();

View File

@ -253,7 +253,8 @@ public class IndexingMemoryControllerTests extends IndexShardTestCase {
Exception e = expectThrows(IllegalArgumentException.class, Exception e = expectThrows(IllegalArgumentException.class,
() -> new MockController(Settings.builder() () -> new MockController(Settings.builder()
.put("indices.memory.interval", "-42s").build())); .put("indices.memory.interval", "-42s").build()));
assertEquals("failed to parse value [-42s] for setting [indices.memory.interval], must be >= [0ms]", e.getMessage()); assertEquals("failed to parse setting [indices.memory.interval] with value " +
"[-42s] as a time value: negative durations are not supported", e.getMessage());
} }
@ -261,7 +262,8 @@ public class IndexingMemoryControllerTests extends IndexShardTestCase {
Exception e = expectThrows(IllegalArgumentException.class, Exception e = expectThrows(IllegalArgumentException.class,
() -> new MockController(Settings.builder() () -> new MockController(Settings.builder()
.put("indices.memory.shard_inactive_time", "-42s").build())); .put("indices.memory.shard_inactive_time", "-42s").build()));
assertEquals("failed to parse value [-42s] for setting [indices.memory.shard_inactive_time], must be >= [0ms]", e.getMessage()); assertEquals("failed to parse setting [indices.memory.shard_inactive_time] with value " +
"[-42s] as a time value: negative durations are not supported", e.getMessage());
} }

View File

@ -33,8 +33,8 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer; import java.util.function.Consumer;
import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.instanceOf;
public class JvmGcMonitorServiceSettingsTests extends ESTestCase { public class JvmGcMonitorServiceSettingsTests extends ESTestCase {
@ -62,11 +62,12 @@ public class JvmGcMonitorServiceSettingsTests extends ESTestCase {
public void testNegativeSetting() throws InterruptedException { public void testNegativeSetting() throws InterruptedException {
String collector = randomAlphaOfLength(5); String collector = randomAlphaOfLength(5);
Settings settings = Settings.builder().put("monitor.jvm.gc.collector." + collector + ".warn", "-" + randomTimeValue()).build(); final String timeValue = "-" + randomTimeValue();
Settings settings = Settings.builder().put("monitor.jvm.gc.collector." + collector + ".warn", timeValue).build();
execute(settings, (command, interval, name) -> null, e -> { execute(settings, (command, interval, name) -> null, e -> {
assertThat(e, instanceOf(IllegalArgumentException.class)); assertThat(e, instanceOf(IllegalArgumentException.class));
assertThat(e.getMessage(), allOf(containsString("invalid gc_threshold"), assertThat(e.getMessage(), equalTo("failed to parse setting [monitor.jvm.gc.collector." + collector + ".warn] " +
containsString("for [monitor.jvm.gc.collector." + collector + "."))); "with value [" + timeValue + "] as a time value"));
}, true, null); }, true, null);
} }

View File

@ -61,7 +61,7 @@ public class ShardFollowNodeTaskStatusTests extends AbstractSerializingTestCase<
randomNonNegativeLong(), randomNonNegativeLong(),
randomNonNegativeLong(), randomNonNegativeLong(),
randomReadExceptions(), randomReadExceptions(),
randomLong(), randomNonNegativeLong(),
randomBoolean() ? new ElasticsearchException("fatal error") : null); randomBoolean() ? new ElasticsearchException("fatal error") : null);
} }

View File

@ -59,7 +59,7 @@ public class StatsResponsesTests extends AbstractWireSerializingTestCase<FollowS
randomNonNegativeLong(), randomNonNegativeLong(),
randomNonNegativeLong(), randomNonNegativeLong(),
Collections.emptyNavigableMap(), Collections.emptyNavigableMap(),
randomLong(), randomNonNegativeLong(),
randomBoolean() ? new ElasticsearchException("fatal error") : null); randomBoolean() ? new ElasticsearchException("fatal error") : null);
responses.add(new FollowStatsAction.StatsResponse(status)); responses.add(new FollowStatsAction.StatsResponse(status));
} }

View File

@ -5,6 +5,9 @@
*/ */
package org.elasticsearch.xpack.core.ilm; package org.elasticsearch.xpack.core.ilm;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.Version;
import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings; import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamInput;
@ -12,6 +15,7 @@ import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ContextParser;
import org.elasticsearch.common.xcontent.ObjectParser.ValueType; import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder;
@ -30,6 +34,7 @@ import java.util.stream.Collectors;
* particular point in the lifecycle of an index. * particular point in the lifecycle of an index.
*/ */
public class Phase implements ToXContentObject, Writeable { public class Phase implements ToXContentObject, Writeable {
private static final Logger logger = LogManager.getLogger(Phase.class);
public static final ParseField MIN_AGE = new ParseField("min_age"); public static final ParseField MIN_AGE = new ParseField("min_age");
public static final ParseField ACTIONS_FIELD = new ParseField("actions"); public static final ParseField ACTIONS_FIELD = new ParseField("actions");
@ -40,7 +45,20 @@ public class Phase implements ToXContentObject, Writeable {
.collect(Collectors.toMap(LifecycleAction::getWriteableName, Function.identity())))); .collect(Collectors.toMap(LifecycleAction::getWriteableName, Function.identity()))));
static { static {
PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(), PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(),
(p, c) -> TimeValue.parseTimeValue(p.text(), MIN_AGE.getPreferredName()), MIN_AGE, ValueType.VALUE); (ContextParser<String, Object>) (p, c) -> {
// In earlier versions it was possible to create a Phase with a negative `min_age` which would then cause errors
// when the phase is read from the cluster state during startup (even before negative timevalues were strictly
// disallowed) so this is a hack to treat negative `min_age`s as 0 to prevent those errors.
// They will be saved as `0` so this hack can be removed once we no longer have to read cluster states from 7.x.
assert Version.CURRENT.major < 9 : "remove this hack now that we don't have to read 7.x cluster states";
final String timeValueString = p.text();
if (timeValueString.startsWith("-")) {
logger.warn("phase has negative min_age value of [{}] - this will be treated as a min_age of 0",
timeValueString);
return TimeValue.ZERO;
}
return TimeValue.parseTimeValue(timeValueString, MIN_AGE.getPreferredName());
}, MIN_AGE, ValueType.VALUE);
PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(), PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(),
(p, c, n) -> p.namedObject(LifecycleAction.class, n, null), v -> { (p, c, n) -> p.namedObject(LifecycleAction.class, n, null), v -> {
throw new IllegalArgumentException("ordered " + ACTIONS_FIELD.getPreferredName() + " are not supported"); throw new IllegalArgumentException("ordered " + ACTIONS_FIELD.getPreferredName() + " are not supported");

View File

@ -47,7 +47,7 @@ public class IndexLifecycleExplainResponseTests extends AbstractSerializingTestC
boolean stepNull = randomBoolean(); boolean stepNull = randomBoolean();
return IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10), return IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10),
randomAlphaOfLength(10), randomAlphaOfLength(10),
randomBoolean() ? null : randomNonNegativeLong(), randomBoolean() ? null : randomLongBetween(0, System.currentTimeMillis()),
stepNull ? null : randomAlphaOfLength(10), stepNull ? null : randomAlphaOfLength(10),
stepNull ? null : randomAlphaOfLength(10), stepNull ? null : randomAlphaOfLength(10),
stepNull ? null : randomAlphaOfLength(10), stepNull ? null : randomAlphaOfLength(10),

View File

@ -62,8 +62,8 @@ import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map; import java.util.Map;
import static org.elasticsearch.xpack.core.ml.utils.QueryProviderTests.createRandomValidQueryProvider;
import static org.elasticsearch.xpack.core.ml.job.messages.Messages.DATAFEED_AGGREGATIONS_INTERVAL_MUST_BE_GREATER_THAN_ZERO; import static org.elasticsearch.xpack.core.ml.job.messages.Messages.DATAFEED_AGGREGATIONS_INTERVAL_MUST_BE_GREATER_THAN_ZERO;
import static org.elasticsearch.xpack.core.ml.utils.QueryProviderTests.createRandomValidQueryProvider;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo;
@ -468,8 +468,8 @@ public class DatafeedConfigTests extends AbstractSerializingTestCase<DatafeedCon
public void testCheckValid_GivenNegativeQueryDelay() { public void testCheckValid_GivenNegativeQueryDelay() {
DatafeedConfig.Builder conf = new DatafeedConfig.Builder("datafeed1", "job1"); DatafeedConfig.Builder conf = new DatafeedConfig.Builder("datafeed1", "job1");
IllegalArgumentException e = ESTestCase.expectThrows(IllegalArgumentException.class, IllegalArgumentException e = ESTestCase.expectThrows(IllegalArgumentException.class,
() -> conf.setQueryDelay(TimeValue.timeValueMillis(-10))); () -> conf.setQueryDelay(TimeValue.timeValueMillis(-1)));
assertEquals("query_delay cannot be less than 0. Value = -10", e.getMessage()); assertEquals("query_delay cannot be less than 0. Value = -1", e.getMessage());
} }
public void testCheckValid_GivenZeroFrequency() { public void testCheckValid_GivenZeroFrequency() {

View File

@ -182,7 +182,7 @@ public class SnapshotRetentionConfigurationTests extends ESTestCase {
} }
private void assertUnsuccessfulNotCountedTowardsMaximum(boolean failure) { private void assertUnsuccessfulNotCountedTowardsMaximum(boolean failure) {
SnapshotRetentionConfiguration conf = new SnapshotRetentionConfiguration(() -> 1, TimeValue.timeValueDays(1), 2, 2); SnapshotRetentionConfiguration conf = new SnapshotRetentionConfiguration(() -> 5, TimeValue.timeValueDays(1), 2, 2);
SnapshotInfo s1 = makeInfo(1); SnapshotInfo s1 = makeInfo(1);
SnapshotInfo s2 = makeFailureOrPartial(2, failure); SnapshotInfo s2 = makeFailureOrPartial(2, failure);
SnapshotInfo s3 = makeFailureOrPartial(3, failure); SnapshotInfo s3 = makeFailureOrPartial(3, failure);

View File

@ -54,7 +54,7 @@ public class EnrichStatsResponseTests extends AbstractWireSerializingTestCase<En
String action = randomAlphaOfLength(5); String action = randomAlphaOfLength(5);
String description = randomAlphaOfLength(5); String description = randomAlphaOfLength(5);
long startTime = randomLong(); long startTime = randomLong();
long runningTimeNanos = randomLong(); long runningTimeNanos = randomNonNegativeLong();
boolean cancellable = randomBoolean(); boolean cancellable = randomBoolean();
TaskId parentTaskId = TaskId.EMPTY_TASK_ID; TaskId parentTaskId = TaskId.EMPTY_TASK_ID;
Map<String, String> headers = randomBoolean() Map<String, String> headers = randomBoolean()

View File

@ -93,14 +93,22 @@ class IndexLifecycleRunner {
} }
final TimeValue after = stepRegistry.getIndexAgeForPhase(policy, phase); final TimeValue after = stepRegistry.getIndexAgeForPhase(policy, phase);
final long now = nowSupplier.getAsLong(); final long now = nowSupplier.getAsLong();
final TimeValue age = new TimeValue(now - lifecycleDate); final long ageMillis = now - lifecycleDate;
final TimeValue age;
if (ageMillis >= 0) {
age = new TimeValue(ageMillis);
} else if (ageMillis == Long.MIN_VALUE) {
age = new TimeValue(Long.MAX_VALUE);
} else {
age = new TimeValue(-ageMillis);
}
if (logger.isTraceEnabled()) { if (logger.isTraceEnabled()) {
logger.trace("[{}] checking for index age to be at least [{}] before performing actions in " + logger.trace("[{}] checking for index age to be at least [{}] before performing actions in " +
"the \"{}\" phase. Now: {}, lifecycle date: {}, age: [{}/{}s]", "the \"{}\" phase. Now: {}, lifecycle date: {}, age: [{}{}/{}s]",
indexMetaData.getIndex().getName(), after, phase, indexMetaData.getIndex().getName(), after, phase,
new TimeValue(now).seconds(), new TimeValue(now).seconds(),
new TimeValue(lifecycleDate).seconds(), new TimeValue(lifecycleDate).seconds(),
age, age.seconds()); ageMillis < 0 ? "-" : "", age, age.seconds());
} }
return now >= lifecycleDate + after.getMillis(); return now >= lifecycleDate + after.getMillis();
} }

View File

@ -236,13 +236,15 @@ public class SnapshotLifecycleTaskTests extends ESTestCase {
Boolean.parseBoolean((String) policy.getConfig().get("include_global_state")); Boolean.parseBoolean((String) policy.getConfig().get("include_global_state"));
assertThat(req.includeGlobalState(), equalTo(globalState)); assertThat(req.includeGlobalState(), equalTo(globalState));
long startTime = randomNonNegativeLong();
long endTime = randomLongBetween(startTime, Long.MAX_VALUE);
return new CreateSnapshotResponse( return new CreateSnapshotResponse(
new SnapshotInfo( new SnapshotInfo(
new SnapshotId(req.snapshot(), "uuid"), new SnapshotId(req.snapshot(), "uuid"),
Arrays.asList(req.indices()), Arrays.asList(req.indices()),
randomNonNegativeLong(), startTime,
"snapshot started", "snapshot started",
randomNonNegativeLong(), endTime,
3, 3,
Collections.singletonList( Collections.singletonList(
new SnapshotShardFailure("nodeId", new ShardId("index", "uuid", 0), "forced failure")), new SnapshotShardFailure("nodeId", new ShardId("index", "uuid", 0), "forced failure")),

View File

@ -143,14 +143,14 @@ public class IndicesStatsMonitoringDocTests extends BaseFilteredMonitoringDocTes
private CommonStats mockCommonStats() { private CommonStats mockCommonStats() {
final CommonStats commonStats = new CommonStats(CommonStatsFlags.ALL); final CommonStats commonStats = new CommonStats(CommonStatsFlags.ALL);
commonStats.getDocs().add(new DocsStats(1L, -1L, randomNonNegativeLong())); commonStats.getDocs().add(new DocsStats(1L, 0L, randomNonNegativeLong()));
commonStats.getStore().add(new StoreStats(2L)); commonStats.getStore().add(new StoreStats(2L));
final IndexingStats.Stats indexingStats = new IndexingStats.Stats(3L, 4L, -1L, -1L, -1L, -1L, -1L, -1L, true, 5L); final IndexingStats.Stats indexingStats = new IndexingStats.Stats(3L, 4L, 0L, 0L, 0L, 0L, 0L, 0L, true, 5L);
commonStats.getIndexing().add(new IndexingStats(indexingStats, null)); commonStats.getIndexing().add(new IndexingStats(indexingStats, null));
final SearchStats.Stats searchStats = new SearchStats.Stats(6L, 7L, -1L, -1L, -1L, -1L, -1L, -1L, -1L, -1L, -1L, -1L); final SearchStats.Stats searchStats = new SearchStats.Stats(6L, 7L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L);
commonStats.getSearch().add(new SearchStats(searchStats, -1L, null)); commonStats.getSearch().add(new SearchStats(searchStats, 0L, null));
return commonStats; return commonStats;
} }

View File

@ -89,28 +89,6 @@ public class WatcherDateTimeUtilsTests extends ESTestCase {
assertThat(parsed.millis(), is(values.get(key).millis())); assertThat(parsed.millis(), is(values.get(key).millis()));
} }
public void testParseTimeValueStringNegative() throws Exception {
int value = -1 * randomIntBetween(2, 200);
Map<String, TimeValue> values = new HashMap<>();
values.put(value + "s", TimeValue.timeValueSeconds(value));
values.put(value + "m", TimeValue.timeValueMinutes(value));
values.put(value + "h", TimeValue.timeValueHours(value));
String key = randomFrom(values.keySet().toArray(new String[values.size()]));
XContentParser parser = createParser(jsonBuilder().startObject().field("value", key).endObject());
parser.nextToken(); // start object
parser.nextToken(); // field name
parser.nextToken(); // value
try {
WatcherDateTimeUtils.parseTimeValue(parser, "test");
fail("Expected ElasticsearchParseException");
} catch (ElasticsearchParseException e) {
assertThat(e.getMessage(), is("failed to parse time unit"));
}
}
public void testParseTimeValueNull() throws Exception { public void testParseTimeValueNull() throws Exception {
XContentParser parser = createParser(jsonBuilder().startObject().nullField("value").endObject()); XContentParser parser = createParser(jsonBuilder().startObject().nullField("value").endObject());
parser.nextToken(); // start object parser.nextToken(); // start object