Repository navigation
tools: use ubuntu-latest for public lint-js-and-md jobs - #66409
Conversation
|
Review requested:
|
3e63215 to
4548c8d
Compare
|
As I said before, I don't think this is the right approach, IMO a solution à la #66228, where we reduce the load instead of paying for a more powerful runner, would be much preferable. I suggested in #64972 (comment) to add cache, that would be a preferable outcome |
|
Makes sense. I’ll close this. Thanks! |
4548c8d to
41f27fc
Compare
|
As discussed here, timeouts still occur despite caching. I've reopened this PR with updated benchmarks, including a two-CPU ARM approximation to help assess the private-repository cost trade-off. |
|
If costs in the private repo used for security releases are still a concern, then I'd suggest checking if the workflow can check if it's running in a public / private repository and then choose the runner accordingly. It would however be simpler to use one runner in both instances. According to the assessment in the original post, the incremental cost in the private repo would be around $5 / 1000 runs. I can't judge whether that would be significant or not. |
|
@aduh95 @trivikr timeouts are still occurring after caching: 21 of 261 Would you be comfortable moving forward with ARM? As Mike mentioned, we could also select the runner conditionally: ARM for public repositories and |
41f27fc to
d61d0d0
Compare
|
On reflection, the potential cost increase for private repositories is still a concern, so I've updated this to use ARM only for public repositories. Private repositories will continue using |
| lint-js-and-md: | ||
| if: github.event.pull_request.draft == false | ||
| runs-on: ubuntu-slim | ||
| runs-on: ${{ github.event.repository.private && 'ubuntu-slim' || 'ubuntu-26.04-arm' }} |
There was a problem hiding this comment.
We might as well use ubuntu-latest so that's one fewer thing to keep up-to-date
| runs-on: ${{ github.event.repository.private && 'ubuntu-slim' || 'ubuntu-26.04-arm' }} | |
| runs-on: ${{ github.event.repository.private && 'ubuntu-slim' || 'ubuntu-latest' }} |
There was a problem hiding this comment.
That makes sense. I'm running the same benchmark with ubuntu-latest and will share the results once it finishes.
There was a problem hiding this comment.
Updated to ubuntu-latest. I ran the same benchmark: all 128 warm-cache and 128 cold-cache runs passed, averaging 1m45s and 2m56s respectively. Results.
Use ubuntu-latest in public repositories to reduce runtime and provide more headroom when the ESLint cache cannot be restored. Keep ubuntu-slim in private repositories to avoid increasing runner costs. Assisted-by: Codex Signed-off-by: inoway46 <inoueyuya416@gmail.com>
d61d0d0 to
d20ffc8
Compare
|
You should be able to claim "Fixes: #64972" in the original PR text. |
|
Landed in ae353ec |
Use ubuntu-latest for CodeQL, documentation, both tarball jobs, lint-cpp and format-cpp in public repositories after observing timeouts on ubuntu-slim. Keep around 30 other jobs on ubuntu-slim, and retain ubuntu-slim in private repositories to avoid increasing runner costs. Refs: nodejs#66409 Refs: nodejs#61663 Refs: nodejs#64972 Assisted-by: Codex Signed-off-by: inoway46 <inoueyuya416@gmail.com>
According to the title, the scope of the issue remained the same as "tools: use ubuntu-latest for public lint-js-and-md jobs". Even if other issues were identified in this issue, since the title remained, the issue needed to be closed for clarity. |
Use ubuntu-latest in public repositories to reduce runtime and provide more headroom when the ESLint cache cannot be restored. Keep ubuntu-slim in private repositories to avoid increasing runner costs. Assisted-by: Codex Signed-off-by: inoway46 <inoueyuya416@gmail.com> PR-URL: #66409 Refs: #64972 Reviewed-By: Mike McCready <66998419+MikeMcC399@users.noreply.github.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Move
lint-js-and-mdtoubuntu-latestin public repositories for faster execution and more timeout headroom. Private repositories will continue usingubuntu-slimto avoid increasing runner costs.Warm cache
ubuntu-slimubuntu-latestCold cache
ubuntu-slimubuntu-latestThe benchmarks used the same source commit and Node.js version, in separate runs.
Benchmarks: slim, ubuntu-latest.
Refs: nodejs/node#64972