diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ccr/CcrStatsResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ccr/CcrStatsResponseTests.java index a687ea45f2023..6b4ef517577d5 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ccr/CcrStatsResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ccr/CcrStatsResponseTests.java @@ -109,7 +109,7 @@ static FollowStatsAction.StatsResponses createStatsResponse() { randomNonNegativeLong(), randomNonNegativeLong(), Collections.emptyNavigableMap(), - randomLong(), + randomNonNegativeLong(), randomBoolean() ? new ElasticsearchException("fatal error") : null); responses.add(new FollowStatsAction.StatsResponse(status)); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/enrich/StatsResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/enrich/StatsResponseTests.java index aac22348abb6d..efda1ab5ed4ea 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/enrich/StatsResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/enrich/StatsResponseTests.java @@ -87,7 +87,7 @@ private static TaskInfo randomTaskInfo() { String action = randomAlphaOfLength(5); String description = randomAlphaOfLength(5); long startTime = randomLong(); - long runningTimeNanos = randomLong(); + long runningTimeNanos = randomNonNegativeLong(); boolean cancellable = randomBoolean(); TaskId parentTaskId = TaskId.EMPTY_TASK_ID; Map headers = randomBoolean() ? diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java index 1cf9bb523a867..7e41dd55dcef2 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java @@ -57,7 +57,7 @@ private static IndexLifecycleExplainResponse randomManagedIndexExplainResponse() boolean stepNull = randomBoolean(); return IndexLifecycleExplainResponse.newManagedIndexResponse(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), diff --git a/libs/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java b/libs/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java index edca86637e1b5..b2f9128673eb4 100644 --- a/libs/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java +++ b/libs/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java @@ -47,6 +47,9 @@ public TimeValue(long millis) { } public TimeValue(long duration, TimeUnit timeUnit) { + if (duration < -1) { + throw new IllegalArgumentException("duration cannot be negative, was given [" + duration + "]"); + } this.duration = duration; this.timeUnit = timeUnit; } @@ -201,7 +204,7 @@ public double getDaysFrac() { * 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 - * 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 * {@code 106751.9d} due to the way that values are internally converted @@ -216,7 +219,7 @@ public String toString() { * 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 - * 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 * specified. * @@ -358,20 +361,20 @@ public static TimeValue parseTimeValue(String sValue, TimeValue defaultValue, St } final String normalized = sValue.toLowerCase(Locale.ROOT).trim(); 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")) { - return new TimeValue(parse(sValue, normalized, "micros"), TimeUnit.MICROSECONDS); + return new TimeValue(parse(sValue, normalized, "micros", settingName), TimeUnit.MICROSECONDS); } 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")) { - return new TimeValue(parse(sValue, normalized, "s"), TimeUnit.SECONDS); + return new TimeValue(parse(sValue, normalized, "s", settingName), TimeUnit.SECONDS); } else if (sValue.endsWith("m")) { // 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")) { - return new TimeValue(parse(sValue, normalized, "h"), TimeUnit.HOURS); + return new TimeValue(parse(sValue, normalized, "h", settingName), TimeUnit.HOURS); } 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")) { return TimeValue.MINUS_ONE; } else if (normalized.matches("0+")) { @@ -383,10 +386,16 @@ public static TimeValue parseTimeValue(String sValue, TimeValue defaultValue, St } } - 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(); 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) { try { @SuppressWarnings("unused") final double ignored = Double.parseDouble(s); diff --git a/libs/core/src/test/java/org/elasticsearch/common/unit/TimeValueTests.java b/libs/core/src/test/java/org/elasticsearch/common/unit/TimeValueTests.java index 3dec990b7b251..5ee32099115d9 100644 --- a/libs/core/src/test/java/org/elasticsearch/common/unit/TimeValueTests.java +++ b/libs/core/src/test/java/org/elasticsearch/common/unit/TimeValueTests.java @@ -238,4 +238,26 @@ public void testConversionHashCode() { TimeValue secondValue = new TimeValue(firstValue.getSeconds(), TimeUnit.SECONDS); 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); + } } diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexingStats.java b/server/src/main/java/org/elasticsearch/index/shard/IndexingStats.java index 17ac5a90e31a3..804697ac24fda 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexingStats.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexingStats.java @@ -133,7 +133,9 @@ public long getDeleteCount() { /** * 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 diff --git a/server/src/main/java/org/elasticsearch/monitor/jvm/JvmGcMonitorService.java b/server/src/main/java/org/elasticsearch/monitor/jvm/JvmGcMonitorService.java index 70de50710c999..c6a554e6374d7 100644 --- a/server/src/main/java/org/elasticsearch/monitor/jvm/JvmGcMonitorService.java +++ b/server/src/main/java/org/elasticsearch/monitor/jvm/JvmGcMonitorService.java @@ -28,8 +28,8 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.monitor.jvm.JvmStats.GarbageCollector; -import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.Scheduler.Cancellable; +import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.ThreadPool.Names; import java.util.HashMap; @@ -165,13 +165,20 @@ public JvmGcMonitorService(Settings settings, ThreadPool threadPool) { } 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) { 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; } diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java index b7fbfeb6d9763..ab3973e7b1b38 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java @@ -530,7 +530,7 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa if (verbose || endTime != 0) { builder.field(END_TIME, DATE_TIME_FORMATTER.format(Instant.ofEpochMilli(endTime).atZone(ZoneOffset.UTC))); 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()) { builder.startArray(FAILURES); diff --git a/server/src/test/java/org/elasticsearch/action/search/SearchResponseMergerTests.java b/server/src/test/java/org/elasticsearch/action/search/SearchResponseMergerTests.java index df04549ddee88..5d5c56751d65c 100644 --- a/server/src/test/java/org/elasticsearch/action/search/SearchResponseMergerTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/SearchResponseMergerTests.java @@ -94,12 +94,12 @@ private void awaitResponsesAdded() throws InterruptedException { } public void testMergeTookInMillis() throws InterruptedException { - long currentRelativeTime = randomLong(); + long currentRelativeTime = randomNonNegativeLong(); SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime); SearchResponseMerger merger = new SearchResponseMerger(randomIntBetween(0, 1000), randomIntBetween(0, 10000), SearchContext.TRACK_TOTAL_HITS_ACCURATE, timeProvider, emptyReduceContextBuilder()); 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()); addResponse(merger, searchResponse); } @@ -399,7 +399,7 @@ public void testMergeAggs() throws InterruptedException { } public void testMergeSearchHits() throws InterruptedException { - final long currentRelativeTime = randomLong(); + final long currentRelativeTime = randomNonNegativeLong(); final SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime); final int size = randomIntBetween(0, 100); final int from = size > 0 ? randomIntBetween(0, 100) : 0; @@ -557,7 +557,7 @@ public void testMergeSearchHits() throws InterruptedException { } public void testMergeNoResponsesAdded() { - long currentRelativeTime = randomLong(); + long currentRelativeTime = randomNonNegativeLong(); final SearchTimeProvider timeProvider = new SearchTimeProvider(randomLong(), 0, () -> currentRelativeTime); SearchResponseMerger merger = new SearchResponseMerger(0, 10, Integer.MAX_VALUE, timeProvider, emptyReduceContextBuilder()); SearchResponse.Clusters clusters = SearchResponseTests.randomClusters(); diff --git a/server/src/test/java/org/elasticsearch/common/RoundingTests.java b/server/src/test/java/org/elasticsearch/common/RoundingTests.java index 8e19bdaf5547a..64537b2b008b8 100644 --- a/server/src/test/java/org/elasticsearch/common/RoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/RoundingTests.java @@ -129,8 +129,8 @@ public void testDayRounding() { Rounding tzRounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH) .timeZone(ZoneOffset.ofHours(timezoneOffset)).build(); assertThat(tzRounding.round(0), equalTo(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis())); - assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(0L - TimeValue - .timeValueHours(timezoneOffset).millis())); + assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(TimeValue + .timeValueHours(-timezoneOffset).millis())); ZoneId tz = ZoneId.of("-08:00"); tzRounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); diff --git a/server/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java b/server/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java index 029eb3b041d3d..c28ed44aefa72 100644 --- a/server/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java @@ -128,8 +128,8 @@ public void testDayRounding() { Rounding tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(DateTimeZone.forOffsetHours(timezoneOffset)) .build(); assertThat(tzRounding.round(0), equalTo(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis())); - assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(0L - TimeValue - .timeValueHours(timezoneOffset).millis())); + assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(TimeValue + .timeValueHours(-timezoneOffset).millis())); DateTimeZone tz = DateTimeZone.forID("-08:00"); tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); diff --git a/server/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java b/server/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java index ccef3aa11e80b..537a068a37328 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java @@ -253,7 +253,8 @@ public void testNegativeInterval() { Exception e = expectThrows(IllegalArgumentException.class, () -> new MockController(Settings.builder() .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 void testNegativeShardInactiveTime() { Exception e = expectThrows(IllegalArgumentException.class, () -> new MockController(Settings.builder() .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()); } diff --git a/server/src/test/java/org/elasticsearch/monitor/jvm/JvmGcMonitorServiceSettingsTests.java b/server/src/test/java/org/elasticsearch/monitor/jvm/JvmGcMonitorServiceSettingsTests.java index 5b60c82feb14f..4e4ae1cb664c0 100644 --- a/server/src/test/java/org/elasticsearch/monitor/jvm/JvmGcMonitorServiceSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/monitor/jvm/JvmGcMonitorServiceSettingsTests.java @@ -33,8 +33,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; -import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; public class JvmGcMonitorServiceSettingsTests extends ESTestCase { @@ -62,11 +62,12 @@ public void testDisabledSetting() throws InterruptedException { public void testNegativeSetting() throws InterruptedException { 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 -> { assertThat(e, instanceOf(IllegalArgumentException.class)); - assertThat(e.getMessage(), allOf(containsString("invalid gc_threshold"), - containsString("for [monitor.jvm.gc.collector." + collector + "."))); + assertThat(e.getMessage(), equalTo("failed to parse setting [monitor.jvm.gc.collector." + collector + ".warn] " + + "with value [" + timeValue + "] as a time value")); }, true, null); } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/ShardFollowNodeTaskStatusTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/ShardFollowNodeTaskStatusTests.java index 413960b69c834..7b56b00cd6260 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/ShardFollowNodeTaskStatusTests.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/ShardFollowNodeTaskStatusTests.java @@ -61,7 +61,7 @@ protected ShardFollowNodeTaskStatus createTestInstance() { randomNonNegativeLong(), randomNonNegativeLong(), randomReadExceptions(), - randomLong(), + randomNonNegativeLong(), randomBoolean() ? new ElasticsearchException("fatal error") : null); } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/StatsResponsesTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/StatsResponsesTests.java index 4e9aadf8d82a8..3707b5ab4542d 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/StatsResponsesTests.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/StatsResponsesTests.java @@ -59,7 +59,7 @@ static FollowStatsAction.StatsResponses createStatsResponse() { randomNonNegativeLong(), randomNonNegativeLong(), Collections.emptyNavigableMap(), - randomLong(), + randomNonNegativeLong(), randomBoolean() ? new ElasticsearchException("fatal error") : null); responses.add(new FollowStatsAction.StatsResponse(status)); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/Phase.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/Phase.java index b2209d0956676..3324e3f35a5bf 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/Phase.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/Phase.java @@ -5,6 +5,9 @@ */ 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.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -12,6 +15,7 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ContextParser; import org.elasticsearch.common.xcontent.ObjectParser.ValueType; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -30,6 +34,7 @@ * particular point in the lifecycle of an index. */ 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 ACTIONS_FIELD = new ParseField("actions"); @@ -40,7 +45,20 @@ public class Phase implements ToXContentObject, Writeable { .collect(Collectors.toMap(LifecycleAction::getWriteableName, Function.identity())))); static { PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(), - (p, c) -> TimeValue.parseTimeValue(p.text(), MIN_AGE.getPreferredName()), MIN_AGE, ValueType.VALUE); + (ContextParser) (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(), (p, c, n) -> p.namedObject(LifecycleAction.class, n, null), v -> { throw new IllegalArgumentException("ordered " + ACTIONS_FIELD.getPreferredName() + " are not supported"); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/IndexLifecycleExplainResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/IndexLifecycleExplainResponseTests.java index 18302cc96ef8a..862c408e6d645 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/IndexLifecycleExplainResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/IndexLifecycleExplainResponseTests.java @@ -47,7 +47,7 @@ private static IndexLifecycleExplainResponse randomManagedIndexExplainResponse() boolean stepNull = randomBoolean(); return IndexLifecycleExplainResponse.newManagedIndexResponse(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), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java index 6beb2f613859c..1a7b26dc6a709 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java @@ -62,8 +62,8 @@ import java.util.Locale; 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.utils.QueryProviderTests.createRandomValidQueryProvider; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -468,8 +468,8 @@ public void testCheckValid_GivenIndicesContainsOnlyEmptyStrings() { public void testCheckValid_GivenNegativeQueryDelay() { DatafeedConfig.Builder conf = new DatafeedConfig.Builder("datafeed1", "job1"); IllegalArgumentException e = ESTestCase.expectThrows(IllegalArgumentException.class, - () -> conf.setQueryDelay(TimeValue.timeValueMillis(-10))); - assertEquals("query_delay cannot be less than 0. Value = -10", e.getMessage()); + () -> conf.setQueryDelay(TimeValue.timeValueMillis(-1))); + assertEquals("query_delay cannot be less than 0. Value = -1", e.getMessage()); } public void testCheckValid_GivenZeroFrequency() { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionConfigurationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionConfigurationTests.java index 430b076bab2c4..9659ca0fe2f54 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionConfigurationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionConfigurationTests.java @@ -182,7 +182,7 @@ public void testPartialsNotCountedTowardsMaximum() { } 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 s2 = makeFailureOrPartial(2, failure); SnapshotInfo s3 = makeFailureOrPartial(3, failure); diff --git a/x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/action/EnrichStatsResponseTests.java b/x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/action/EnrichStatsResponseTests.java index f0dbf5ef2ba39..a71ade28c4d0e 100644 --- a/x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/action/EnrichStatsResponseTests.java +++ b/x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/action/EnrichStatsResponseTests.java @@ -54,7 +54,7 @@ public static TaskInfo randomTaskInfo() { String action = randomAlphaOfLength(5); String description = randomAlphaOfLength(5); long startTime = randomLong(); - long runningTimeNanos = randomLong(); + long runningTimeNanos = randomNonNegativeLong(); boolean cancellable = randomBoolean(); TaskId parentTaskId = TaskId.EMPTY_TASK_ID; Map headers = randomBoolean() diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java index cf0e15838e3d0..a776e80626240 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java @@ -93,14 +93,22 @@ boolean isReadyToTransitionToThisPhase(final String policy, final IndexMetaData } final TimeValue after = stepRegistry.getIndexAgeForPhase(policy, phase); 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()) { 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, new TimeValue(now).seconds(), new TimeValue(lifecycleDate).seconds(), - age, age.seconds()); + ageMillis < 0 ? "-" : "", age, age.seconds()); } return now >= lifecycleDate + after.getMillis(); } diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTaskTests.java index 5474602cdfda6..8f2e54981684d 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTaskTests.java @@ -236,13 +236,15 @@ public void testPartialFailureSnapshot() throws Exception { Boolean.parseBoolean((String) policy.getConfig().get("include_global_state")); assertThat(req.includeGlobalState(), equalTo(globalState)); + long startTime = randomNonNegativeLong(); + long endTime = randomLongBetween(startTime, Long.MAX_VALUE); return new CreateSnapshotResponse( new SnapshotInfo( new SnapshotId(req.snapshot(), "uuid"), Arrays.asList(req.indices()), - randomNonNegativeLong(), + startTime, "snapshot started", - randomNonNegativeLong(), + endTime, 3, Collections.singletonList( new SnapshotShardFailure("nodeId", new ShardId("index", "uuid", 0), "forced failure")), diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndicesStatsMonitoringDocTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndicesStatsMonitoringDocTests.java index be2eb529d86be..1e90e6b38e62d 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndicesStatsMonitoringDocTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/indices/IndicesStatsMonitoringDocTests.java @@ -143,14 +143,14 @@ public void testToXContent() throws IOException { private CommonStats mockCommonStats() { 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)); - 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)); - final SearchStats.Stats searchStats = new SearchStats.Stats(6L, 7L, -1L, -1L, -1L, -1L, -1L, -1L, -1L, -1L, -1L, -1L); - commonStats.getSearch().add(new SearchStats(searchStats, -1L, null)); + 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, 0L, null)); return commonStats; } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/WatcherDateTimeUtilsTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/WatcherDateTimeUtilsTests.java index 8c08c12a493da..67cf5153520e0 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/WatcherDateTimeUtilsTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/WatcherDateTimeUtilsTests.java @@ -89,28 +89,6 @@ public void testParseTimeValueString() throws Exception { assertThat(parsed.millis(), is(values.get(key).millis())); } - public void testParseTimeValueStringNegative() throws Exception { - int value = -1 * randomIntBetween(2, 200); - Map 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 { XContentParser parser = createParser(jsonBuilder().startObject().nullField("value").endObject()); parser.nextToken(); // start object