Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add v0.27.0 release notes #1507

Merged
merged 18 commits into from
Jul 14, 2020
Merged

Add v0.27.0 release notes #1507

merged 18 commits into from
Jul 14, 2020

Conversation

imiric
Copy link
Contributor

@imiric imiric commented Jun 15, 2020

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2020

Codecov Report

Merging #1507 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1507      +/-   ##
==========================================
+ Coverage   77.18%   77.21%   +0.03%     
==========================================
  Files         162      162              
  Lines       13172    13172              
==========================================
+ Hits        10167    10171       +4     
+ Misses       2485     2481       -4     
  Partials      520      520              
Impacted Files Coverage Δ
stats/cloud/collector.go 79.04% <0.00%> (+0.59%) ⬆️
js/runner.go 83.73% <0.00%> (+0.69%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc31776...357b718. Read the comment docs.

Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some more issues should be linked and I have small nitpicks on the wording here and there

release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved

- Tests with infinite duration are now only possible via the `externally-controlled` executor.

- Previously, all iterations were interruptible - as soon as the specified `duration` expired, or when VUs were ramped down in a stage, any running iterations were interrupted. Now all executors besides the `externally-controlled` one have a `gracefulStop` period of `30s` by default (closes #898). Additionally, the `ramping-vus` executor has a `gracefulRampDown` parameter that configures the ramp-down grace period. For those periods, no new iterations will be started by the executors, but any currently running iterations will be allowed up to the specified periods to finish their execution.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably ... a bug ? (or to keep it the same as before 1007), but the externally-controlled executor does call gracefulStop if it's pausing vus ...
cc @na--

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fallback to @na-- on how to reword this, as I just copied it from 1007.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm it's a bit tricky - the externally-controlled executor doesn't have gracefulStop as an option... but it's true that when you pause it, it calls the vuHandle.gracefulStop() method. This was done to preserve the logic of how "pausing" worked in the old k6, which didn't interrupt any running iterations, it just stopped k6 from making new iterations, i.e. what gracefulStop does: https://github.com/loadimpact/k6/blob/459da79ef51b37e5eaba4575c9065d9e592e5c49/core/local/local.go#L234-L250

so, I think the sentence here is technically correct... also, I think this whole paragraph should be more prominent, i.e. moved somewhere on top of the breaking changes, in second place perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this higher up, but maybe we should explain why the externally-controlled executor doesn't have the option? We mention it here and it the new docs, but it's never explained.

Since it's technically still using vuHandle.gracefulStop() it would make sense to be able to define this value in the config too. Avoiding special cases is always good if possible. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling vuHandle.gracefulStop() is very much not the same thing as the gracefulStop option. It just means "don't start more iterations with this VU", whereas the option the gracefulStop option implies "don't start new iterations and call hardStop after this period, if an iteration was still running".

And yes, if there's a sufficiently high demand for it, we could probably implement support for the gracefulStop option in the externally-controlled executor as well. As to why we haven't, it's because of a couple of reasons:

  • the externally controlled executor is pretty much the least important executor of them all, for now, it exists mostly because of keeping some backwards compatibility and because it might be useful to a small number of people
  • users of the externally-controlled executor can achieve exactly the same result pretty easily with just the k6 REST API, if they want to - instead of directly setting the running VUs to 0, you can first run k6 pause, wait 30 seconds (or however long you want to give VUs to finish gracefully), and then set the active VUs to 0 - hey, presto, you have gracefulStop 😄

Though you are right, that second fact is probably worth mentioning in the docs, though probably not in the release notes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, they can't have something like going down from 100 to 50 VUs gracefully ... but to be honest I don't like the externally-controlled executor and unless there is demand I am for not touching it :D

@na--
Copy link
Member

na-- commented Jun 18, 2020

Still haven't read this, just commenting on something that seems to be missing after remembering it and not finding it with Ctrl+F here 😅 We should mention that we've changed the default setupTimeout and teardownTimeout values to 60s (were 10s before) - #1356. This is also something we should update in https://k6.io/docs/using-k6/options#setup-timeout

@imiric
Copy link
Contributor Author

imiric commented Jun 18, 2020

@na-- That's a breaking change, right? I added it in 4df3093.

@mstoykov mstoykov added this to the v0.27.0 milestone Jun 30, 2020
@imiric imiric changed the base branch from new-schedulers to master July 7, 2020 08:34
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved

- Tests with infinite duration are now only possible via the `externally-controlled` executor.

- Previously, all iterations were interruptible - as soon as the specified `duration` expired, or when VUs were ramped down in a stage, any running iterations were interrupted. Now all executors besides the `externally-controlled` one have a `gracefulStop` period of `30s` by default (closes #898). Additionally, the `ramping-vus` executor has a `gracefulRampDown` parameter that configures the ramp-down grace period. For those periods, no new iterations will be started by the executors, but any currently running iterations will be allowed up to the specified periods to finish their execution.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm it's a bit tricky - the externally-controlled executor doesn't have gracefulStop as an option... but it's true that when you pause it, it calls the vuHandle.gracefulStop() method. This was done to preserve the logic of how "pausing" worked in the old k6, which didn't interrupt any running iterations, it just stopped k6 from making new iterations, i.e. what gracefulStop does: https://github.com/loadimpact/k6/blob/459da79ef51b37e5eaba4575c9065d9e592e5c49/core/local/local.go#L234-L250

so, I think the sentence here is technically correct... also, I think this whole paragraph should be more prominent, i.e. moved somewhere on top of the breaking changes, in second place perhaps?

@imiric imiric requested review from na-- and mstoykov July 13, 2020 10:09
@imiric imiric changed the title WIP Add v0.27.0 release notes Add v0.27.0 release notes Jul 13, 2020
@imiric imiric marked this pull request as ready for review July 13, 2020 10:10
mstoykov
mstoykov previously approved these changes Jul 13, 2020
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I am pretty sure we've missed something, so I will go through all the issues/PRs marked as 0.27.0 and try to see if we've missed something, it's probably good idea for all of us to do it, though :D

release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Show resolved Hide resolved
release notes/v0.27.0.md Show resolved Hide resolved
Ivan Mirić and others added 2 commits July 13, 2020 16:19
Co-authored-by: na-- <n@andreev.sh>
Co-authored-by: na-- <n@andreev.sh>
na--
na-- previously approved these changes Jul 14, 2020
Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM besides the last few minor corrections I suggested inline. Also, please squash this when merging it in master 😄

release notes/v0.27.0.md Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
release notes/v0.27.0.md Outdated Show resolved Hide resolved
Co-authored-by: na-- <n@andreev.sh>
@imiric imiric merged commit b26d97a into master Jul 14, 2020
@imiric imiric deleted the release/v0.27.0-notes branch July 14, 2020 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants