Remove custom PeriodType for watcher PeriodThrottler (elastic/x-pack-elasticsearch#4327)

This constructor was actually never used, other than in tests, and even then,
there is no need for a custom period type as the human-readable toString value
will suffice.

Original commit: elastic/x-pack-elasticsearch@fc666a04b9
This commit is contained in:
Lee Hinman 2018-04-10 08:01:32 -06:00 committed by GitHub
parent 4c7bd71bdf
commit 9e3b03531c
4 changed files with 10 additions and 20 deletions

View File

@ -9,7 +9,6 @@ import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.xpack.core.watcher.actions.ActionStatus; import org.elasticsearch.xpack.core.watcher.actions.ActionStatus;
import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext;
import org.joda.time.PeriodType;
import java.time.Clock; import java.time.Clock;
@ -22,16 +21,10 @@ import static org.elasticsearch.xpack.core.watcher.actions.throttler.Throttler.T
public class PeriodThrottler implements Throttler { public class PeriodThrottler implements Throttler {
@Nullable private final TimeValue period; @Nullable private final TimeValue period;
private final PeriodType periodType;
private final Clock clock; private final Clock clock;
public PeriodThrottler(Clock clock, TimeValue period) { public PeriodThrottler(Clock clock, TimeValue period) {
this(clock, period, PeriodType.minutes());
}
public PeriodThrottler(Clock clock, TimeValue period, PeriodType periodType) {
this.period = period; this.period = period;
this.periodType = periodType;
this.clock = clock; this.clock = clock;
} }
@ -57,7 +50,7 @@ public class PeriodThrottler implements Throttler {
TimeValue timeElapsed = TimeValue.timeValueMillis(clock.millis() - status.lastSuccessfulExecution().timestamp().getMillis()); TimeValue timeElapsed = TimeValue.timeValueMillis(clock.millis() - status.lastSuccessfulExecution().timestamp().getMillis());
if (timeElapsed.getMillis() <= period.getMillis()) { if (timeElapsed.getMillis() <= period.getMillis()) {
return Result.throttle(PERIOD, "throttling interval is set to [{}] but time elapsed since last execution is [{}]", return Result.throttle(PERIOD, "throttling interval is set to [{}] but time elapsed since last execution is [{}]",
period.format(periodType), timeElapsed.format(periodType)); period, timeElapsed);
} }
return Result.NO; return Result.NO;
} }

View File

@ -14,7 +14,6 @@ import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext;
import org.elasticsearch.xpack.core.watcher.watch.Payload; import org.elasticsearch.xpack.core.watcher.watch.Payload;
import org.elasticsearch.xpack.core.watcher.watch.WatchStatus; import org.elasticsearch.xpack.core.watcher.watch.WatchStatus;
import org.joda.time.DateTime; import org.joda.time.DateTime;
import org.joda.time.PeriodType;
import java.time.Clock; import java.time.Clock;
@ -28,9 +27,8 @@ import static org.mockito.Mockito.when;
public class PeriodThrottlerTests extends ESTestCase { public class PeriodThrottlerTests extends ESTestCase {
public void testBelowPeriodSuccessful() throws Exception { public void testBelowPeriodSuccessful() throws Exception {
PeriodType periodType = randomFrom(PeriodType.millis(), PeriodType.seconds(), PeriodType.minutes());
TimeValue period = TimeValue.timeValueSeconds(randomIntBetween(2, 5)); TimeValue period = TimeValue.timeValueSeconds(randomIntBetween(2, 5));
PeriodThrottler throttler = new PeriodThrottler(Clock.systemUTC(), period, periodType); PeriodThrottler throttler = new PeriodThrottler(Clock.systemUTC(), period);
WatchExecutionContext ctx = mockExecutionContext("_name", Payload.EMPTY); WatchExecutionContext ctx = mockExecutionContext("_name", Payload.EMPTY);
ActionStatus actionStatus = mock(ActionStatus.class); ActionStatus actionStatus = mock(ActionStatus.class);
@ -45,14 +43,13 @@ public class PeriodThrottlerTests extends ESTestCase {
assertThat(result, notNullValue()); assertThat(result, notNullValue());
assertThat(result.throttle(), is(true)); assertThat(result.throttle(), is(true));
assertThat(result.reason(), notNullValue()); assertThat(result.reason(), notNullValue());
assertThat(result.reason(), startsWith("throttling interval is set to [" + period.format(periodType) + "]")); assertThat(result.reason(), startsWith("throttling interval is set to [" + period + "]"));
assertThat(result.type(), is(Throttler.Type.PERIOD)); assertThat(result.type(), is(Throttler.Type.PERIOD));
} }
public void testAbovePeriod() throws Exception { public void testAbovePeriod() throws Exception {
PeriodType periodType = randomFrom(PeriodType.millis(), PeriodType.seconds(), PeriodType.minutes());
TimeValue period = TimeValue.timeValueSeconds(randomIntBetween(2, 5)); TimeValue period = TimeValue.timeValueSeconds(randomIntBetween(2, 5));
PeriodThrottler throttler = new PeriodThrottler(Clock.systemUTC(), period, periodType); PeriodThrottler throttler = new PeriodThrottler(Clock.systemUTC(), period);
WatchExecutionContext ctx = mockExecutionContext("_name", Payload.EMPTY); WatchExecutionContext ctx = mockExecutionContext("_name", Payload.EMPTY);
ActionStatus actionStatus = mock(ActionStatus.class); ActionStatus actionStatus = mock(ActionStatus.class);

View File

@ -40,7 +40,7 @@ public class HttpConnectionTimeoutTests extends ESTestCase {
fail("expected timeout exception"); fail("expected timeout exception");
} catch (ConnectTimeoutException ete) { } catch (ConnectTimeoutException ete) {
TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start); TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start);
logger.info("http connection timed out after {}", timeout.format()); logger.info("http connection timed out after {}", timeout);
// it's supposed to be 10, but we'll give it an error margin of 2 seconds // it's supposed to be 10, but we'll give it an error margin of 2 seconds
assertThat(timeout.seconds(), greaterThan(8L)); assertThat(timeout.seconds(), greaterThan(8L));
assertThat(timeout.seconds(), lessThan(12L)); assertThat(timeout.seconds(), lessThan(12L));
@ -66,7 +66,7 @@ public class HttpConnectionTimeoutTests extends ESTestCase {
fail("expected timeout exception"); fail("expected timeout exception");
} catch (ConnectTimeoutException ete) { } catch (ConnectTimeoutException ete) {
TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start); TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start);
logger.info("http connection timed out after {}", timeout.format()); logger.info("http connection timed out after {}", timeout);
// it's supposed to be 7, but we'll give it an error margin of 2 seconds // it's supposed to be 7, but we'll give it an error margin of 2 seconds
assertThat(timeout.seconds(), greaterThan(3L)); assertThat(timeout.seconds(), greaterThan(3L));
assertThat(timeout.seconds(), lessThan(7L)); assertThat(timeout.seconds(), lessThan(7L));
@ -93,7 +93,7 @@ public class HttpConnectionTimeoutTests extends ESTestCase {
fail("expected timeout exception"); fail("expected timeout exception");
} catch (ConnectTimeoutException ete) { } catch (ConnectTimeoutException ete) {
TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start); TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start);
logger.info("http connection timed out after {}", timeout.format()); logger.info("http connection timed out after {}", timeout);
// it's supposed to be 7, but we'll give it an error margin of 2 seconds // it's supposed to be 7, but we'll give it an error margin of 2 seconds
assertThat(timeout.seconds(), greaterThan(3L)); assertThat(timeout.seconds(), greaterThan(3L));
assertThat(timeout.seconds(), lessThan(7L)); assertThat(timeout.seconds(), lessThan(7L));

View File

@ -51,7 +51,7 @@ public class HttpReadTimeoutTests extends ESTestCase {
long start = System.nanoTime(); long start = System.nanoTime();
expectThrows(SocketTimeoutException.class, () -> httpClient.execute(request)); expectThrows(SocketTimeoutException.class, () -> httpClient.execute(request));
TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start); TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start);
logger.info("http connection timed out after {}", timeout.format()); logger.info("http connection timed out after {}", timeout);
// it's supposed to be 10, but we'll give it an error margin of 2 seconds // it's supposed to be 10, but we'll give it an error margin of 2 seconds
assertThat(timeout.seconds(), greaterThan(8L)); assertThat(timeout.seconds(), greaterThan(8L));
@ -73,7 +73,7 @@ public class HttpReadTimeoutTests extends ESTestCase {
long start = System.nanoTime(); long start = System.nanoTime();
expectThrows(SocketTimeoutException.class, () -> httpClient.execute(request)); expectThrows(SocketTimeoutException.class, () -> httpClient.execute(request));
TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start); TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start);
logger.info("http connection timed out after {}", timeout.format()); logger.info("http connection timed out after {}", timeout);
// it's supposed to be 3, but we'll give it an error margin of 2 seconds // it's supposed to be 3, but we'll give it an error margin of 2 seconds
assertThat(timeout.seconds(), greaterThan(1L)); assertThat(timeout.seconds(), greaterThan(1L));
@ -96,7 +96,7 @@ public class HttpReadTimeoutTests extends ESTestCase {
long start = System.nanoTime(); long start = System.nanoTime();
expectThrows(SocketTimeoutException.class, () -> httpClient.execute(request)); expectThrows(SocketTimeoutException.class, () -> httpClient.execute(request));
TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start); TimeValue timeout = TimeValue.timeValueNanos(System.nanoTime() - start);
logger.info("http connection timed out after {}", timeout.format()); logger.info("http connection timed out after {}", timeout);
// it's supposed to be 3, but we'll give it an error margin of 2 seconds // it's supposed to be 3, but we'll give it an error margin of 2 seconds
assertThat(timeout.seconds(), greaterThan(1L)); assertThat(timeout.seconds(), greaterThan(1L));