From d5293ea108313f226c713eb13f2cde51e3bd0ce6 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 27 Mar 2020 16:10:49 -0600 Subject: [PATCH] [6.8] Disallow negative TimeValues (#53913) (#54304) * 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. * Re-apply IndexLifecycleRunner changes lost in merge * Make SnapshotStatsTests generate more realistic times * Fix negative TimeValue in deprecation test * Re-apply CcrStatsResponseTests fix lost in merge --- .../client/ccr/CcrStatsResponseTests.java | 2 +- .../elasticsearch/common/unit/TimeValue.java | 29 ++++++++++++------- .../common/unit/TimeValueTests.java | 22 ++++++++++++++ .../index/shard/IndexingStats.java | 4 ++- .../monitor/jvm/JvmGcMonitorService.java | 17 +++++++---- .../elasticsearch/snapshots/SnapshotInfo.java | 2 +- .../snapshots/status/SnapshotStatsTests.java | 6 ++-- .../rounding/TimeZoneRoundingTests.java | 4 +-- .../IndexingMemoryControllerTests.java | 6 ++-- .../jvm/JvmGcMonitorServiceSettingsTests.java | 9 +++--- .../ShardFollowNodeTaskStatusTests.java | 2 +- .../xpack/ccr/action/StatsResponsesTests.java | 2 +- .../core/ml/datafeed/DatafeedConfigTests.java | 4 +-- .../deprecation/IndexDeprecationChecks.java | 4 +-- .../action/EnrichStatsResponseTests.java | 0 .../indexlifecycle/IndexLifecycleRunner.java | 14 +++++++-- .../IndicesStatsMonitoringDocTests.java | 8 ++--- .../support/WatcherDateTimeUtilsTests.java | 22 -------------- 18 files changed, 92 insertions(+), 65 deletions(-) create mode 100644 x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/action/EnrichStatsResponseTests.java 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 cb8072f6bafb3..37e83d8bfbe71 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 @@ -390,7 +390,7 @@ static IndicesFollowStats randomIndicesFollowStats() { randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong(), - randomLong(), + randomNonNegativeLong(), readExceptions, randomBoolean() ? new ElasticsearchException("fatal error") : null)); } 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 f40fbbe73a9dc..b5a3003b194d5 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 @@ -74,6 +74,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; } @@ -186,7 +189,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)}. */ @Override public String toString() { @@ -278,20 +281,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+")) { @@ -303,10 +306,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 af6b89be5fffe..b810524e66be9 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 @@ -218,4 +218,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 c7b6a99d2c7e1..834e5ebb822ef 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexingStats.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexingStats.java @@ -120,7 +120,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 aaaa41e77c442..e4640c5840f77 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java @@ -520,7 +520,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/admin/cluster/snapshots/status/SnapshotStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStatsTests.java index 2822a9661fd15..f8e7dc646af3d 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStatsTests.java @@ -19,17 +19,17 @@ package org.elasticsearch.action.admin.cluster.snapshots.status; -import java.io.IOException; - import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractXContentTestCase; +import java.io.IOException; + public class SnapshotStatsTests extends AbstractXContentTestCase { @Override protected SnapshotStats createTestInstance() { long startTime = randomNonNegativeLong(); - long time = randomNonNegativeLong(); + long time = randomLongBetween(0, Long.MAX_VALUE - startTime); int incrementalFileCount = randomIntBetween(0, Integer.MAX_VALUE); int totalFileCount = randomIntBetween(0, Integer.MAX_VALUE); int processedFileCount = randomIntBetween(0, Integer.MAX_VALUE); 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 e49f25772a726..8269da45fd4e3 100644 --- a/server/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java @@ -104,8 +104,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 768244308c038..108dd7393fbfc 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java @@ -260,7 +260,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()); } @@ -268,7 +269,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 9cac01d278e74..5050228b13185 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 @@ -60,7 +60,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 c173404955826..0f1db9620bedb 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 @@ -57,7 +57,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/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 2a8d39778e128..3ec08e5ed25ba 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 @@ -366,8 +366,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/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IndexDeprecationChecks.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IndexDeprecationChecks.java index ed0eeefb84975..e4913b4c24c9b 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IndexDeprecationChecks.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IndexDeprecationChecks.java @@ -14,7 +14,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.joda.JodaDeprecationPatterns; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.analysis.AnalysisRegistry; import org.elasticsearch.xpack.core.deprecation.DeprecationIssue; @@ -218,8 +217,7 @@ static DeprecationIssue nodeLeftDelayedTimeCheck(IndexMetaData indexMetaData) { String setting = UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(); String value = indexMetaData.getSettings().get(setting); if (Strings.isNullOrEmpty(value) == false) { - TimeValue parsedValue = TimeValue.parseTimeValue(value, setting); - if (parsedValue.getNanos() < 0) { + if (value.startsWith("-")) { return new DeprecationIssue(DeprecationIssue.Level.WARNING, "Negative values for " + setting + " are deprecated and should be set to 0", "https://www.elastic.co/guide/en/elasticsearch/reference/7.0/breaking-changes-7.0.html" + 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 new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunner.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunner.java index 05ad342f3e779..0e9f876b79f76 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunner.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunner.java @@ -81,14 +81,22 @@ boolean isReadyToTransitionToThisPhase(final String policy, final IndexMetaData assert lifecycleDate != null && lifecycleDate >= 0 : "expected index to have a lifecycle date but it did not"; 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/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