Skip to content

Comments

HDDS-10273. Intermittent build failure while downloading nodejs#6664

Merged
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-10273
May 14, 2024
Merged

HDDS-10273. Intermittent build failure while downloading nodejs#6664
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-10273

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Download from nodejs.org seems to be unreliable.

This PR fixes intermittent failures in:

  • compile (8, macos-12), which previously had to download Node.js for each run, since the binary tarball is platform-specific, and we only cached the one for Linux
  • various checks in the rare case that populate-cache runs into download problem

Changes:

  1. Download both Linux and Mac versions of Node.js. Use curl, with retry enabled.
  2. Indicate partially built cache by creating a marker file, which is deleted if all downloads succeed.
  3. Log cache contents even in case of cache hit, making it easier to check contents.
  4. Allow triggering populate-cache workflow manually for easier testing.

https://issues.apache.org/jira/browse/HDDS-10273

How was this patch tested?

Triggered the workflow manually in my fork:
https://github.com/adoroszlai/ozone/actions/workflows/populate-cache.yml

Node.js tarballs from latest run:
https://github.com/adoroszlai/ozone/actions/runs/9018931552/job/24780630992#step:8:300

@adoroszlai adoroszlai added the CI label May 9, 2024
@adoroszlai adoroszlai self-assigned this May 9, 2024
@adoroszlai adoroszlai requested review from errose28 and smengcl May 9, 2024 17:48
@errose28
Copy link
Contributor

errose28 commented May 9, 2024

Overall idea makes sense, but why do we do so much manual cache management? It seems like https://github.com/actions/setup-java and https://github.com/actions/setup-node can handle it for us. Are there extra requirements we have?

@adoroszlai
Copy link
Contributor Author

Overall idea makes sense, but why do we do so much manual cache management? It seems like https://github.com/actions/setup-java and https://github.com/actions/setup-node can handle it for us. Are there extra requirements we have?

  1. setup-java didn't have this feature when we first started using cache. Its simple config seems to correspond to our initial implementation with cache, so if this was available at the time, we could have chosen it.
  2. setup-java does not give control over building a cache vs. only using it. Ozone has many concurrent checks which execute different Maven plugins. Basic checks like checkstyle can benefit from using the cache, but shouldn't build it, since they do not download many of the dependencies. So we rely on cache/save and cache/restore.
  3. Building the cache from scratch in a separate workflow:
    • can produce a smaller cache, by not having to carry old dependencies forever,
    • helps avoid cache expiry, which may be a problem in forks with little activity.

Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

Thanks @adoroszlai !

@adoroszlai adoroszlai merged commit fdec8f8 into apache:master May 14, 2024
@adoroszlai adoroszlai deleted the HDDS-10273 branch May 14, 2024 06:26
@adoroszlai
Copy link
Contributor Author

Thanks @errose28, @smengcl for the review.

adoroszlai added a commit that referenced this pull request May 14, 2024
@adoroszlai
Copy link
Contributor Author

It turns out cache/save cannot overwrite existing cache, so I reverted this. Will post new PR.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants