Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make randomTimeValue() return a TimeValue #107689

Merged
merged 14 commits into from
Apr 23, 2024
15 changes: 14 additions & 1 deletion libs/core/src/main/java/org/elasticsearch/core/TimeValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -329,13 +329,26 @@ public String getStringRep() {
};
}

/**
* @param sValue Value to parse, which may not be {@code null}.
* @param settingName Name of the parameter or setting. On invalid input, this value is included in the exception message. Otherwise,
* this parameter is unused.
* @return The {@link TimeValue} which the input string represents.
*/
public static TimeValue parseTimeValue(String sValue, String settingName) {
Objects.requireNonNull(settingName);
Objects.requireNonNull(sValue);
return parseTimeValue(sValue, null, settingName);
}

public static TimeValue parseTimeValue(String sValue, TimeValue defaultValue, String settingName) {
/**
* @param sValue Value to parse, which may be {@code null}.
* @param defaultValue Value to return if {@code sValue} is {@code null}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be null as well? It is not forbidden but also not explicitly allowed?

* @param settingName Name of the parameter or setting. On invalid input, this value is included in the exception message. Otherwise,
* this parameter is unused.
* @return The {@link TimeValue} which the input string represents, or {@code defaultValue} if the input is {@code null}.
*/
public static TimeValue parseTimeValue(@Nullable String sValue, TimeValue defaultValue, String settingName) {
settingName = Objects.requireNonNull(settingName);
if (sValue == null) {
return defaultValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void testParseTimeValue() {
}

public void testRoundTrip() {
final String s = randomTimeValue();
final String s = randomTimeValue().getStringRep();
assertThat(TimeValue.parseTimeValue(s, null, "test").getStringRep(), equalTo(s));
final TimeValue t = new TimeValue(randomIntBetween(1, 128), randomFrom(TimeUnit.values()));
assertThat(TimeValue.parseTimeValue(t.getStringRep(), null, "test"), equalTo(t));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ private static RolloverConditions randomRolloverConditions(boolean includeMaxAge
ByteSizeValue minSize = randomBoolean() ? randomByteSizeValue() : null;
ByteSizeValue minPrimaryShardSize = randomBoolean() ? randomByteSizeValue() : null;
Long minDocs = randomBoolean() ? randomNonNegativeLong() : null;
TimeValue minAge = randomBoolean() ? TimeValue.parseTimeValue(randomPositiveTimeValue(), "rollover_action_test") : null;
TimeValue minAge = randomBoolean() ? randomPositiveTimeValue() : null;
Long minPrimaryShardDocs = randomBoolean() ? randomNonNegativeLong() : null;

return RolloverConditions.newBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import java.util.Map;

import static org.apache.lucene.tests.util.TestUtil.randomSimpleString;
import static org.elasticsearch.core.TimeValue.parseTimeValue;

/**
* Round trip tests for all {@link Writeable} things declared in this plugin.
Expand All @@ -58,8 +57,8 @@ public void testReindexRequest() throws IOException {
while (headers.size() < headersCount) {
headers.put(randomAlphaOfLength(5), randomAlphaOfLength(5));
}
TimeValue socketTimeout = parseTimeValue(randomPositiveTimeValue(), "socketTimeout");
TimeValue connectTimeout = parseTimeValue(randomPositiveTimeValue(), "connectTimeout");
TimeValue socketTimeout = randomPositiveTimeValue();
TimeValue connectTimeout = randomPositiveTimeValue();
reindex.setRemoteInfo(
new RemoteInfo(
randomAlphaOfLength(5),
Expand Down Expand Up @@ -121,7 +120,7 @@ private void randomRequest(AbstractBulkByScrollRequest<?> request) {
}
request.setAbortOnVersionConflict(random().nextBoolean());
request.setRefresh(rarely());
request.setTimeout(TimeValue.parseTimeValue(randomTimeValue(), null, "test"));
request.setTimeout(randomTimeValue());
request.setWaitForActiveShards(randomIntBetween(0, 10));
request.setRequestsPerSecond(between(0, Integer.MAX_VALUE));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public void testInitialSearchParamsMisc() {

TimeValue scroll = null;
if (randomBoolean()) {
scroll = TimeValue.parseTimeValue(randomPositiveTimeValue(), "test");
scroll = randomPositiveTimeValue();
searchRequest.scroll(scroll);
}
int size = between(0, Integer.MAX_VALUE);
Expand Down Expand Up @@ -251,7 +251,7 @@ public void testInitialSearchEntity() throws IOException {
public void testScrollParams() {
String scroll = randomAlphaOfLength(30);
Version remoteVersion = Version.fromId(between(0, Version.CURRENT.id));
TimeValue keepAlive = TimeValue.parseTimeValue(randomPositiveTimeValue(), "test");
TimeValue keepAlive = randomPositiveTimeValue();
assertScroll(remoteVersion, scroll(scroll, keepAlive, remoteVersion).getParameters(), keepAlive);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ private static Tuple<ServiceAccountCredentials, byte[]> randomCredential(final S
}

private static TimeValue randomTimeout() {
return randomFrom(TimeValue.MINUS_ONE, TimeValue.ZERO, TimeValue.parseTimeValue(randomPositiveTimeValue(), "test"));
return randomFrom(TimeValue.MINUS_ONE, TimeValue.ZERO, randomPositiveTimeValue());
}

private static void assertGoogleCredential(ServiceAccountCredentials expected, ServiceAccountCredentials actual) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,11 @@ private void runGlobalCheckpointSyncTest(
public void testPersistGlobalCheckpoint() throws Exception {
internalCluster().ensureAtLeastNumDataNodes(2);
Settings.Builder indexSettings = Settings.builder()
.put(IndexService.GLOBAL_CHECKPOINT_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(100, 1000, "ms"))
.put(IndexService.GLOBAL_CHECKPOINT_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(100, 1000, TimeUnit.MILLISECONDS))
.put("index.number_of_replicas", randomIntBetween(0, 1));
if (randomBoolean()) {
indexSettings.put(IndexSettings.INDEX_TRANSLOG_DURABILITY_SETTING.getKey(), Translog.Durability.ASYNC)
.put(IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(100, 1000, "ms"));
.put(IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(100, 1000, TimeUnit.MILLISECONDS));
}
prepareCreate("test", indexSettings).get();
if (randomBoolean()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,18 @@ public void testFlushOnInactive() throws Exception {
List<String> dataNodes = internalCluster().startDataOnlyNodes(
2,
Settings.builder()
.put(IndexingMemoryController.SHARD_INACTIVE_TIME_SETTING.getKey(), randomTimeValue(10, 1000, "ms"))
.put(IndexingMemoryController.SHARD_MEMORY_INTERVAL_TIME_SETTING.getKey(), randomTimeValue(10, 1000, "ms"))
.put(IndexingMemoryController.SHARD_INACTIVE_TIME_SETTING.getKey(), randomTimeValue(10, 1000, TimeUnit.MILLISECONDS))
.put(IndexingMemoryController.SHARD_MEMORY_INTERVAL_TIME_SETTING.getKey(), randomTimeValue(10, 1000, TimeUnit.MILLISECONDS))
.build()
);
assertAcked(
indicesAdmin().prepareCreate(indexName)
.setSettings(
indexSettings(1, 1).put(IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(200, 500, "ms"))
.put(IndexService.GLOBAL_CHECKPOINT_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(50, 200, "ms"))
indexSettings(1, 1).put(
IndexSettings.INDEX_TRANSLOG_SYNC_INTERVAL_SETTING.getKey(),
randomTimeValue(200, 500, TimeUnit.MILLISECONDS)
)
.put(IndexService.GLOBAL_CHECKPOINT_SYNC_INTERVAL_SETTING.getKey(), randomTimeValue(50, 200, TimeUnit.MILLISECONDS))
.put("index.routing.allocation.include._name", String.join(",", dataNodes))
.build()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static RolloverConditions randomRolloverConditions() {
ByteSizeValue minSize = randomBoolean() ? randomByteSizeValue() : null;
ByteSizeValue minPrimaryShardSize = randomBoolean() ? randomByteSizeValue() : null;
Long minDocs = randomBoolean() ? randomNonNegativeLong() : null;
TimeValue minAge = randomBoolean() ? TimeValue.parseTimeValue(randomPositiveTimeValue(), "rollover_action_test") : null;
TimeValue minAge = randomBoolean() ? randomPositiveTimeValue() : null;
Long minPrimaryShardDocs = randomBoolean() ? randomNonNegativeLong() : null;

return RolloverConditions.newBuilder()
Expand Down Expand Up @@ -89,10 +89,7 @@ protected RolloverConditions mutateInstance(RolloverConditions instance) {
ByteSizeUnit maxPrimaryShardSizeUnit = randomFrom(ByteSizeUnit.values());
return new ByteSizeValue(randomNonNegativeLong() / maxPrimaryShardSizeUnit.toBytes(1), maxPrimaryShardSizeUnit);
});
case 2 -> maxAge = randomValueOtherThan(
maxAge,
() -> TimeValue.parseTimeValue(randomPositiveTimeValue(), "rollover_action_test")
);
case 2 -> maxAge = randomValueOtherThan(maxAge, () -> randomPositiveTimeValue());
case 3 -> maxDocs = maxDocs == null ? randomNonNegativeLong() : maxDocs + 1;
case 4 -> maxPrimaryShardDocs = maxPrimaryShardDocs == null ? randomNonNegativeLong() : maxPrimaryShardDocs + 1;
case 5 -> minSize = randomValueOtherThan(minSize, () -> {
Expand All @@ -103,10 +100,7 @@ protected RolloverConditions mutateInstance(RolloverConditions instance) {
ByteSizeUnit minPrimaryShardSizeUnit = randomFrom(ByteSizeUnit.values());
return new ByteSizeValue(randomNonNegativeLong() / minPrimaryShardSizeUnit.toBytes(1), minPrimaryShardSizeUnit);
});
case 7 -> minAge = randomValueOtherThan(
minAge,
() -> TimeValue.parseTimeValue(randomPositiveTimeValue(), "rollover_action_test")
);
case 7 -> minAge = randomValueOtherThan(minAge, () -> randomPositiveTimeValue());
case 8 -> minDocs = minDocs == null ? randomNonNegativeLong() : minDocs + 1;
case 9 -> minPrimaryShardDocs = minPrimaryShardDocs == null ? randomNonNegativeLong() : minPrimaryShardDocs + 1;
default -> throw new AssertionError("Illegal randomisation branch");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public static RolloverConfiguration randomRolloverConditions() {
ByteSizeValue minSize = randomBoolean() ? randomByteSizeValue() : null;
ByteSizeValue minPrimaryShardSize = randomBoolean() ? randomByteSizeValue() : null;
Long minDocs = randomBoolean() ? randomNonNegativeLong() : null;
TimeValue minAge = randomBoolean() ? TimeValue.parseTimeValue(randomPositiveTimeValue(), "rollover_action_test") : null;
TimeValue minAge = randomBoolean() ? randomPositiveTimeValue() : null;
Long minPrimaryShardDocs = randomBoolean() ? randomNonNegativeLong() : null;

RolloverConditions.Builder concreteConditionsBuilder = RolloverConditions.newBuilder()
Expand Down Expand Up @@ -346,12 +346,12 @@ public void testXContentSerializationWithKnownDataRetention() throws IOException
private static final List<Consumer<RolloverConfiguration.ValueParser>> conditionsGenerator = Arrays.asList(
(builder) -> builder.addMaxIndexDocsCondition(randomNonNegativeLong()),
(builder) -> builder.addMaxIndexSizeCondition(randomByteSizeValue().getStringRep(), "test"),
(builder) -> builder.addMaxIndexAgeCondition(randomPositiveTimeValue(), "test"),
(builder) -> builder.addMaxIndexAgeCondition(randomPositiveTimeValue().getStringRep(), "test"),
(builder) -> builder.addMaxPrimaryShardSizeCondition(randomByteSizeValue().getStringRep(), "test"),
(builder) -> builder.addMaxPrimaryShardDocsCondition(randomNonNegativeLong()),
(builder) -> builder.addMinIndexDocsCondition(randomNonNegativeLong()),
(builder) -> builder.addMinIndexSizeCondition(randomByteSizeValue().getStringRep(), "test"),
(builder) -> builder.addMinIndexAgeCondition(randomPositiveTimeValue(), "test"),
(builder) -> builder.addMinIndexAgeCondition(randomPositiveTimeValue().getStringRep(), "test"),
(builder) -> builder.addMinPrimaryShardSizeCondition(randomByteSizeValue().getStringRep(), "test"),
(builder) -> builder.addMinPrimaryShardDocsCondition(randomNonNegativeLong())
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.test.AbstractWireSerializingTestCase;
import org.elasticsearch.test.ESTestCase;

import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -31,7 +30,7 @@ public class UpdateSettingsRequestSerializationTests extends AbstractWireSeriali
protected UpdateSettingsRequest mutateInstance(UpdateSettingsRequest request) {
UpdateSettingsRequest mutation = copyRequest(request);
List<Runnable> mutators = new ArrayList<>();
Supplier<TimeValue> timeValueSupplier = () -> TimeValue.parseTimeValue(ESTestCase.randomTimeValue(), "_setting");
Supplier<TimeValue> timeValueSupplier = () -> randomTimeValue();
mutators.add(() -> mutation.masterNodeTimeout(randomValueOtherThan(request.masterNodeTimeout(), timeValueSupplier)));
mutators.add(() -> mutation.ackTimeout(randomValueOtherThan(request.ackTimeout(), timeValueSupplier)));
mutators.add(() -> mutation.settings(mutateSettings(request.settings())));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ public void testCreateDataStreamWithFailureStoreWithRefreshRate() throws Excepti
assertThat(newState.metadata().index(backingIndexName), notNullValue());
assertThat(newState.metadata().index(failureStoreIndexName), notNullValue());
assertThat(
newState.metadata().index(failureStoreIndexName).getSettings().get(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey()),
IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.get(newState.metadata().index(failureStoreIndexName).getSettings()),
equalTo(timeValue)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import static org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata.Type.SIGTERM;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItem;
Expand Down Expand Up @@ -78,11 +79,7 @@ public void testIsNodeShuttingDown() {
.setReason("shutdown for a unit test")
.setType(type)
.setStartedAtMillis(randomNonNegativeLong())
.setGracePeriod(
type == SingleNodeShutdownMetadata.Type.SIGTERM
? TimeValue.parseTimeValue(randomTimeValue(), this.getTestName())
: null
)
.setGracePeriod(type == SIGTERM ? randomTimeValue() : null)
.build()
)
);
Expand Down Expand Up @@ -154,11 +151,11 @@ private SingleNodeShutdownMetadata randomNodeShutdownInfo() {
.setReason(randomAlphaOfLength(5))
.setStartedAtMillis(randomNonNegativeLong());
if (type.equals(SingleNodeShutdownMetadata.Type.RESTART) && randomBoolean()) {
builder.setAllocationDelay(TimeValue.parseTimeValue(randomTimeValue(), this.getTestName()));
builder.setAllocationDelay(randomTimeValue());
} else if (type.equals(SingleNodeShutdownMetadata.Type.REPLACE)) {
builder.setTargetNodeName(randomAlphaOfLengthBetween(5, 10));
} else if (type.equals(SingleNodeShutdownMetadata.Type.SIGTERM)) {
builder.setGracePeriod(TimeValue.parseTimeValue(randomTimeValue(), this.getTestName()));
builder.setGracePeriod(randomTimeValue());
}
return builder.setNodeSeen(randomBoolean()).build();
}
Expand Down
Loading