Skip to content

Commit 39d4be9

Browse files
Fixed wrong failure calcualtions and related tests
Signed-off-by: Martin Gaievski <[email protected]>
1 parent 3663ef6 commit 39d4be9

File tree

5 files changed

+24
-4
lines changed

5 files changed

+24
-4
lines changed

server/src/main/java/org/opensearch/search/pipeline/Pipeline.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,14 +314,15 @@ <Result extends SearchPhaseResult> void runSearchPhaseResultsTransformer(
314314
e
315315
);
316316
} else {
317+
// Only track pipeline-level failure when processor failure is not ignored
318+
onTransformPhaseResultsFailure();
317319
throw e;
318320
}
319321
}
320322

321323
}
322324
}
323-
} catch (RuntimeException e) {
324-
onTransformPhaseResultsFailure();
325+
} catch (Exception e) {
325326
throw new SearchPipelineProcessingException(e);
326327
} finally {
327328
long took = TimeUnit.NANOSECONDS.toMillis(relativeTimeSupplier.getAsLong() - pipelineStart);

server/src/main/java/org/opensearch/search/pipeline/PipelineWithMetrics.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,18 +249,24 @@ protected void onResponseProcessorFailed(Processor processor) {
249249
protected void beforeTransformPhaseResults() {
250250
super.beforeTransformPhaseResults();
251251
totalPhaseResultsMetrics.before();
252+
// Phase results processing is also tracked as part of individual pipeline request processing
253+
pipelineRequestMetrics.before();
252254
}
253255

254256
@Override
255257
protected void afterTransformPhaseResults(long timeInNanos) {
256258
super.afterTransformPhaseResults(timeInNanos);
257259
totalPhaseResultsMetrics.after(timeInNanos);
260+
// Phase results processing is also tracked as part of individual pipeline request processing
261+
pipelineRequestMetrics.after(timeInNanos);
258262
}
259263

260264
@Override
261265
protected void onTransformPhaseResultsFailure() {
262266
super.onTransformPhaseResultsFailure();
263267
totalPhaseResultsMetrics.failed();
268+
// Phase results processing failures are also tracked as part of individual pipeline request processing
269+
pipelineRequestMetrics.failed();
264270
}
265271

266272
protected void beforePhaseResultsProcessor(Processor processor) {

server/src/main/java/org/opensearch/search/pipeline/SearchPipelineStats.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,28 @@ public class SearchPipelineStats implements Writeable, ToXContentFragment {
3737

3838
private final OperationStats totalRequestStats;
3939
private final OperationStats totalResponseStats;
40+
private final OperationStats totalPhaseResultsStats;
4041
private final List<PerPipelineStats> perPipelineStats;
4142
private final Map<String, PipelineDetailStats> perPipelineProcessorStats;
4243

4344
public SearchPipelineStats(
4445
OperationStats totalRequestStats,
4546
OperationStats totalResponseStats,
47+
OperationStats totalPhaseResultsStats,
4648
List<PerPipelineStats> perPipelineStats,
4749
Map<String, PipelineDetailStats> perPipelineProcessorStats
4850
) {
4951
this.totalRequestStats = totalRequestStats;
5052
this.totalResponseStats = totalResponseStats;
53+
this.totalPhaseResultsStats = totalPhaseResultsStats;
5154
this.perPipelineStats = perPipelineStats;
5255
this.perPipelineProcessorStats = perPipelineProcessorStats;
5356
}
5457

5558
public SearchPipelineStats(StreamInput in) throws IOException {
5659
this.totalRequestStats = new OperationStats(in);
5760
this.totalResponseStats = new OperationStats(in);
61+
this.totalPhaseResultsStats = new OperationStats(in);
5862
int size = in.readVInt();
5963
List<PerPipelineStats> perPipelineStats = new ArrayList<>(size);
6064
Map<String, PipelineDetailStats> pipelineDetailStatsMap = new TreeMap<>();
@@ -152,6 +156,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
152156
public void writeTo(StreamOutput out) throws IOException {
153157
totalRequestStats.writeTo(out);
154158
totalResponseStats.writeTo(out);
159+
totalPhaseResultsStats.writeTo(out);
155160
out.writeVInt(perPipelineStats.size());
156161
for (PerPipelineStats pipelineStat : perPipelineStats) {
157162
out.writeString(pipelineStat.pipelineId);
@@ -259,6 +264,7 @@ SearchPipelineStats build() {
259264
return new SearchPipelineStats(
260265
totalRequestStats,
261266
totalResponseStats,
267+
totalPhaseResultsStats,
262268
unmodifiableList(perPipelineStats),
263269
unmodifiableMap(pipelineDetailStatsMap)
264270
);
@@ -401,6 +407,10 @@ OperationStats getTotalResponseStats() {
401407
return totalResponseStats;
402408
}
403409

410+
OperationStats getTotalPhaseResultsStats() {
411+
return totalPhaseResultsStats;
412+
}
413+
404414
List<PerPipelineStats> getPipelineStats() {
405415
return perPipelineStats;
406416
}

server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,8 +1315,9 @@ public void testStats() throws Exception {
13151315
);
13161316

13171317
SearchPipelineStats stats = searchPipelineService.stats();
1318-
assertPipelineStats(stats.getTotalRequestStats(), 4, 2);
1318+
assertPipelineStats(stats.getTotalRequestStats(), 2, 1);
13191319
assertPipelineStats(stats.getTotalResponseStats(), 2, 1);
1320+
assertPipelineStats(stats.getTotalPhaseResultsStats(), 2, 1);
13201321
for (SearchPipelineStats.PerPipelineStats perPipelineStats : stats.getPipelineStats()) {
13211322
SearchPipelineStats.PipelineDetailStats detailStats = stats.getPerPipelineProcessorStats()
13221323
.get(perPipelineStats.getPipelineId());
@@ -1470,8 +1471,9 @@ public void testStatsEnabledIgnoreFailure() throws Exception {
14701471

14711472
// when ignoreFailure enabled, the search pipelines will all succeed.
14721473
SearchPipelineStats stats = searchPipelineService.stats();
1473-
assertPipelineStats(stats.getTotalRequestStats(), 4, 0);
1474+
assertPipelineStats(stats.getTotalRequestStats(), 2, 0);
14741475
assertPipelineStats(stats.getTotalResponseStats(), 2, 0);
1476+
assertPipelineStats(stats.getTotalPhaseResultsStats(), 2, 0);
14751477

14761478
for (SearchPipelineStats.PerPipelineStats perPipelineStats : stats.getPipelineStats()) {
14771479
SearchPipelineStats.PipelineDetailStats detailStats = stats.getPerPipelineProcessorStats()

server/src/test/java/org/opensearch/search/pipeline/SearchPipelineStatsTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ private static SearchPipelineStats createStats() {
4242
return new SearchPipelineStats(
4343
new OperationStats(1, 2, 3, 4),
4444
new OperationStats(5, 6, 7, 8),
45+
new OperationStats(9, 10, 11, 12),
4546
List.of(
4647
new SearchPipelineStats.PerPipelineStats("p1", new OperationStats(9, 10, 11, 12), new OperationStats(13, 14, 15, 16)),
4748
new SearchPipelineStats.PerPipelineStats("p2", new OperationStats(17, 18, 19, 20), new OperationStats(21, 22, 23, 24))

0 commit comments

Comments
 (0)