Skip to content

Commit 49f41d1

Browse files
author
Hendrik Muhs
committed
[Transform] fix NPE in derive stats if shouldStopAtNextCheckpo… (#52940)
fixes a NPE in _stats in case shouldStopAtNextCheckpoint is set.
1 parent d102158 commit 49f41d1

File tree

2 files changed

+109
-50
lines changed

2 files changed

+109
-50
lines changed

‎x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportGetTransformStatsAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.elasticsearch.cluster.ClusterState;
1919
import org.elasticsearch.cluster.service.ClusterService;
2020
import org.elasticsearch.common.Nullable;
21+
import org.elasticsearch.common.Strings;
2122
import org.elasticsearch.common.inject.Inject;
2223
import org.elasticsearch.index.IndexNotFoundException;
2324
import org.elasticsearch.persistent.PersistentTasksCustomMetaData;
@@ -189,7 +190,7 @@ static TransformStats deriveStats(TransformTask task, @Nullable TransformCheckpo
189190
&& derivedState.equals(TransformStats.State.STOPPED) == false
190191
&& derivedState.equals(TransformStats.State.FAILED) == false) {
191192
derivedState = TransformStats.State.STOPPING;
192-
reason = reason.isEmpty() ? "transform is set to stop at the next checkpoint" : reason;
193+
reason = Strings.isNullOrEmpty(reason) ? "transform is set to stop at the next checkpoint" : reason;
193194
}
194195
return new TransformStats(
195196
task.getTransformId(),

‎x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/action/TransportGetTransformStatsActionTests.java

Lines changed: 107 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -28,101 +28,159 @@ public class TransportGetTransformStatsActionTests extends ESTestCase {
2828

2929
public void testDeriveStatsStopped() {
3030
String transformId = "transform-with-stats";
31-
String reason = "";
31+
String reason = null;
3232
TransformIndexerStats stats = TransformIndexerStatsTests.randomStats();
33-
TransformState stoppedState =
34-
new TransformState(TransformTaskState.STOPPED, IndexerState.STOPPED, null, 0, reason, null, null, true);
33+
TransformState stoppedState = new TransformState(
34+
TransformTaskState.STOPPED,
35+
IndexerState.STOPPED,
36+
null,
37+
0,
38+
reason,
39+
null,
40+
null,
41+
true
42+
);
3543
withIdStateAndStats(transformId, stoppedState, stats);
3644
TransformCheckpointingInfo info = new TransformCheckpointingInfo(
3745
new TransformCheckpointStats(1, null, null, 1, 1),
3846
new TransformCheckpointStats(2, null, null, 2, 5),
3947
2,
40-
Instant.now());
41-
42-
assertThat(TransportGetTransformStatsAction.deriveStats(task, null),
43-
equalTo(new TransformStats(transformId, TransformStats.State.STOPPED, "", null, stats, TransformCheckpointingInfo.EMPTY)));
44-
assertThat(TransportGetTransformStatsAction.deriveStats(task, info),
45-
equalTo(new TransformStats(transformId, TransformStats.State.STOPPED, "", null, stats, info)));
46-
48+
Instant.now()
49+
);
50+
51+
assertThat(
52+
TransportGetTransformStatsAction.deriveStats(task, null),
53+
equalTo(new TransformStats(transformId, TransformStats.State.STOPPED, reason, null, stats, TransformCheckpointingInfo.EMPTY))
54+
);
55+
assertThat(
56+
TransportGetTransformStatsAction.deriveStats(task, info),
57+
equalTo(new TransformStats(transformId, TransformStats.State.STOPPED, reason, null, stats, info))
58+
);
4759

4860
reason = "foo";
4961
stoppedState = new TransformState(TransformTaskState.STOPPED, IndexerState.STOPPED, null, 0, reason, null, null, true);
5062
withIdStateAndStats(transformId, stoppedState, stats);
5163

52-
assertThat(TransportGetTransformStatsAction.deriveStats(task, null),
53-
equalTo(new TransformStats(transformId, TransformStats.State.STOPPED, reason, null, stats, TransformCheckpointingInfo.EMPTY)));
54-
assertThat(TransportGetTransformStatsAction.deriveStats(task, info),
55-
equalTo(new TransformStats(transformId, TransformStats.State.STOPPED, reason, null, stats, info)));
64+
assertThat(
65+
TransportGetTransformStatsAction.deriveStats(task, null),
66+
equalTo(new TransformStats(transformId, TransformStats.State.STOPPED, reason, null, stats, TransformCheckpointingInfo.EMPTY))
67+
);
68+
assertThat(
69+
TransportGetTransformStatsAction.deriveStats(task, info),
70+
equalTo(new TransformStats(transformId, TransformStats.State.STOPPED, reason, null, stats, info))
71+
);
5672
}
5773

5874
public void testDeriveStatsFailed() {
5975
String transformId = "transform-with-stats";
60-
String reason = "";
76+
String reason = null;
6177
TransformIndexerStats stats = TransformIndexerStatsTests.randomStats();
62-
TransformState failedState =
63-
new TransformState(TransformTaskState.FAILED, IndexerState.STOPPED, null, 0, reason, null, null, true);
78+
TransformState failedState = new TransformState(TransformTaskState.FAILED, IndexerState.STOPPED, null, 0, reason, null, null, true);
6479
withIdStateAndStats(transformId, failedState, stats);
6580
TransformCheckpointingInfo info = new TransformCheckpointingInfo(
6681
new TransformCheckpointStats(1, null, null, 1, 1),
6782
new TransformCheckpointStats(2, null, null, 2, 5),
6883
2,
69-
Instant.now());
70-
71-
assertThat(TransportGetTransformStatsAction.deriveStats(task, null),
72-
equalTo(new TransformStats(transformId, TransformStats.State.FAILED, "", null, stats, TransformCheckpointingInfo.EMPTY)));
73-
assertThat(TransportGetTransformStatsAction.deriveStats(task, info),
74-
equalTo(new TransformStats(transformId, TransformStats.State.FAILED, "", null, stats, info)));
75-
84+
Instant.now()
85+
);
86+
87+
assertThat(
88+
TransportGetTransformStatsAction.deriveStats(task, null),
89+
equalTo(new TransformStats(transformId, TransformStats.State.FAILED, reason, null, stats, TransformCheckpointingInfo.EMPTY))
90+
);
91+
assertThat(
92+
TransportGetTransformStatsAction.deriveStats(task, info),
93+
equalTo(new TransformStats(transformId, TransformStats.State.FAILED, reason, null, stats, info))
94+
);
7695

7796
reason = "the task is failed";
7897
failedState = new TransformState(TransformTaskState.FAILED, IndexerState.STOPPED, null, 0, reason, null, null, true);
7998
withIdStateAndStats(transformId, failedState, stats);
8099

81-
assertThat(TransportGetTransformStatsAction.deriveStats(task, null),
82-
equalTo(new TransformStats(transformId, TransformStats.State.FAILED, reason, null, stats, TransformCheckpointingInfo.EMPTY)));
83-
assertThat(TransportGetTransformStatsAction.deriveStats(task, info),
84-
equalTo(new TransformStats(transformId, TransformStats.State.FAILED, reason, null, stats, info)));
100+
assertThat(
101+
TransportGetTransformStatsAction.deriveStats(task, null),
102+
equalTo(new TransformStats(transformId, TransformStats.State.FAILED, reason, null, stats, TransformCheckpointingInfo.EMPTY))
103+
);
104+
assertThat(
105+
TransportGetTransformStatsAction.deriveStats(task, info),
106+
equalTo(new TransformStats(transformId, TransformStats.State.FAILED, reason, null, stats, info))
107+
);
85108
}
86109

87-
88110
public void testDeriveStats() {
89111
String transformId = "transform-with-stats";
90-
String reason = "";
112+
String reason = null;
91113
TransformIndexerStats stats = TransformIndexerStatsTests.randomStats();
92-
TransformState runningState =
93-
new TransformState(TransformTaskState.STARTED, IndexerState.INDEXING, null, 0, reason, null, null, true);
114+
TransformState runningState = new TransformState(
115+
TransformTaskState.STARTED,
116+
IndexerState.INDEXING,
117+
null,
118+
0,
119+
reason,
120+
null,
121+
null,
122+
true
123+
);
94124
withIdStateAndStats(transformId, runningState, stats);
95125
TransformCheckpointingInfo info = new TransformCheckpointingInfo(
96126
new TransformCheckpointStats(1, null, null, 1, 1),
97127
new TransformCheckpointStats(2, null, null, 2, 5),
98128
2,
99-
Instant.now());
100-
101-
assertThat(TransportGetTransformStatsAction.deriveStats(task, null),
102-
equalTo(new TransformStats(transformId, TransformStats.State.STOPPING,
103-
"transform is set to stop at the next checkpoint", null, stats, TransformCheckpointingInfo.EMPTY)));
104-
assertThat(TransportGetTransformStatsAction.deriveStats(task, info),
105-
equalTo(new TransformStats(transformId, TransformStats.State.STOPPING,
106-
"transform is set to stop at the next checkpoint", null, stats, info)));
107-
129+
Instant.now()
130+
);
131+
132+
assertThat(
133+
TransportGetTransformStatsAction.deriveStats(task, null),
134+
equalTo(
135+
new TransformStats(
136+
transformId,
137+
TransformStats.State.STOPPING,
138+
"transform is set to stop at the next checkpoint",
139+
null,
140+
stats,
141+
TransformCheckpointingInfo.EMPTY
142+
)
143+
)
144+
);
145+
assertThat(
146+
TransportGetTransformStatsAction.deriveStats(task, info),
147+
equalTo(
148+
new TransformStats(
149+
transformId,
150+
TransformStats.State.STOPPING,
151+
"transform is set to stop at the next checkpoint",
152+
null,
153+
stats,
154+
info
155+
)
156+
)
157+
);
108158

109159
reason = "foo";
110160
runningState = new TransformState(TransformTaskState.STARTED, IndexerState.INDEXING, null, 0, reason, null, null, true);
111161
withIdStateAndStats(transformId, runningState, stats);
112162

113-
assertThat(TransportGetTransformStatsAction.deriveStats(task, null),
114-
equalTo(new TransformStats(transformId, TransformStats.State.STOPPING, reason, null, stats, TransformCheckpointingInfo.EMPTY)));
115-
assertThat(TransportGetTransformStatsAction.deriveStats(task, info),
116-
equalTo(new TransformStats(transformId, TransformStats.State.STOPPING, reason, null, stats, info)));
163+
assertThat(
164+
TransportGetTransformStatsAction.deriveStats(task, null),
165+
equalTo(new TransformStats(transformId, TransformStats.State.STOPPING, reason, null, stats, TransformCheckpointingInfo.EMPTY))
166+
);
167+
assertThat(
168+
TransportGetTransformStatsAction.deriveStats(task, info),
169+
equalTo(new TransformStats(transformId, TransformStats.State.STOPPING, reason, null, stats, info))
170+
);
117171

118172
// Stop at next checkpoint is false.
119173
runningState = new TransformState(TransformTaskState.STARTED, IndexerState.INDEXING, null, 0, reason, null, null, false);
120174
withIdStateAndStats(transformId, runningState, stats);
121175

122-
assertThat(TransportGetTransformStatsAction.deriveStats(task, null),
123-
equalTo(new TransformStats(transformId, TransformStats.State.INDEXING, reason, null, stats, TransformCheckpointingInfo.EMPTY)));
124-
assertThat(TransportGetTransformStatsAction.deriveStats(task, info),
125-
equalTo(new TransformStats(transformId, TransformStats.State.INDEXING, reason, null, stats, info)));
176+
assertThat(
177+
TransportGetTransformStatsAction.deriveStats(task, null),
178+
equalTo(new TransformStats(transformId, TransformStats.State.INDEXING, reason, null, stats, TransformCheckpointingInfo.EMPTY))
179+
);
180+
assertThat(
181+
TransportGetTransformStatsAction.deriveStats(task, info),
182+
equalTo(new TransformStats(transformId, TransformStats.State.INDEXING, reason, null, stats, info))
183+
);
126184
}
127185

128186
private void withIdStateAndStats(String transformId, TransformState state, TransformIndexerStats stats) {

0 commit comments

Comments
 (0)