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

Reduce HTTP requests for checksum file in fetch-checksum #536

Closed
eleanorLYJ opened this issue Jan 11, 2025 · 3 comments
Closed

Reduce HTTP requests for checksum file in fetch-checksum #536

eleanorLYJ opened this issue Jan 11, 2025 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@eleanorLYJ
Copy link
Collaborator

After adding more test cases, the CI pipeline fails due to excessive HTTP requests to GitHub during the fetch-checksum step. This triggers rate-limiting errors (403: rate limit exceeded).

Proposed Solution:

Check if the checksum file exists and is less than one day old before fetching it. This will reduce unnecessary requests while ensuring the file is up-to-date.

@jserv
Copy link
Contributor

jserv commented Jan 11, 2025

Network quality issues may sometimes cause unexpected disconnections when downloading from certain remote sites, requiring retries.

@vacantron
Copy link
Collaborator

vacantron commented Feb 3, 2025

It seems to be too many requests in a short period that make artifact is called when running make all . Maybe maintain the local copies of sha1sum files which use the recently commit hash as the postfix of them to reduce the refetching?

@vacantron
Copy link
Collaborator

vacantron commented Feb 10, 2025

I think the main reason causing this is

rv32emu/mk/artifact.mk

Lines 39 to 47 in c9dfd36

ifeq ($(call has, PREBUILT), 1)
ifeq ($(call has, SYSTEM), 1)
LATEST_RELEASE := $(shell wget -q https://api.github.com/repos/sysprog21/rv32emu-prebuilt/releases -O- | grep '"tag_name"' | grep "Linux-Image" | head -n 1 | sed -E 's/.*"tag_name": "([^"]+)".*/\1/')
else ifeq ($(call has, ARCH_TEST), 1)
LATEST_RELEASE := $(shell wget -q https://api.github.com/repos/sysprog21/rv32emu-prebuilt/releases -O- | grep '"tag_name"' | grep "sail" | head -n 1 | sed -E 's/.*"tag_name": "([^"]+)".*/\1/')
else
LATEST_RELEASE := $(shell wget -q https://api.github.com/repos/sysprog21/rv32emu-prebuilt/releases -O- | grep '"tag_name"' | grep "ELF" | head -n 1 | sed -E 's/.*"tag_name": "([^"]+)".*/\1/')
endif
PREBUILT_BLOB_URL = https://github.com/sysprog21/rv32emu-prebuilt/releases/download/$(LATEST_RELEASE)

Every time we run make, the GitHub API is called. We can reduce the number of HTTP requests by checking the prebuilt files is existing or not.

However, in CI, we always run make distclean before building. The method mentioned above would not work.

rv32emu/Makefile

Lines 418 to 421 in c9dfd36

distclean: clean
-$(RM) $(DOOM_DATA) $(QUAKE_DATA) $(BUILDROOT_DATA) $(LINUX_DATA)
$(RM) -r $(OUT)/linux-image
$(RM) -r $(TIMIDITY_DATA)

@jserv jserv added the bug Something isn't working label Feb 10, 2025
vacantron added a commit that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close #536
vacantron added a commit to vacantron/rv32emu that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close sysprog21#536
vacantron added a commit to vacantron/rv32emu that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close sysprog21#536
vacantron added a commit to vacantron/rv32emu that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close sysprog21#536
vacantron added a commit to vacantron/rv32emu that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close sysprog21#536
vacantron added a commit to vacantron/rv32emu that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close sysprog21#536
vacantron added a commit to vacantron/rv32emu that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close sysprog21#536
vacantron added a commit to vacantron/rv32emu that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close sysprog21#536
vacantron added a commit to vacantron/rv32emu that referenced this issue Feb 11, 2025
Check prebuilt tarballs are existing or not first. If so, skip the
downloading of releases tags and tarballs.

In the CI, we first call "make artifact" to prepare the tarballs, so the
GitHub API for fetching releases tags would not be called and be skipped
when "make" is called next time.

After this commit, wget in the CI would not complain about error code
403 (rate limit exceeded) anymore. However, the build on Aarch64 still
failed because of other reansons, e.g. software bugs of GCC or QEMU.

Close sysprog21#536
@jserv jserv closed this as completed in c85279e Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants