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

Propagate the real context when initializing VUs #2790

Closed
na-- opened this issue Dec 1, 2022 · 0 comments · Fixed by #2800
Closed

Propagate the real context when initializing VUs #2790

na-- opened this issue Dec 1, 2022 · 0 comments · Fixed by #2800

Comments

@na--
Copy link
Member

na-- commented Dec 1, 2022

Currently, VU initialization doesn't receive a context. The last time we propagate the context is here:

k6/core/local/local.go

Lines 172 to 173 in 564197a

func (e *ExecutionScheduler) initVUsConcurrently(
ctx context.Context, samplesOut chan<- metrics.SampleContainer, count uint64,

But then we don't pass it to the layers below:

newVU, err := e.initVU(samplesOut, logger)

vu, err := e.state.Test.Runner.NewVU(vuIDLocal, vuIDGlobal, samplesOut)

k6/js/runner.go

Lines 115 to 126 in 564197a

func (r *Runner) NewVU(idLocal, idGlobal uint64, samplesOut chan<- metrics.SampleContainer) (lib.InitializedVU, error) {
vu, err := r.newVU(idLocal, idGlobal, samplesOut)
if err != nil {
return nil, err
}
return lib.InitializedVU(vu), nil
}
//nolint:funlen
func (r *Runner) newVU(idLocal, idGlobal uint64, samplesOut chan<- metrics.SampleContainer) (*VU, error) {
// Instantiate a new bundle, make a VU out of it.
bi, err := r.Bundle.Instantiate(r.preInitState.Logger, idLocal)

k6/js/bundle.go

Lines 242 to 247 in 564197a

func (b *Bundle) Instantiate(logger logrus.FieldLogger, vuID uint64) (*BundleInstance, error) {
// Instantiate the bundle into a new VM using a bound init context. This uses a context with a
// runtime, but no state, to allow module-provided types to function within the init context.
vuImpl := &moduleVUImpl{runtime: goja.New()}
init := newBoundInitContext(b.BaseInitContext, vuImpl)
if err := b.instantiate(logger, vuImpl.runtime, init, vuID); err != nil {

And, at the end, we end up using context.Background() 😞

k6/js/bundle.go

Line 329 in 564197a

init.moduleVUImpl.ctx = context.Background()

This is not ideal because we don't have any way of aborting the VU initialization if the user hits Ctrl+C or if one of the VUs fails during the concurrent VU initialization and we need to abort the initialization of the rest of the VUs and wait for that to finish quickly (see here and here).

Currently, we just rely on the fact that we'll use os.Exit() to stop k6, however that assumption breaks in a few cases:

In general, the current behavior breaks the very reasonable expectation that once the k6 run command has finished and returned, all of its child goroutines have also finished and returned before it...

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...
@na-- na-- added this to the v0.43.0 milestone Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant