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

Remove the Engine and split metrics/VUs handling #1889

Closed
na-- opened this issue Mar 8, 2021 · 2 comments · Fixed by #2815
Closed

Remove the Engine and split metrics/VUs handling #1889

na-- opened this issue Mar 8, 2021 · 2 comments · Fixed by #2815
Assignees
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 maintenance refactor
Milestone

Comments

@na--
Copy link
Member

na-- commented Mar 8, 2021

The current Engine vaguely resembles a Rube Goldberg machine... It has slowly been getting better, for example #1869 improved it slightly by mostly decoupling the outputs from the Engine, but it still has quite a lot of moving parts that can be streamlined and improved. And as #1887, it still has some gotchas and potentially some other hidden bugs or data races....

I'm opening this issue as an epic where we can gather Engine issues and ideas for fixing them. Here are some specific problems that currently exist, though there are quite a few other TODOs sprinkled in the code already:

Needless to say, but this refactoring should definitely be done after #1832, and maybe even some of its related issues, given that they will probably substantially change the requirements.

@na-- na-- added refactor evaluation needed proposal needs to be validated or tested before fully implementing it in k6 maintenance labels Mar 8, 2021
@na--
Copy link
Member Author

na-- commented Mar 6, 2022

I've been thinking on some related issues to this problem recently and I think I was wrong ⬆️, especially with this:

stopping and starting outputs doesn't really need to be in the Engine anymore, so it probably shouldn't be

I think it might actually be the other way around 😅 Instead of refactoring metrics handling or outputs out of the Engine, we should probably refactor the test execution out of it 💡 It is actually the thing that doesn't really fit all that well. Most of it is handled by the ExecutionScheduler anyway, the Engine just gets in the way.

So, my current idea is that we should actually focus the Engine completely on metrics, outputs, thresholds data crunching, etc. We can even rename it to something more appropriate like a MetricsEngine or something like that and move it out of core/ and into metrics/. Maybe even split it apart and tackle the built-in metrics and the outputs separately... 🤔

And we can focus the ExecutionScheduler on the actual test execution and likely also get it out of core/ and into some better place, e.g. /execution (related to #1302).

A big potential benefit of this decoupling would be the fact that we might be able to much more easily support test suites (i.e. multiple test scripts in a single k6 run, see #1342) with it. A single central MetricsEngine and multiple independent ExecutionSchedulers, one for every sub-test 💡

@na--
Copy link
Member Author

na-- commented Apr 26, 2022

A lot of these changes were done in #2426 and already merged. A rough version of the remainder are implemented in #2438, so they need to be split into a separate PR next cycle, after rebasing and refactoring the tests to use the new APIs.

@na-- na-- modified the milestones: v0.39.0, v0.40.0 Jun 20, 2022
@na-- na-- modified the milestones: v0.40.0, v0.41.0 Aug 19, 2022
@na-- na-- self-assigned this Sep 13, 2022
@na-- na-- modified the milestones: v0.41.0, TBD Oct 28, 2022
@na-- na-- changed the title Refactor the engine metrics handling Remove the Engine and split metrics/VUs handling Nov 2, 2022
na-- added a commit that referenced this issue Dec 1, 2022
…it phase

These will only be properly fixed when #2790 and #1889 are implemented, so the locking is needed for now...
na-- added a commit that referenced this issue Dec 1, 2022
…it phase

These will only be properly fixed when #2790 and #1889 are implemented, so the locking is needed for now...
na-- added a commit that referenced this issue Dec 1, 2022
…it phase

These will only be properly fixed when #2790 and #1889 are implemented, so the locking is needed for now...
na-- added a commit that referenced this issue Dec 1, 2022
…it phase

These will only be properly fixed when #2790 and #1889 are implemented, so the locking is needed for now...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 maintenance refactor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant