Skip to content

Commit

Permalink
[7.x][Transform] use ISO dates in output instead of epoch millis (#65584
Browse files Browse the repository at this point in the history
) (#65952)

Transform writes dates as epoch millis, this does not work for historic data in some cases or is
unsupported. Dates should be written as such. With this PR transform starts writing dates in ISO
format, but as existing transform might rely on the format it provides backwards compatibility for
old jobs as well as a setting to write dates as epoch millis.

fixes #63787
backport #65584
  • Loading branch information
Hendrik Muhs authored Dec 7, 2020
1 parent 085bb9c commit 8e377da
Show file tree
Hide file tree
Showing 27 changed files with 510 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
Expand All @@ -34,30 +35,43 @@ public class SettingsConfig implements ToXContentObject {

private static final ParseField MAX_PAGE_SEARCH_SIZE = new ParseField("max_page_search_size");
private static final ParseField DOCS_PER_SECOND = new ParseField("docs_per_second");
private static final ParseField DATES_AS_EPOCH_MILLIS = new ParseField("dates_as_epoch_millis");
private static final int DEFAULT_MAX_PAGE_SEARCH_SIZE = -1;
private static final float DEFAULT_DOCS_PER_SECOND = -1F;

// use an integer as we need to code 4 states: true, false, null (unchanged), default (defined server side)
private static final int DEFAULT_DATES_AS_EPOCH_MILLIS = -1;

private final Integer maxPageSearchSize;
private final Float docsPerSecond;
private final Integer datesAsEpochMillis;

private static final ConstructingObjectParser<SettingsConfig, Void> PARSER = new ConstructingObjectParser<>(
"settings_config",
true,
args -> new SettingsConfig((Integer) args[0], (Float) args[1])
args -> new SettingsConfig((Integer) args[0], (Float) args[1], (Integer) args[2])
);

static {
PARSER.declareIntOrNull(optionalConstructorArg(), DEFAULT_MAX_PAGE_SEARCH_SIZE, MAX_PAGE_SEARCH_SIZE);
PARSER.declareFloatOrNull(optionalConstructorArg(), DEFAULT_DOCS_PER_SECOND, DOCS_PER_SECOND);
// this boolean requires 4 possible values: true, false, not_specified, default, therefore using a custom parser
PARSER.declareField(
optionalConstructorArg(),
p -> p.currentToken() == XContentParser.Token.VALUE_NULL ? DEFAULT_DATES_AS_EPOCH_MILLIS : p.booleanValue() ? 1 : 0,
DATES_AS_EPOCH_MILLIS,
ValueType.BOOLEAN_OR_NULL
);
}

public static SettingsConfig fromXContent(final XContentParser parser) {
return PARSER.apply(parser, null);
}

SettingsConfig(Integer maxPageSearchSize, Float docsPerSecond) {
SettingsConfig(Integer maxPageSearchSize, Float docsPerSecond, Integer datesAsEpochMillis) {
this.maxPageSearchSize = maxPageSearchSize;
this.docsPerSecond = docsPerSecond;
this.datesAsEpochMillis = datesAsEpochMillis;
}

@Override
Expand All @@ -77,6 +91,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field(DOCS_PER_SECOND.getPreferredName(), docsPerSecond);
}
}
if (datesAsEpochMillis != null) {
if (datesAsEpochMillis.equals(DEFAULT_DATES_AS_EPOCH_MILLIS)) {
builder.field(DATES_AS_EPOCH_MILLIS.getPreferredName(), (Boolean) null);
} else {
builder.field(DATES_AS_EPOCH_MILLIS.getPreferredName(), datesAsEpochMillis > 0 ? true : false);
}
}
builder.endObject();
return builder;
}
Expand All @@ -89,6 +110,10 @@ public Float getDocsPerSecond() {
return docsPerSecond;
}

public Boolean getDatesAsEpochMillis() {
return datesAsEpochMillis != null ? datesAsEpochMillis > 0 : null;
}

@Override
public boolean equals(Object other) {
if (other == this) {
Expand All @@ -99,12 +124,14 @@ public boolean equals(Object other) {
}

SettingsConfig that = (SettingsConfig) other;
return Objects.equals(maxPageSearchSize, that.maxPageSearchSize) && Objects.equals(docsPerSecond, that.docsPerSecond);
return Objects.equals(maxPageSearchSize, that.maxPageSearchSize)
&& Objects.equals(docsPerSecond, that.docsPerSecond)
&& Objects.equals(datesAsEpochMillis, that.datesAsEpochMillis);
}

@Override
public int hashCode() {
return Objects.hash(maxPageSearchSize, docsPerSecond);
return Objects.hash(maxPageSearchSize, docsPerSecond, datesAsEpochMillis);
}

public static Builder builder() {
Expand All @@ -114,6 +141,7 @@ public static Builder builder() {
public static class Builder {
private Integer maxPageSearchSize;
private Float docsPerSecond;
private Integer datesAsEpochMillis;

/**
* Sets the paging maximum paging maxPageSearchSize that transform can use when
Expand Down Expand Up @@ -143,8 +171,24 @@ public Builder setRequestsPerSecond(Float docsPerSecond) {
return this;
}

/**
* Whether to write the output of a date aggregation as millis since epoch or as formatted string (ISO format).
*
* Transforms created before 7.11 write dates as epoch_millis. The new default is ISO string.
* You can use this setter to configure the old style writing as epoch millis.
*
* An explicit `null` resets to default.
*
* @param datesAsEpochMillis true if dates should be written as epoch_millis.
* @return the {@link Builder} with datesAsEpochMilli set.
*/
public Builder setDatesAsEpochMillis(Boolean datesAsEpochMillis) {
this.datesAsEpochMillis = datesAsEpochMillis == null ? DEFAULT_DATES_AS_EPOCH_MILLIS : datesAsEpochMillis ? 1 : 0;
return this;
}

public SettingsConfig build() {
return new SettingsConfig(maxPageSearchSize, docsPerSecond);
return new SettingsConfig(maxPageSearchSize, docsPerSecond, datesAsEpochMillis);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@
public class SettingsConfigTests extends AbstractXContentTestCase<SettingsConfig> {

public static SettingsConfig randomSettingsConfig() {
return new SettingsConfig(randomBoolean() ? null : randomIntBetween(10, 10_000), randomBoolean() ? null : randomFloat());
return new SettingsConfig(
randomBoolean() ? null : randomIntBetween(10, 10_000),
randomBoolean() ? null : randomFloat(),
randomBoolean() ? null : randomIntBetween(-1, 1)
);
}

@Override
Expand Down Expand Up @@ -67,6 +71,7 @@ public void testExplicitNullOnWriteParser() throws IOException {

SettingsConfig emptyConfig = fromString("{}");
assertNull(emptyConfig.getMaxPageSearchSize());
assertNull(emptyConfig.getDatesAsEpochMillis());

settingsAsMap = xContentToMap(emptyConfig);
assertTrue(settingsAsMap.isEmpty());
Expand All @@ -77,6 +82,15 @@ public void testExplicitNullOnWriteParser() throws IOException {
settingsAsMap = xContentToMap(config);
assertThat(settingsAsMap.getOrDefault("max_page_search_size", "not_set"), equalTo("not_set"));
assertNull(settingsAsMap.getOrDefault("docs_per_second", "not_set"));
assertThat(settingsAsMap.getOrDefault("dates_as_epoch_millis", "not_set"), equalTo("not_set"));

config = fromString("{\"dates_as_epoch_millis\" : null}");
assertFalse(config.getDatesAsEpochMillis());

settingsAsMap = xContentToMap(config);
assertThat(settingsAsMap.getOrDefault("max_page_search_size", "not_set"), equalTo("not_set"));
assertThat(settingsAsMap.getOrDefault("docs_per_second", "not_set"), equalTo("not_set"));
assertNull(settingsAsMap.getOrDefault("dates_as_epoch_millis", "not_set"));
}

public void testExplicitNullOnWriteBuilder() throws IOException {
Expand All @@ -87,9 +101,11 @@ public void testExplicitNullOnWriteBuilder() throws IOException {
Map<String, Object> settingsAsMap = xContentToMap(config);
assertNull(settingsAsMap.getOrDefault("max_page_search_size", "not_set"));
assertThat(settingsAsMap.getOrDefault("docs_per_second", "not_set"), equalTo("not_set"));
assertThat(settingsAsMap.getOrDefault("dates_as_epoch_millis", "not_set"), equalTo("not_set"));

SettingsConfig emptyConfig = new SettingsConfig.Builder().build();
assertNull(emptyConfig.getMaxPageSearchSize());
assertNull(emptyConfig.getDatesAsEpochMillis());

settingsAsMap = xContentToMap(emptyConfig);
assertTrue(settingsAsMap.isEmpty());
Expand All @@ -100,6 +116,16 @@ public void testExplicitNullOnWriteBuilder() throws IOException {
settingsAsMap = xContentToMap(config);
assertThat(settingsAsMap.getOrDefault("max_page_search_size", "not_set"), equalTo("not_set"));
assertNull(settingsAsMap.getOrDefault("docs_per_second", "not_set"));
assertThat(settingsAsMap.getOrDefault("dates_as_epoch_millis", "not_set"), equalTo("not_set"));

config = new SettingsConfig.Builder().setDatesAsEpochMillis(null).build();
// returns false, however it's `null` as in "use default", checked next
assertFalse(config.getDatesAsEpochMillis());

settingsAsMap = xContentToMap(config);
assertThat(settingsAsMap.getOrDefault("max_page_search_size", "not_set"), equalTo("not_set"));
assertThat(settingsAsMap.getOrDefault("docs_per_second", "not_set"), equalTo("not_set"));
assertNull(settingsAsMap.getOrDefault("dates_as_epoch_millis", "not_set"));
}

private Map<String, Object> xContentToMap(ToXContent xcontent) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ public class SettingsConfigTests extends AbstractResponseTestCase<
public static org.elasticsearch.xpack.core.transform.transforms.SettingsConfig randomSettingsConfig() {
return new org.elasticsearch.xpack.core.transform.transforms.SettingsConfig(
randomBoolean() ? null : randomIntBetween(10, 10_000),
randomBoolean() ? null : randomFloat()
randomBoolean() ? null : randomFloat(),
randomBoolean() ? null : randomIntBetween(0, 1)
);
}

Expand All @@ -43,6 +44,7 @@ public static void assertHlrcEquals(
) {
assertEquals(serverTestInstance.getMaxPageSearchSize(), clientInstance.getMaxPageSearchSize());
assertEquals(serverTestInstance.getDocsPerSecond(), clientInstance.getDocsPerSecond());
assertEquals(serverTestInstance.getDatesAsEpochMillis(), clientInstance.getDatesAsEpochMillis());
}

@Override
Expand Down
10 changes: 10 additions & 0 deletions docs/reference/migration/migrate_7_11.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,13 @@ will be applied to the values of a keyword field; for example, a
field configured with a lowercase normalizer will return highlighted
snippets in lower case.
====
[discrete]
[[breaking_711_api_changes]]
=== API changes

[discrete]
==== {dataframe-transform-cap} API changes

Transform no longer writes dates used in a `group_by` as `epoch_millis` but as
formatted ISO string. Previously constructed transforms will still use `epoch_millis`.
You can configure and change the output format in the settings of the transform.
15 changes: 11 additions & 4 deletions docs/reference/rest-api/common-parms.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,10 @@ The destination for the {transform}.
end::dest[]

tag::dest-index[]
The _destination index_ for the {transform}. The mappings of the destination
index are deduced based on the source fields when possible. If alternate
mappings are required, use the
https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-create-index.html[Create index API]
The _destination index_ for the {transform}. The mappings of the destination
index are deduced based on the source fields when possible. If alternate
mappings are required, use the
https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-create-index.html[Create index API]
prior to starting the {transform}.
end::dest-index[]

Expand Down Expand Up @@ -971,6 +971,13 @@ tag::transform-settings[]
Defines optional {transform} settings.
end::transform-settings[]

tag::transform-settings-dates-as-epoch-milli[]
Defines if dates in the ouput should be written as ISO formatted string (default)
or as millis since epoch. `epoch_millis` has been the default for transforms created
before version `7.11`. For compatible output set this to `true`.
The default value is `false`.
end::transform-settings-dates-as-epoch-milli[]

tag::transform-settings-docs-per-second[]
Specifies a limit on the number of input documents per second. This setting
throttles the {transform} by adding a wait time between search requests. The
Expand Down
7 changes: 5 additions & 2 deletions docs/reference/transform/apis/put-transform.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Instantiates a {transform}.
[[put-transform-prereqs]]
== {api-prereq-title}

If the {es} {security-features} are enabled, you must have the following
If the {es} {security-features} are enabled, you must have the following
built-in roles and privileges:

* `transform_admin`
Expand Down Expand Up @@ -134,6 +134,9 @@ include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=transform-settings]
.Properties of `settings`
[%collapsible%open]
====
`dates_as_epoch_millis`:::
(Optional, boolean)
include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=transform-settings-dates-as-epoch-milli]
`docs_per_second`:::
(Optional, float)
include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=transform-settings-docs-per-second]
Expand Down Expand Up @@ -183,7 +186,7 @@ include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=sync-time]
`delay`::::
(Optional, <<time-units, time units>>)
include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=sync-time-delay]

`field`::::
(Required, string)
include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=sync-time-field]
Expand Down
7 changes: 5 additions & 2 deletions docs/reference/transform/apis/update-transform.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Updates certain properties of a {transform}.
[[update-transform-prereqs]]
== {api-prereq-title}

If the {es} {security-features} are enabled, you must have the following
If the {es} {security-features} are enabled, you must have the following
built-in roles and privileges:

* `transform_admin`
Expand Down Expand Up @@ -110,6 +110,9 @@ include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=transform-settings]
.Properties of `settings`
[%collapsible%open]
====
`dates_as_epoch_millis`:::
(Optional, boolean)
include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=transform-settings-dates-as-epoch-milli]
`docs_per_second`:::
(Optional, float)
include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=transform-settings-docs-per-second]
Expand All @@ -131,7 +134,7 @@ include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=source-transforms]
`index`:::
(Required, string or array)
include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=source-index-transforms]
`query`:::
(Optional, object)
include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=source-query-transforms]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,7 @@ public enum ValueType {
INT(VALUE_NUMBER, VALUE_STRING),
INT_OR_NULL(VALUE_NUMBER, VALUE_STRING, VALUE_NULL),
BOOLEAN(VALUE_BOOLEAN, VALUE_STRING),
BOOLEAN_OR_NULL(VALUE_BOOLEAN, VALUE_STRING, VALUE_NULL),
STRING_ARRAY(START_ARRAY, VALUE_STRING),
FLOAT_ARRAY(START_ARRAY, VALUE_NUMBER, VALUE_STRING),
DOUBLE_ARRAY(START_ARRAY, VALUE_NUMBER, VALUE_STRING),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public final class TransformField {
public static final ParseField FORCE = new ParseField("force");
public static final ParseField MAX_PAGE_SEARCH_SIZE = new ParseField("max_page_search_size");
public static final ParseField DOCS_PER_SECOND = new ParseField("docs_per_second");
public static final ParseField DATES_AS_EPOCH_MILLIS = new ParseField("dates_as_epoch_millis");
public static final ParseField FIELD = new ParseField("field");
public static final ParseField SYNC = new ParseField("sync");
public static final ParseField TIME_BASED_SYNC = new ParseField("time");
Expand Down
Loading

0 comments on commit 8e377da

Please sign in to comment.