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

🌱 [UNDER TESTING] Increase parallelism #1345

Closed
wants to merge 1 commit into from
Closed

Conversation

azeemshaikh38
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • What is the current behavior? (You can also link to an open issue here)

  • What is the new behavior (if this is a feature change)?

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

  • Other information:

@@ -38,7 +38,7 @@ jobs:
- name: Run presubmit tests
run: |
go env -w GOFLAGS=-mod=mod
make -j 5 all
make -j 12 all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you tried using "make -jnproc" to dynamically retrieve the number of processors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. nproc worked great!

@laurentsimon
Copy link
Contributor

error release failed after 1800.01s error=context deadline exceeded. The Validate does many checks: shall we split it into smaller ones? The linter does not need to build the code, for example...

@azeemshaikh38 azeemshaikh38 temporarily deployed to integration-test December 3, 2021 16:30 Inactive
@github-actions
Copy link

github-actions bot commented Dec 3, 2021

Integration tests success for
[b8d0a30]
(https://github.com/ossf/scorecard/actions/runs/1536080110)

@@ -38,7 +38,7 @@ jobs:
- name: Run presubmit tests
run: |
go env -w GOFLAGS=-mod=mod
make -j 5 all
make -j $(nproc) all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of make -j all, split them into individual jobs with the actions and they run parallel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, we need to maintain order in our build process. For example, update-dependencies step should always run before any other step while tree-status should be the last step. Similarly, some steps which generate files like proto and mocks should run before we build. Explicitly specifying this ordering in the Makefile is the best thing I could come up with. It's not ideal, but if folks have better ideas, please do let me know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split them into individual jobs with the actions and they run parallel.

+1, example from Kubernetes RelEng: https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-sigs/release-utils/release-utils-presubmits.yaml

There we usually prefer to run separate Prow jobs to split them up along the following boundaries:

  • unit/integration tests
  • e2e tests
  • verifies (linting, go mod tidy diffs, boilerplate)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like boundaries too :-)

@github-actions
Copy link

Stale pull request message

@github-actions
Copy link

Stale pull request message

@github-actions
Copy link

github-actions bot commented Feb 6, 2022

Stale pull request message

@naveensrinivasan
Copy link
Member

@azeemshaikh38 Can we close this? Now that we have some parallelism.

@azeemshaikh38
Copy link
Contributor Author

@azeemshaikh38 Can we close this? Now that we have some parallelism.

Done, thanks!

@azeemshaikh38 azeemshaikh38 deleted the azeems/parallel branch August 29, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants