-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: switch the load tester to pypy-3.10 #426
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had one question
Makefile
Outdated
@@ -29,7 +29,7 @@ lint: | |||
|
|||
load: | |||
SERVER_URL=$(STAGE_SERVER_URL) ENDPOINT_URL=$(STAGE_ENDPOINT_URL) \ | |||
docker-compose \ | |||
docker compose \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? Removing the -
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, from what I was able to find:
The docker compose (with a space) is a newer project to migrate compose to Go with the rest of the docker project. This is the v2 branch of the docker/compose repo. It's been first introduced to Docker Desktop users, so docker users on Linux didn't see the command. In addition to migrating to Go, it uses the compose-spec, and part of the rewrite may result in behavior differences.
The original python project, called docker-compose, aka v1 of docker/compose repo, has now been deprecated and development has moved over to v2. To install the v2 docker compose as a CLI plugin on Linux, supported distribution can now install the docker-compose-plugin package. E.g. on debian, I run apt-get install docker-compose-plugin.
https://stackoverflow.com/questions/66514436/difference-between-docker-compose-and-docker-compose
https://docs.docker.com/compose/#compose-v2-and-the-new-docker-compose-command
(TIL)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. I thought it had been standard for a while now (maybe not). We can keep the old dash version if you all would prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wow, well yeah, lets not add new stuff with old code. (TIL as well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made it a variable so it can be overided if needed, e.g. make load DOCKER_COMPOSE=docker-compose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there situations where we may need to run the older v1 python version instead of the recommended v2 version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Docker compose is only used for local execution -- locust is invoked differently when ran on GCP via a k8s cluster, so I don't think so? but I'm not sure how we will invoke it in CI.
5140081
to
e62a1f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pjenvey did you run these changes for a distributed GCP test? 10k on local?
Just locally |
e62a1f9
to
79a2488
Compare
SYNC-3874
Here's a draft of using pypy. This seems to work locally for me via
make load
, running a 10k user load test w/ 10 per second ramp up against stage (however I was only seeing =~ 2k Autoconnect total connections established on grafana for some reason).