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

Unify duration configurations #1305

Closed
na-- opened this issue Jan 9, 2020 · 3 comments · Fixed by #1738
Closed

Unify duration configurations #1305

na-- opened this issue Jan 9, 2020 · 3 comments · Fixed by #1738
Assignees
Labels
Milestone

Comments

@na--
Copy link
Member

na-- commented Jan 9, 2020

This seemingly normal script won't work as most people expect:

import http from "k6/http";

export let options = {
    duration: "5s",
    thresholds: {
        "iterations": ["count == 2"],
        "http_reqs": ["count == 2"],
    },
};

export default function () {
    let response = http.get(
        "https://httpbin.test.loadimpact.com/delay/3",
        { timeout: "2s" },
    );
}

Because the request timeout doesn't understand stringy durations, "2s" would be converted to 2 (because JavaScript 🤦‍♂️ ), so k6 would understand it as "2 milliseconds". So it's going to start to make a ton of requests instead of just 3...

However this script also doesn't work like how you'd expect either:

import http from "k6/http";

export let options = {
    duration: 5000,
    thresholds: {
        "iterations": ["count == 2"],
        "http_reqs": ["count == 2"],
    },
};

export default function () {
    let response = http.get(
        "https://httpbin.test.loadimpact.com/delay/3",
        { timeout: 2000 },
    );
}

k6 runs, doesn't abort with a configuration error or something like that... but it interprets the 5000 value for duration as 5000 nanoseconds, i.e. 0.005 milliseconds, so it just immediately aborts... 🤦‍♂️ This is just a consequence of the way we've wrapped Go's time.Duration (which is nanosecond-based) in our types.Duration custom type (that parses things like "2s"). Only, when it doesn't get a string, it falls back to the nanosecond code...

So, I think we should unify and make everything saner... To do that with minimal breaking changes, I propose that we:

  • use the same duration type (types.Duration / types.NullDuration?) everywhere we accept user-specified duration
  • modify it so it either accepts valid string durations, or integer durations in milliseconds, everything else should be an error

So, the only breaking change is that users won't be able to specify the global options in nanoseconds, which won't be a big loss and I hope nobody has done...

@na-- na-- added the ux label Jan 9, 2020
@na--
Copy link
Member Author

na-- commented Jan 20, 2020

The thresholds is another place in k6 where we have configuration of time duration values. Thankfully, that's also in milliseconds, but there's currently no way to use string-y units like 1.5s... This is a lower-priority item to fix, since we have to synchronize with the cloud execution as well, but it doesn't make sense not to support it eventually. Of course, after we make the threshold parsing sane (#961).

@na--
Copy link
Member Author

na-- commented Aug 28, 2020

ws.SetTimeout() and ws.SetInterval() are also places where we accept only millisecond-based duration values. And we recently found a problem with them accepting ints - if a user passes a float value, goja transforms it to 0 (#1608)... Even if that gets fixed, it's probably better for us, when we unify everything, to change all places to accept either a stringy duration (eg. 30s, 1.5m, 2h30m1s, etc.) , or a float millisecond value.

@na--
Copy link
Member Author

na-- commented Sep 3, 2020

This overhaul is also necessary for timeUnit and duration and any other such properties in the new scenarios configuration, since they also accept integer nanosecond values 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant