Skip to content

chore(ci): harden jemalloc cache resilience - #4043

Open
brbrr wants to merge 1 commit into
mainfrom
chore/harden-jemalloc-cache
Open

chore(ci): harden jemalloc cache resilience#4043
brbrr wants to merge 1 commit into
mainfrom
chore/harden-jemalloc-cache

Conversation

@brbrr

@brbrr brbrr commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

User description

This PR should improve cache resilience for jemalloc by adding jemalloc libdir into cache key


PR Type

Enhancement


Description

  • Add jemalloc libdir to CI cache keys

  • Move Jemalloc installs before Go setup

  • Remove later duplicate dependency installs


File Walkthrough

Relevant files
Configuration changes
build-binaries.yml
Move Jemalloc setup and cache dependencies                             

.github/workflows/build-binaries.yml

  • Move Linux and macOS dependency installs before Go setup
  • Record jemalloc libdir in .jemalloc-libdir
  • Include .jemalloc-libdir in setup-go cache-dependency-path
  • Remove old later dependency installation steps
+16/-10 
juno-test.yml
Add Jemalloc cache dependency to tests                                     

.github/workflows/juno-test.yml

  • Move Jemalloc and macOS dependency installs before Go setup
  • Record jemalloc libdir in .jemalloc-libdir
  • Include .jemalloc-libdir in setup-go cache-dependency-path
  • Remove old later Jemalloc installation steps
+14/-8   

@brbrr brbrr self-assigned this Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @brbrr's task in 2m 12s —— View job


Review: chore(ci): harden jemalloc cache resilience

The core idea is sound — installing jemalloc before Set up Go and recording pkg-config --variable=libdir jemalloc into .jemalloc-libdir so it can feed the Go module cache key is a reasonable way to bust the cache when the runner's jemalloc install path/version changes. The step reordering in both files is correct and necessary for the recorded path to exist before actions/setup-go runs.

Important

  • .github/workflows/build-binaries.yml:50-52 and .github/workflows/juno-test.yml:60-62: explicitly setting cache-dependency-path replaces actions/setup-go's default **/go.sum glob rather than adding to it. Both new lists only contain go.mod and .jemalloc-libdir, so go.sum no longer contributes to the Go module cache key — a dependency change that only touches go.sum won't invalidate the cache. Since the goal of this PR is cache resilience, go.sum should be included alongside go.mod and .jemalloc-libdir in both files. Left inline comments with the suggested fix on each.

No other correctness or safety issues found in the diff.

Comment on lines +50 to +52
cache-dependency-path: |
go.mod
.jemalloc-libdir

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Setting cache-dependency-path explicitly replaces actions/setup-go's default glob (**/go.sum) rather than adding to it. This list only hashes go.mod and .jemalloc-libdirgo.sum is no longer part of the Go module cache key, so a dependency bump that only touches go.sum (e.g. a new indirect requirement's checksum, or a go mod tidy that doesn't change go.mod's content) won't bust the module cache.

Since the goal here is cache resilience, this looks like it should include go.sum too:

cache-dependency-path: |
  go.mod
  go.sum
  .jemalloc-libdir

Fix this →

Comment on lines +60 to +62
cache-dependency-path: |
go.mod
.jemalloc-libdir

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same issue as in build-binaries.yml: this cache-dependency-path overrides actions/setup-go's default **/go.sum glob instead of extending it, so go.sum is dropped from the module cache key here too. Recommend adding it back:

cache-dependency-path: |
  go.mod
  go.sum
  .jemalloc-libdir

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🏅 Score: 88
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Ineffective cache

The setup-go step adds cache-dependency-path but does not enable cache: true, so the Go module cache is not keyed and Go dependencies are not cached. The added .jemalloc-libdir file therefore has no effect in juno-test; if Go caching is intended, add cache: true.

- name: Set up go
  uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v6.0.0
  with:
    go-version-file: go.mod
    cache-dependency-path: |
      go.mod
      .jemalloc-libdir

@brbrr
brbrr deployed to Development September 7, 2026 11:09 — with GitHub Actions Active
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.30%. Comparing base (5a74a35) to head (d465ae0).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4043      +/-   ##
==========================================
+ Coverage   79.28%   79.30%   +0.02%     
==========================================
  Files         464      465       +1     
  Lines       35793    35785       -8     
==========================================
+ Hits        28377    28381       +4     
+ Misses       7407     7395      -12     
  Partials        9        9              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant