tag1consulting / goose

Load testing framework, inspired by Locust
https://tag1.com/goose
Apache License 2.0
809 stars 70 forks source link

Make GooseAttack.execute async and remove Tokio runtime initialization #356

Closed finnefloyd closed 3 years ago

finnefloyd commented 3 years ago

In complex use case, a user may need to run some code alongside the GooseAttack which is currently complicated be Goose create a tokio Runtime which prevent user to use #[tokio::main] and run other futures.

This change remove the runtime initialization from Goose and make the function execute async.

jeremyandrews commented 3 years ago

It seems the tests aren’t working.

finnefloyd commented 3 years ago

I ran them locally and they passed.

it look's like there is an issues with the runner 2021-09-01T09:21:49.0070419Z Found online and idle hosted runner in the current repository's organization account that matches the required labels: 'ubuntu-latest' 2021-09-01T09:21:49.0764957Z Waiting for a Hosted runner in the 'organization' to pick this job... 2021-09-01T09:21:49.3159568Z Job is waiting for a hosted runner to come online. 2021-09-01T09:21:53.4901978Z Job is about to start running on the hosted runner: GitHub Actions 4 (hosted)

Are you able to restart it? I'm not.

jeremyandrews commented 3 years ago

I’ve restarted it

jeremyandrews commented 3 years ago

I re-ran the tests and they failed again - perhaps we need additional configuration in order for the tests to run properly in GitHub actions.

I tried running the tests locally, and ran into a different problem with tests/logs.rs where 11 tests failed all with the following error (on Mac OS X):

thread '<unnamed>' panicked at 'Cannot build local tokio runtime: Os { code: 24, kind: Other, message: "Too many open files" }', /Users/jeremyandrews/.cargo/registry/src/github.com-1ecc6299db9ec823/httpmock-0.6.2/src/api/server.rs:285:14
thread 'test_tasks_logs_raw_gaggle' panicked at 'Cannot get server address: RecvError(())', /Users/jeremyandrews/.cargo/registry/src/github.com-1ecc6299db9ec823/httpmock-0.6.2/src/api/server.rs:290:37
jeremyandrews commented 3 years ago

If locally I remove flavor = "multi_thread" I see the same behavior as GitHub actions: I wonder if it's not respecting this setting, or if it has different defaults.

finnefloyd commented 3 years ago

I'm working on Ubuntu and the tests are passing which is weird because the CI runner use Ubuntu as well. I'll try to find time to look into the CI issues.

Unfortunately, I don't own a Macbook so I'll not be able to investigate your issues.

finnefloyd commented 3 years ago

I finally manage to fix the CI.

The issues was that Tokio must create a run with the number of threads equal to the number of core of the machine it's running on which in our case must be one. But it's not enough because nng is not async in a Rust point of view so to block the thread used in the tokio runtime. The solution is to set a minimum of 8 threads for all the gaggle tests.

finnefloyd commented 3 years ago

Thanks for the review. I addressed all your comments.

jeremyandrews commented 3 years ago

There are still quite a few calls to .execute()? instead of .execute().await? in the README which should be updated to avoid confusing people using the examples.