Skip to content

Commit ebabeb6

Browse files
committed
TRT-2487: job-run-aggregator: remove relaxing in disruption aggregation
I do not _entirely_ understand why the relaxation for disruption tests exists here but the TODO exists this may be worth tightening. The `pityFactor` will relax the threshold in some cases (high attempts, high pass rate) so the additional relaxation would only apply in the remaining cases.
1 parent c96dc28 commit ebabeb6

2 files changed

Lines changed: 8 additions & 15 deletions

File tree

pkg/jobrunaggregator/jobrunaggregatoranalyzer/pass_fail.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -490,12 +490,6 @@ func (a *weeklyAverageFromTenDays) innerCheckPercentileDisruptionWithGrace(
490490
strictRequiredNumberOfPasses := requiredPassesByPassPercentageByNumberOfAttempts[numberOfAttempts][workingPercentage]
491491
requiredNumberOfPasses, pityFactorMsg := pityFactor(numberOfAttempts, strictRequiredNumberOfPasses)
492492

493-
if requiredNumberOfPasses == strictRequiredNumberOfPasses {
494-
// TODO try to tighten this after we can keep the test in for about a week.
495-
// We need to come back and revisit the possibility of removing this adjustment.
496-
requiredNumberOfPasses = requiredNumberOfPasses - 1 // subtracting one because our current sample missed by one
497-
}
498-
499493
if requiredNumberOfPasses <= 0 {
500494
message := fmt.Sprintf("Current percentile is so low that we cannot latch, skipping (P%d=%.2fs successes=%v failures=%v)", thresholdPercentile, threshold, successRuns, failureRuns)
501495
failureJobRunIDs = sets.StringKeySet(jobRunIDToAvailabilityResultForBackend).List()

pkg/jobrunaggregator/jobrunaggregatoranalyzer/pass_fail_test.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -500,16 +500,15 @@ func TestInnerCheckPercentileDisruptionWithPityFactor(t *testing.T) {
500500
}{
501501
// Note: In innerCheckPercentileDisruptionWithGrace, workingPercentage = thresholdPercentile
502502
// So a P95 test uses 95% as the working percentage for calculating required passes
503-
// The code also has -1 adjustment when pity factor doesn't relax the requirement
504503
{
505504
name: "10 attempts, P95 (95% working): 8 passes, 2 failures - exceeds requirement",
506505
disruptions: []int{0, 0, 0, 1, 1, 1, 1, 1, 5, 5},
507506
thresholdPercentile: 95,
508507
historicalDisruption: 2.0,
509508
graceSeconds: 0,
510509
expectedStatus: testCasePassed,
511-
expectedMinPasses: 6, // strict=7, pity=min(10-2,7)=7, then 7-1=6 (no relaxation)
512-
description: "Strict=7, pity=min(10-2,7)=7, -1 adj=6. 8 passes exceeds requirement",
510+
expectedMinPasses: 7, // strict=7, pity=min(10-2,7)=7 (no relaxation)
511+
description: "Strict=7, pity=min(10-2,7)=7. 8 passes exceeds requirement",
513512
},
514513
{
515514
name: "10 attempts, P95 (95% working): 7 passes, 3 failures - meets requirement",
@@ -518,8 +517,8 @@ func TestInnerCheckPercentileDisruptionWithPityFactor(t *testing.T) {
518517
historicalDisruption: 2.0,
519518
graceSeconds: 0,
520519
expectedStatus: testCasePassed,
521-
expectedMinPasses: 6, // strict=7, pity=min(10-2,7)=7, then 7-1=6 (no relaxation)
522-
description: "Strict=7, pity=min(10-2,7)=7, -1 adj=6. 7 passes exceeds requirement",
520+
expectedMinPasses: 7, // strict=7, pity=min(10-2,7)=7 (no relaxation)
521+
description: "Strict=7, pity=min(10-2,7)=7. 7 passes meets requirement exactly",
523522
},
524523
{
525524
name: "6 attempts, P80 (80% working): 4 passes, 2 failures - exceeds requirement",
@@ -528,8 +527,8 @@ func TestInnerCheckPercentileDisruptionWithPityFactor(t *testing.T) {
528527
historicalDisruption: 2.0,
529528
graceSeconds: 0,
530529
expectedStatus: testCasePassed,
531-
expectedMinPasses: 1, // strict=2, pity=min(6-2,2)=2, then 2-1=1 (no relaxation)
532-
description: "Strict=2, pity=min(6-2,2)=2, -1 adj=1. 4 passes exceeds requirement",
530+
expectedMinPasses: 2, // strict=2, pity=min(6-2,2)=2 (no relaxation)
531+
description: "Strict=2, pity=min(6-2,2)=2. 4 passes exceeds requirement",
533532
},
534533
{
535534
name: "12 attempts, P95 (95% working): 10 passes, 2 failures - exceeds requirement",
@@ -538,8 +537,8 @@ func TestInnerCheckPercentileDisruptionWithPityFactor(t *testing.T) {
538537
historicalDisruption: 2.0,
539538
graceSeconds: 0,
540539
expectedStatus: testCasePassed,
541-
expectedMinPasses: 8, // strict=9, pity=min(12-2,9)=9, then 9-1=8 (no relaxation)
542-
description: "Strict=9, pity=min(12-2,9)=9, -1 adj=8. 10 passes exceeds requirement",
540+
expectedMinPasses: 9, // strict=9, pity=min(12-2,9)=9 (no relaxation)
541+
description: "Strict=9, pity=min(12-2,9)=9. 10 passes exceeds requirement",
543542
},
544543
}
545544

0 commit comments

Comments
 (0)