Skip to content

Commit c8974aa

Browse files
author
David Roberts
authored
[ML-DataFrame] Remove ID field from data frame indexer stats (#44768)
This is a followup to #44350. The indexer stats used to be persisted standalone, but now are only persisted as part of a state-and-stats document. During the review of #44350 it was decided that we'll stick with this design, so there will never be a need for an indexer stats object to store its transform ID as it is stored on the enclosing document. This PR removes the indexer stats document ID.
1 parent 06dea85 commit c8974aa

File tree

14 files changed

+46
-99
lines changed

14 files changed

+46
-99
lines changed

‎client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/hlrc/DataFrameIndexerTransformStatsTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public class DataFrameIndexerTransformStatsTests extends AbstractHlrcXContentTes
3131

3232
public static DataFrameIndexerTransformStats fromHlrc(
3333
org.elasticsearch.client.dataframe.transforms.DataFrameIndexerTransformStats instance) {
34-
return DataFrameIndexerTransformStats.withDefaultTransformId(instance.getNumPages(), instance.getNumDocuments(),
34+
return new DataFrameIndexerTransformStats(instance.getNumPages(), instance.getNumDocuments(),
3535
instance.getOutputDocuments(), instance.getNumInvocations(), instance.getIndexTime(), instance.getSearchTime(),
3636
instance.getIndexTotal(), instance.getSearchTotal(), instance.getIndexFailures(), instance.getSearchFailures());
3737
}
@@ -48,16 +48,16 @@ public DataFrameIndexerTransformStats convertHlrcToInternal(
4848
return fromHlrc(instance);
4949
}
5050

51-
public static DataFrameIndexerTransformStats randomStats(String transformId) {
52-
return new DataFrameIndexerTransformStats(transformId, randomLongBetween(10L, 10000L),
51+
public static DataFrameIndexerTransformStats randomStats() {
52+
return new DataFrameIndexerTransformStats(randomLongBetween(10L, 10000L),
5353
randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L),
5454
randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L),
5555
randomLongBetween(0L, 10000L));
5656
}
5757

5858
@Override
5959
protected DataFrameIndexerTransformStats createTestInstance() {
60-
return randomStats(DataFrameIndexerTransformStats.DEFAULT_TRANSFORM_ID);
60+
return randomStats();
6161
}
6262

6363
@Override

‎client/rest-high-level/src/test/java/org/elasticsearch/client/dataframe/transforms/hlrc/DataFrameTransformStatsTests.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@ public static DataFrameTransformStats randomDataFrameTransformStats() {
7979
randomFrom(DataFrameTransformTaskState.values()),
8080
randomBoolean() ? null : randomAlphaOfLength(100),
8181
randomBoolean() ? null : randomNodeAttributes(),
82-
// TODO: remove this ID field from the server side as it's no longer needed
83-
randomStats("_all"),
82+
randomStats(),
8483
DataFrameTransformCheckpointingInfoTests.randomDataFrameTransformCheckpointingInfo());
8584
}
8685

@@ -132,8 +131,8 @@ public static NodeAttributes randomNodeAttributes() {
132131
attributes);
133132
}
134133

135-
public static DataFrameIndexerTransformStats randomStats(String transformId) {
136-
return new DataFrameIndexerTransformStats(transformId, randomLongBetween(10L, 10000L),
134+
public static DataFrameIndexerTransformStats randomStats() {
135+
return new DataFrameIndexerTransformStats(randomLongBetween(10L, 10000L),
137136
randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L),
138137
randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L),
139138
randomLongBetween(0L, 10000L));

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/dataframe/transforms/DataFrameIndexerTransformStats.java‎

Lines changed: 18 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,23 @@
66

77
package org.elasticsearch.xpack.core.dataframe.transforms;
88

9-
import org.elasticsearch.common.Nullable;
9+
import org.elasticsearch.Version;
1010
import org.elasticsearch.common.ParseField;
1111
import org.elasticsearch.common.io.stream.StreamInput;
1212
import org.elasticsearch.common.io.stream.StreamOutput;
1313
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
1414
import org.elasticsearch.common.xcontent.XContentBuilder;
1515
import org.elasticsearch.common.xcontent.XContentParser;
16-
import org.elasticsearch.xpack.core.dataframe.DataFrameField;
1716
import org.elasticsearch.xpack.core.indexing.IndexerJobStats;
1817

1918
import java.io.IOException;
2019
import java.util.Objects;
2120

2221
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
23-
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
2422

2523
public class DataFrameIndexerTransformStats extends IndexerJobStats {
26-
public static final String DEFAULT_TRANSFORM_ID = "_all";
24+
25+
private static final String DEFAULT_TRANSFORM_ID = "_all"; // TODO remove when no longer needed for wire BWC
2726

2827
public static final String NAME = "data_frame_indexer_transform_stats";
2928
public static ParseField NUM_PAGES = new ParseField("pages_processed");
@@ -39,12 +38,11 @@ public class DataFrameIndexerTransformStats extends IndexerJobStats {
3938

4039
private static final ConstructingObjectParser<DataFrameIndexerTransformStats, Void> LENIENT_PARSER = new ConstructingObjectParser<>(
4140
NAME, true,
42-
args -> new DataFrameIndexerTransformStats(args[0] != null ? (String) args[0] : DEFAULT_TRANSFORM_ID,
43-
(long) args[1], (long) args[2], (long) args[3], (long) args[4], (long) args[5], (long) args[6], (long) args[7],
44-
(long) args[8], (long) args[9], (long) args[10]));
41+
args -> new DataFrameIndexerTransformStats(
42+
(long) args[0], (long) args[1], (long) args[2], (long) args[3], (long) args[4], (long) args[5], (long) args[6],
43+
(long) args[7], (long) args[8], (long) args[9]));
4544

4645
static {
47-
LENIENT_PARSER.declareString(optionalConstructorArg(), DataFrameField.ID);
4846
LENIENT_PARSER.declareLong(constructorArg(), NUM_PAGES);
4947
LENIENT_PARSER.declareLong(constructorArg(), NUM_INPUT_DOCUMENTS);
5048
LENIENT_PARSER.declareLong(constructorArg(), NUM_OUTPUT_DOCUMENTS);
@@ -57,60 +55,38 @@ public class DataFrameIndexerTransformStats extends IndexerJobStats {
5755
LENIENT_PARSER.declareLong(constructorArg(), SEARCH_FAILURES);
5856
}
5957

60-
private final String transformId;
61-
6258
/**
63-
* Certain situations call for a default transform ID, e.g. when merging many different transforms for statistics gather.
64-
*
65-
* The returned stats object cannot be stored in the index as the transformId does not refer to a real transform configuration
66-
*
67-
* @return new DataFrameIndexerTransformStats with empty stats and a default transform ID
59+
* Create with all stats set to zero
6860
*/
69-
public static DataFrameIndexerTransformStats withDefaultTransformId() {
70-
return new DataFrameIndexerTransformStats(DEFAULT_TRANSFORM_ID);
71-
}
72-
73-
public static DataFrameIndexerTransformStats withDefaultTransformId(long numPages, long numInputDocuments, long numOutputDocuments,
74-
long numInvocations, long indexTime, long searchTime,
75-
long indexTotal, long searchTotal, long indexFailures,
76-
long searchFailures) {
77-
return new DataFrameIndexerTransformStats(DEFAULT_TRANSFORM_ID, numPages, numInputDocuments,
78-
numOutputDocuments, numInvocations, indexTime, searchTime, indexTotal, searchTotal,
79-
indexFailures, searchFailures);
80-
}
81-
82-
public DataFrameIndexerTransformStats(String transformId) {
61+
public DataFrameIndexerTransformStats() {
8362
super();
84-
this.transformId = Objects.requireNonNull(transformId, "parameter transformId must not be null");
8563
}
8664

87-
public DataFrameIndexerTransformStats(String transformId, long numPages, long numInputDocuments, long numOutputDocuments,
65+
public DataFrameIndexerTransformStats(long numPages, long numInputDocuments, long numOutputDocuments,
8866
long numInvocations, long indexTime, long searchTime, long indexTotal, long searchTotal,
8967
long indexFailures, long searchFailures) {
9068
super(numPages, numInputDocuments, numOutputDocuments, numInvocations, indexTime, searchTime, indexTotal, searchTotal,
9169
indexFailures, searchFailures);
92-
this.transformId = Objects.requireNonNull(transformId, "parameter transformId must not be null");
9370
}
9471

9572
public DataFrameIndexerTransformStats(DataFrameIndexerTransformStats other) {
96-
this(other.transformId, other.numPages, other.numInputDocuments, other.numOuputDocuments, other.numInvocations,
73+
this(other.numPages, other.numInputDocuments, other.numOuputDocuments, other.numInvocations,
9774
other.indexTime, other.searchTime, other.indexTotal, other.searchTotal, other.indexFailures, other.searchFailures);
9875
}
9976

10077
public DataFrameIndexerTransformStats(StreamInput in) throws IOException {
10178
super(in);
102-
transformId = in.readString();
79+
if (in.getVersion().before(Version.V_8_0_0)) { // TODO change to 7.4.0 after backport
80+
in.readString(); // was transformId
81+
}
10382
}
10483

10584
@Override
10685
public void writeTo(StreamOutput out) throws IOException {
10786
super.writeTo(out);
108-
out.writeString(transformId);
109-
}
110-
111-
@Nullable
112-
public String getTransformId() {
113-
return transformId;
87+
if (out.getVersion().before(Version.V_8_0_0)) { // TODO change to 7.4.0 after backport
88+
out.writeString(DEFAULT_TRANSFORM_ID);
89+
}
11490
}
11591

11692
@Override
@@ -126,21 +102,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
126102
builder.field(SEARCH_TIME_IN_MS.getPreferredName(), searchTime);
127103
builder.field(SEARCH_TOTAL.getPreferredName(), searchTotal);
128104
builder.field(SEARCH_FAILURES.getPreferredName(), searchFailures);
129-
if (params.paramAsBoolean(DataFrameField.FOR_INTERNAL_STORAGE, false)) {
130-
// If we are storing something, it should have a valid transform ID.
131-
if (transformId.equals(DEFAULT_TRANSFORM_ID)) {
132-
throw new IllegalArgumentException("when storing transform statistics, a valid transform id must be provided");
133-
}
134-
builder.field(DataFrameField.ID.getPreferredName(), transformId);
135-
}
136105
builder.endObject();
137106
return builder;
138107
}
139108

140109
public DataFrameIndexerTransformStats merge(DataFrameIndexerTransformStats other) {
141-
// We should probably not merge two sets of stats unless one is an accumulation object (i.e. with the default transform id)
142-
// or the stats are referencing the same transform
143-
assert transformId.equals(DEFAULT_TRANSFORM_ID) || this.transformId.equals(other.transformId);
144110
numPages += other.numPages;
145111
numInputDocuments += other.numInputDocuments;
146112
numOuputDocuments += other.numOuputDocuments;
@@ -167,8 +133,7 @@ public boolean equals(Object other) {
167133

168134
DataFrameIndexerTransformStats that = (DataFrameIndexerTransformStats) other;
169135

170-
return Objects.equals(this.transformId, that.transformId)
171-
&& Objects.equals(this.numPages, that.numPages)
136+
return Objects.equals(this.numPages, that.numPages)
172137
&& Objects.equals(this.numInputDocuments, that.numInputDocuments)
173138
&& Objects.equals(this.numOuputDocuments, that.numOuputDocuments)
174139
&& Objects.equals(this.numInvocations, that.numInvocations)
@@ -182,7 +147,7 @@ public boolean equals(Object other) {
182147

183148
@Override
184149
public int hashCode() {
185-
return Objects.hash(transformId, numPages, numInputDocuments, numOuputDocuments, numInvocations,
150+
return Objects.hash(numPages, numInputDocuments, numOuputDocuments, numInvocations,
186151
indexTime, searchTime, indexFailures, searchFailures, indexTotal, searchTotal);
187152
}
188153

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/dataframe/transforms/DataFrameTransformStats.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public static DataFrameTransformStats fromXContent(XContentParser parser) throws
7575
}
7676

7777
public static DataFrameTransformStats initialStats(String id) {
78-
return stoppedStats(id, new DataFrameIndexerTransformStats(id));
78+
return stoppedStats(id, new DataFrameIndexerTransformStats());
7979
}
8080

8181
public static DataFrameTransformStats stoppedStats(String id, DataFrameIndexerTransformStats indexerTransformStats) {

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/DataFrameIndexerTransformStatsTests.java‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,13 @@
77
package org.elasticsearch.xpack.core.dataframe.transforms;
88

99
import org.elasticsearch.common.io.stream.Writeable;
10-
import org.elasticsearch.common.xcontent.ToXContent;
1110
import org.elasticsearch.common.xcontent.XContentParser;
1211
import org.elasticsearch.test.AbstractSerializingTestCase;
13-
import org.elasticsearch.xpack.core.dataframe.DataFrameField;
1412

1513
import java.io.IOException;
16-
import java.util.Collections;
1714

1815
public class DataFrameIndexerTransformStatsTests extends AbstractSerializingTestCase<DataFrameIndexerTransformStats> {
1916

20-
protected static ToXContent.Params TO_XCONTENT_PARAMS = new ToXContent.MapParams(
21-
Collections.singletonMap(DataFrameField.FOR_INTERNAL_STORAGE, "true"));
22-
2317
@Override
2418
protected DataFrameIndexerTransformStats createTestInstance() {
2519
return randomStats();
@@ -36,36 +30,26 @@ protected DataFrameIndexerTransformStats doParseInstance(XContentParser parser)
3630
}
3731

3832
public static DataFrameIndexerTransformStats randomStats() {
39-
return randomStats(randomAlphaOfLength(10));
40-
}
41-
42-
public static DataFrameIndexerTransformStats randomStats(String transformId) {
43-
return new DataFrameIndexerTransformStats(transformId, randomLongBetween(10L, 10000L),
33+
return new DataFrameIndexerTransformStats(randomLongBetween(10L, 10000L),
4434
randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L),
4535
randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L), randomLongBetween(0L, 10000L),
4636
randomLongBetween(0L, 10000L));
4737
}
4838

49-
@Override
50-
protected ToXContent.Params getToXContentParams() {
51-
return TO_XCONTENT_PARAMS;
52-
}
53-
5439
public void testMerge() throws IOException {
55-
String transformId = randomAlphaOfLength(10);
56-
DataFrameIndexerTransformStats emptyStats = new DataFrameIndexerTransformStats(transformId);
57-
DataFrameIndexerTransformStats randomStats = randomStats(transformId);
40+
DataFrameIndexerTransformStats emptyStats = new DataFrameIndexerTransformStats();
41+
DataFrameIndexerTransformStats randomStats = randomStats();
5842

5943
assertEquals(randomStats, emptyStats.merge(randomStats));
6044
assertEquals(randomStats, randomStats.merge(emptyStats));
6145

6246
DataFrameIndexerTransformStats randomStatsClone = copyInstance(randomStats);
6347

64-
DataFrameIndexerTransformStats trippleRandomStats = new DataFrameIndexerTransformStats(transformId, 3 * randomStats.getNumPages(),
48+
DataFrameIndexerTransformStats tripleRandomStats = new DataFrameIndexerTransformStats(3 * randomStats.getNumPages(),
6549
3 * randomStats.getNumDocuments(), 3 * randomStats.getOutputDocuments(), 3 * randomStats.getNumInvocations(),
6650
3 * randomStats.getIndexTime(), 3 * randomStats.getSearchTime(), 3 * randomStats.getIndexTotal(),
6751
3 * randomStats.getSearchTotal(), 3 * randomStats.getIndexFailures(), 3 * randomStats.getSearchFailures());
6852

69-
assertEquals(trippleRandomStats, randomStats.merge(randomStatsClone).merge(randomStatsClone));
53+
assertEquals(tripleRandomStats, randomStats.merge(randomStatsClone).merge(randomStatsClone));
7054
}
7155
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/DataFrameTransformStatsTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public static DataFrameTransformStats randomDataFrameTransformStats() {
2020
randomFrom(DataFrameTransformTaskState.values()),
2121
randomBoolean() ? null : randomAlphaOfLength(100),
2222
randomBoolean() ? null : NodeAttributeTests.randomNodeAttributes(),
23-
DataFrameIndexerTransformStatsTests.randomStats(DataFrameIndexerTransformStats.DEFAULT_TRANSFORM_ID),
23+
DataFrameIndexerTransformStatsTests.randomStats(),
2424
DataFrameTransformCheckpointingInfoTests.randomDataFrameTransformCheckpointingInfo());
2525
}
2626

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/DataFrameTransformStoredDocTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class DataFrameTransformStoredDocTests extends AbstractSerializingDataFra
2222
public static DataFrameTransformStoredDoc randomDataFrameTransformStoredDoc(String id) {
2323
return new DataFrameTransformStoredDoc(id,
2424
DataFrameTransformStateTests.randomDataFrameTransformState(),
25-
DataFrameIndexerTransformStatsTests.randomStats(id));
25+
DataFrameIndexerTransformStatsTests.randomStats());
2626
}
2727

2828
public static DataFrameTransformStoredDoc randomDataFrameTransformStoredDoc() {

‎x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/DataFrameInfoTransportAction.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ static DataFrameIndexerTransformStats parseSearchAggs(SearchResponse searchRespo
9393
statisticsList.add(0L);
9494
}
9595
}
96-
return DataFrameIndexerTransformStats.withDefaultTransformId(statisticsList.get(0), // numPages
96+
return new DataFrameIndexerTransformStats(statisticsList.get(0), // numPages
9797
statisticsList.get(1), // numInputDocuments
9898
statisticsList.get(2), // numOutputDocuments
9999
statisticsList.get(3), // numInvocations
@@ -130,7 +130,7 @@ static void getStatisticSummations(Client client, ActionListener<DataFrameIndexe
130130
},
131131
failure -> {
132132
if (failure instanceof ResourceNotFoundException) {
133-
statsListener.onResponse(DataFrameIndexerTransformStats.withDefaultTransformId());
133+
statsListener.onResponse(new DataFrameIndexerTransformStats());
134134
} else {
135135
statsListener.onFailure(failure);
136136
}

‎x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/DataFrameUsageTransportAction.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ protected void masterOperation(Task task, XPackUsageRequest request, ClusterStat
6565
ActionListener<XPackUsageFeatureResponse> listener) {
6666
boolean available = licenseState.isDataFrameAllowed();
6767
if (enabled == false) {
68-
var usage = new DataFrameFeatureSetUsage(available, enabled, Collections.emptyMap(),
69-
DataFrameIndexerTransformStats.withDefaultTransformId());
68+
var usage = new DataFrameFeatureSetUsage(available, enabled, Collections.emptyMap(), new DataFrameIndexerTransformStats());
7069
listener.onResponse(new XPackUsageFeatureResponse(usage));
7170
return;
7271
}
@@ -99,7 +98,7 @@ protected void masterOperation(Task task, XPackUsageRequest request, ClusterStat
9998
long totalTransforms = transformCountSuccess.getHits().getTotalHits().value;
10099
if (totalTransforms == 0) {
101100
var usage = new DataFrameFeatureSetUsage(available, enabled, transformsCountByState,
102-
DataFrameIndexerTransformStats.withDefaultTransformId());
101+
new DataFrameIndexerTransformStats());
103102
listener.onResponse(new XPackUsageFeatureResponse(usage));
104103
return;
105104
}

‎x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/action/TransportPreviewDataFrameTransformAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ private void getPreview(Pivot pivot,
143143
r -> {
144144
try {
145145
final CompositeAggregation agg = r.getAggregations().get(COMPOSITE_AGGREGATION_NAME);
146-
DataFrameIndexerTransformStats stats = DataFrameIndexerTransformStats.withDefaultTransformId();
146+
DataFrameIndexerTransformStats stats = new DataFrameIndexerTransformStats();
147147
// remove all internal fields
148148

149149
if (pipeline == null) {

0 commit comments

Comments
 (0)