From a0ac791515153726b4010d96118adca83eee0c30 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 15 Jun 2022 13:09:40 +0300 Subject: [PATCH 1/4] Fix max duration calculations with multiple 0 vus stages at end In b725f03b we fixed the calculation of the stages, unfortunately we didn't notice that the max duration for the test was wrongly calculated. This also tries to fix cases where graceful* will extend the test more then needed. Previously the code did not take into account that multiple 0 VUs stages can mean that the VUs are long ago stopped before gracefulStop (for example) needs to be taken into account. --- core/local/local_test.go | 31 ++++++++++++++++++++++++++++ lib/executor/ramping_vus.go | 35 ++++++++++++++++++++++++++++---- lib/executor/ramping_vus_test.go | 33 +++++++++++++++--------------- lib/executors.go | 3 ++- 4 files changed, 81 insertions(+), 21 deletions(-) diff --git a/core/local/local_test.go b/core/local/local_test.go index c6273100fed..18ad51a934d 100644 --- a/core/local/local_test.go +++ b/core/local/local_test.go @@ -881,6 +881,37 @@ func TestExecutionSchedulerEndTime(t *testing.T) { assert.True(t, runTime < 10*time.Second, "took more than 10 seconds") } +func TestExecutionSchedulerEndTime0VUsAtEnd(t *testing.T) { + t.Parallel() + runner := &minirunner.MiniRunner{ + Fn: func(ctx context.Context, _ *lib.State, out chan<- metrics.SampleContainer) error { + time.Sleep(100 * time.Millisecond) + return nil + }, + } + _, cancel, execScheduler, _ := newTestExecutionScheduler(t, runner, nil, lib.Options{ + Stages: []lib.Stage{ + { + Duration: types.NullDurationFrom(time.Second), + Target: null.IntFrom(1), + }, + { + Duration: types.NullDurationFrom(time.Second), + Target: null.IntFrom(0), + }, + { + Duration: types.NullDurationFrom(time.Hour), + Target: null.IntFrom(0), + }, + }, + }) + defer cancel() + + endTime, isFinal := lib.GetEndOffset(execScheduler.GetExecutionPlan()) + assert.Equal(t, time.Hour+2*time.Second, endTime) // because of the big 0 vu stage at end + assert.True(t, isFinal) +} + func TestExecutionSchedulerRuntimeErrors(t *testing.T) { t.Parallel() runner := &minirunner.MiniRunner{ diff --git a/lib/executor/ramping_vus.go b/lib/executor/ramping_vus.go index 10bcdff20ba..5819c03f4bd 100644 --- a/lib/executor/ramping_vus.go +++ b/lib/executor/ramping_vus.go @@ -202,13 +202,17 @@ func (vlvc RampingVUsConfig) getRawExecutionSteps(et *lib.ExecutionTuple, zeroEn } } - for _, stage := range vlvc.Stages { + for i, stage := range vlvc.Stages { stageEndVUs := stage.Target.Int64 stageDuration := stage.Duration.TimeDuration() timeTillEnd += stageDuration stageVUDiff := stageEndVUs - fromVUs if stageVUDiff == 0 { + if i == len(vlvc.Stages)-1 { + // add the last step always + steps = append(steps, lib.ExecutionStep{TimeOffset: timeTillEnd, PlannedVUs: uint64(scaled)}) + } continue } if stageDuration == 0 { @@ -244,7 +248,7 @@ func (vlvc RampingVUsConfig) getRawExecutionSteps(et *lib.ExecutionTuple, zeroEn } if zeroEnd { - steps = append(steps, lib.ExecutionStep{TimeOffset: timeTillEnd, PlannedVUs: 0}) + addStep(timeTillEnd, 0) } return steps } @@ -426,6 +430,14 @@ func (vlvc RampingVUsConfig) reserveVUsForGracefulRampDowns( //nolint:funlen }) lastPlannedVUs = rawStep.PlannedVUs } + /* + if newSteps[len(newSteps)-1].TimeOffset < rawSteps[len(rawSteps)-1].TimeOffset { + newSteps = append(newSteps, lib.ExecutionStep{ + TimeOffset: rawSteps[len(rawSteps)-1].TimeOffset, + PlannedVUs: rawSteps[len(rawSteps)-1].PlannedVUs, + }) + } + */ return newSteps } @@ -449,19 +461,34 @@ func (vlvc RampingVUsConfig) reserveVUsForGracefulRampDowns( //nolint:funlen // - If the last stage's target is more than 0, the VUs at the end of the // executor's life will have more time to finish their last iterations. func (vlvc RampingVUsConfig) GetExecutionRequirements(et *lib.ExecutionTuple) []lib.ExecutionStep { - steps := vlvc.getRawExecutionSteps(et, false) + origSteps := vlvc.getRawExecutionSteps(et, false) executorEndOffset := sumStagesDuration(vlvc.Stages) + vlvc.GracefulStop.TimeDuration() // Handle graceful ramp-downs, if we have them + steps := origSteps if vlvc.GracefulRampDown.Duration > 0 { steps = vlvc.reserveVUsForGracefulRampDowns(steps, executorEndOffset) } + lastIndex := len(steps) + for ; lastIndex >= 0; lastIndex-- { + if steps[lastIndex-1].PlannedVUs != 0 { + break + } + } + if len(steps) > lastIndex { + executorEndOffset = steps[lastIndex].TimeOffset + vlvc.GracefulStop.TimeDuration() + } // add one step for the end of the gracefulStop - if steps[len(steps)-1].PlannedVUs != 0 || steps[len(steps)-1].TimeOffset != executorEndOffset { + if steps[len(steps)-1].PlannedVUs != 0 || steps[len(steps)-1].TimeOffset < executorEndOffset { steps = append(steps, lib.ExecutionStep{TimeOffset: executorEndOffset, PlannedVUs: 0}) } + if steps[len(steps)-1].TimeOffset < origSteps[len(origSteps)-1].TimeOffset { + // This can only happen on multiple 0 vus stages at the end + steps = append(steps, lib.ExecutionStep{TimeOffset: origSteps[len(origSteps)-1].TimeOffset, PlannedVUs: 0}) + } + return steps } diff --git a/lib/executor/ramping_vus_test.go b/lib/executor/ramping_vus_test.go index b6d3dfc3687..e1a2ad33666 100644 --- a/lib/executor/ramping_vus_test.go +++ b/lib/executor/ramping_vus_test.go @@ -438,14 +438,14 @@ func TestRampingVUsConfigExecutionPlanExample(t *testing.T) { conf := NewRampingVUsConfig("test") conf.StartVUs = null.IntFrom(4) conf.Stages = []Stage{ - {Target: null.IntFrom(6), Duration: types.NullDurationFrom(2 * time.Second)}, - {Target: null.IntFrom(1), Duration: types.NullDurationFrom(5 * time.Second)}, - {Target: null.IntFrom(5), Duration: types.NullDurationFrom(4 * time.Second)}, - {Target: null.IntFrom(1), Duration: types.NullDurationFrom(4 * time.Second)}, - {Target: null.IntFrom(4), Duration: types.NullDurationFrom(3 * time.Second)}, - {Target: null.IntFrom(4), Duration: types.NullDurationFrom(2 * time.Second)}, - {Target: null.IntFrom(1), Duration: types.NullDurationFrom(0 * time.Second)}, - {Target: null.IntFrom(1), Duration: types.NullDurationFrom(3 * time.Second)}, + {Target: null.IntFrom(6), Duration: types.NullDurationFrom(2 * time.Second)}, // 2 + {Target: null.IntFrom(1), Duration: types.NullDurationFrom(5 * time.Second)}, // 7 + {Target: null.IntFrom(5), Duration: types.NullDurationFrom(4 * time.Second)}, // 11 + {Target: null.IntFrom(1), Duration: types.NullDurationFrom(4 * time.Second)}, // 15 + {Target: null.IntFrom(4), Duration: types.NullDurationFrom(3 * time.Second)}, // 18 + {Target: null.IntFrom(4), Duration: types.NullDurationFrom(2 * time.Second)}, // 20 + {Target: null.IntFrom(1), Duration: types.NullDurationFrom(0 * time.Second)}, // 20 + {Target: null.IntFrom(1), Duration: types.NullDurationFrom(3 * time.Second)}, // 23 } expRawStepsNoZeroEnd := []lib.ExecutionStep{ @@ -469,11 +469,12 @@ func TestRampingVUsConfigExecutionPlanExample(t *testing.T) { {TimeOffset: 17 * time.Second, PlannedVUs: 3}, {TimeOffset: 18 * time.Second, PlannedVUs: 4}, {TimeOffset: 20 * time.Second, PlannedVUs: 1}, + {TimeOffset: 23 * time.Second, PlannedVUs: 1}, } rawStepsNoZeroEnd := conf.getRawExecutionSteps(et, false) assert.Equal(t, expRawStepsNoZeroEnd, rawStepsNoZeroEnd) endOffset, isFinal := lib.GetEndOffset(rawStepsNoZeroEnd) - assert.Equal(t, 20*time.Second, endOffset) + assert.Equal(t, 23*time.Second, endOffset) assert.Equal(t, false, isFinal) rawStepsZeroEnd := conf.getRawExecutionSteps(et, true) @@ -559,13 +560,13 @@ func TestRampingVUsConfigExecutionPlanExampleOneThird(t *testing.T) { {TimeOffset: 15 * time.Second, PlannedVUs: 0}, {TimeOffset: 16 * time.Second, PlannedVUs: 1}, {TimeOffset: 20 * time.Second, PlannedVUs: 0}, + {TimeOffset: 23 * time.Second, PlannedVUs: 0}, } expRawStepsZeroEnd := expRawStepsNoZeroEnd // no need to copy - expRawStepsZeroEnd = append(expRawStepsZeroEnd, lib.ExecutionStep{TimeOffset: 23 * time.Second, PlannedVUs: 0}) rawStepsNoZeroEnd := conf.getRawExecutionSteps(et, false) assert.Equal(t, expRawStepsNoZeroEnd, rawStepsNoZeroEnd) endOffset, isFinal := lib.GetEndOffset(rawStepsNoZeroEnd) - assert.Equal(t, 20*time.Second, endOffset) + assert.Equal(t, 23*time.Second, endOffset) assert.Equal(t, true, isFinal) rawStepsZeroEnd := conf.getRawExecutionSteps(et, true) @@ -580,7 +581,7 @@ func TestRampingVUsConfigExecutionPlanExampleOneThird(t *testing.T) { {TimeOffset: 1 * time.Second, PlannedVUs: 2}, {TimeOffset: 42 * time.Second, PlannedVUs: 1}, {TimeOffset: 50 * time.Second, PlannedVUs: 0}, - {TimeOffset: 53 * time.Second, PlannedVUs: 0}, + {TimeOffset: 80 * time.Second, PlannedVUs: 0}, }, conf.GetExecutionRequirements(et)) // Try a longer GracefulStop than the GracefulRampDown @@ -590,7 +591,7 @@ func TestRampingVUsConfigExecutionPlanExampleOneThird(t *testing.T) { {TimeOffset: 1 * time.Second, PlannedVUs: 2}, {TimeOffset: 42 * time.Second, PlannedVUs: 1}, {TimeOffset: 50 * time.Second, PlannedVUs: 0}, - {TimeOffset: 103 * time.Second, PlannedVUs: 0}, + {TimeOffset: 130 * time.Second, PlannedVUs: 0}, }, conf.GetExecutionRequirements(et)) // Try a much shorter GracefulStop than the GracefulRampDown @@ -611,7 +612,7 @@ func TestRampingVUsConfigExecutionPlanExampleOneThird(t *testing.T) { // Try a zero GracefulStop and GracefulRampDown, i.e. raw steps with 0 end cap conf.GracefulRampDown = types.NullDurationFrom(0 * time.Second) - assert.Equal(t, rawStepsZeroEnd, conf.GetExecutionRequirements(et)) + assert.Equal(t, expRawStepsZeroEnd, conf.GetExecutionRequirements(et)) } func TestRampingVUsConfigExecutionPlanZerosAtEnd(t *testing.T) { @@ -637,13 +638,13 @@ func TestRampingVUsConfigExecutionPlanZerosAtEnd(t *testing.T) { {TimeOffset: 7 * time.Second, PlannedVUs: 2}, {TimeOffset: 8 * time.Second, PlannedVUs: 1}, {TimeOffset: 9 * time.Second, PlannedVUs: 0}, + {TimeOffset: 14 * time.Second, PlannedVUs: 0}, } expRawStepsZeroEnd := expRawStepsNoZeroEnd // no need to copy - expRawStepsZeroEnd = append(expRawStepsZeroEnd, lib.ExecutionStep{TimeOffset: 14 * time.Second, PlannedVUs: 0}) rawStepsNoZeroEnd := conf.getRawExecutionSteps(et, false) assert.Equal(t, expRawStepsNoZeroEnd, rawStepsNoZeroEnd) endOffset, isFinal := lib.GetEndOffset(rawStepsNoZeroEnd) - assert.Equal(t, 9*time.Second, endOffset) + assert.Equal(t, 14*time.Second, endOffset) assert.Equal(t, true, isFinal) rawStepsZeroEnd := conf.getRawExecutionSteps(et, true) diff --git a/lib/executors.go b/lib/executors.go index f0d39afa4f7..15e17cb2b39 100644 --- a/lib/executors.go +++ b/lib/executors.go @@ -303,7 +303,8 @@ func (scs ScenarioConfigs) GetFullExecutionRequirements(et *ExecutionTuple) []Ex stepsLen := len(consolidatedSteps) if stepsLen == 0 || consolidatedSteps[stepsLen-1].PlannedVUs != newPlannedVUs || - consolidatedSteps[stepsLen-1].MaxUnplannedVUs != newMaxUnplannedVUs { + consolidatedSteps[stepsLen-1].MaxUnplannedVUs != newMaxUnplannedVUs || + consolidatedSteps[stepsLen-1].TimeOffset != currentTimeOffset { consolidatedSteps = append(consolidatedSteps, ExecutionStep{ TimeOffset: currentTimeOffset, PlannedVUs: newPlannedVUs, From 7d27047f5caeb2a6f606631d0fc3d9ef4e3fe1ab Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 15 Jun 2022 13:43:24 +0300 Subject: [PATCH 2/4] Don't extend the duration if gracefulStop will not do anything --- lib/executor/ramping_vus.go | 6 +++--- lib/executor/ramping_vus_test.go | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/executor/ramping_vus.go b/lib/executor/ramping_vus.go index 5819c03f4bd..6d28f1f2c71 100644 --- a/lib/executor/ramping_vus.go +++ b/lib/executor/ramping_vus.go @@ -470,14 +470,14 @@ func (vlvc RampingVUsConfig) GetExecutionRequirements(et *lib.ExecutionTuple) [] steps = vlvc.reserveVUsForGracefulRampDowns(steps, executorEndOffset) } lastIndex := len(steps) - for ; lastIndex >= 0; lastIndex-- { + for ; lastIndex > 0; lastIndex-- { if steps[lastIndex-1].PlannedVUs != 0 { break } } - if len(steps) > lastIndex { - executorEndOffset = steps[lastIndex].TimeOffset + vlvc.GracefulStop.TimeDuration() + if len(steps) > lastIndex && steps[lastIndex].TimeOffset < executorEndOffset { + executorEndOffset = steps[lastIndex].TimeOffset } // add one step for the end of the gracefulStop if steps[len(steps)-1].PlannedVUs != 0 || steps[len(steps)-1].TimeOffset < executorEndOffset { diff --git a/lib/executor/ramping_vus_test.go b/lib/executor/ramping_vus_test.go index e1a2ad33666..bd002458d8a 100644 --- a/lib/executor/ramping_vus_test.go +++ b/lib/executor/ramping_vus_test.go @@ -581,7 +581,6 @@ func TestRampingVUsConfigExecutionPlanExampleOneThird(t *testing.T) { {TimeOffset: 1 * time.Second, PlannedVUs: 2}, {TimeOffset: 42 * time.Second, PlannedVUs: 1}, {TimeOffset: 50 * time.Second, PlannedVUs: 0}, - {TimeOffset: 80 * time.Second, PlannedVUs: 0}, }, conf.GetExecutionRequirements(et)) // Try a longer GracefulStop than the GracefulRampDown @@ -591,7 +590,6 @@ func TestRampingVUsConfigExecutionPlanExampleOneThird(t *testing.T) { {TimeOffset: 1 * time.Second, PlannedVUs: 2}, {TimeOffset: 42 * time.Second, PlannedVUs: 1}, {TimeOffset: 50 * time.Second, PlannedVUs: 0}, - {TimeOffset: 130 * time.Second, PlannedVUs: 0}, }, conf.GetExecutionRequirements(et)) // Try a much shorter GracefulStop than the GracefulRampDown From f0cc28fa609efb26af99882b10078f79bd1c50af Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 15 Jun 2022 15:00:20 +0300 Subject: [PATCH 3/4] Remove commented code --- lib/executor/ramping_vus.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/executor/ramping_vus.go b/lib/executor/ramping_vus.go index 6d28f1f2c71..51697458a29 100644 --- a/lib/executor/ramping_vus.go +++ b/lib/executor/ramping_vus.go @@ -430,14 +430,6 @@ func (vlvc RampingVUsConfig) reserveVUsForGracefulRampDowns( //nolint:funlen }) lastPlannedVUs = rawStep.PlannedVUs } - /* - if newSteps[len(newSteps)-1].TimeOffset < rawSteps[len(rawSteps)-1].TimeOffset { - newSteps = append(newSteps, lib.ExecutionStep{ - TimeOffset: rawSteps[len(rawSteps)-1].TimeOffset, - PlannedVUs: rawSteps[len(rawSteps)-1].PlannedVUs, - }) - } - */ return newSteps } From dbb6f50740a7532da7dac087236a122fde8e5511 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Wed, 15 Jun 2022 15:05:11 +0300 Subject: [PATCH 4/4] Add more comments --- lib/executor/ramping_vus.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/executor/ramping_vus.go b/lib/executor/ramping_vus.go index 51697458a29..a23545da6a8 100644 --- a/lib/executor/ramping_vus.go +++ b/lib/executor/ramping_vus.go @@ -209,8 +209,10 @@ func (vlvc RampingVUsConfig) getRawExecutionSteps(et *lib.ExecutionTuple, zeroEn stageVUDiff := stageEndVUs - fromVUs if stageVUDiff == 0 { + // skip those as this doesn't change anything if i == len(vlvc.Stages)-1 { - // add the last step always + // don't skip the last step as we won't add another one after it + // unlike in any of the other cases steps = append(steps, lib.ExecutionStep{TimeOffset: timeTillEnd, PlannedVUs: uint64(scaled)}) } continue