[WIP] Adding retry logics to the mass-rebuild script #12529

Merged

jnsamyak merged 1 commit from f42_rebuild_retry into main

2025-03-04 09:23:54 +00:00

jnsamyak commented

2025-01-14 09:52:05 +00:00

Owner

Here’s a summary of what we added:

Retry Wrapper Function (retry):
- A generic function to wrap other functions and enable retrying failed operations.
- Configurable retry count (MAX_RETRIES) and delay (RETRY_DELAY) between attempts.
Integration into Core Functions:
- buildmeoutput, runme, and runmeoutput functions were modified to utilize the retry mechanism.
- Commands are retried on failure, and logs are generated for each attempt.
Improved Logging and Error Handling:
- Informative messages are displayed for each retry attempt and final failures.
- All attempts are logged, making debugging easier.
Scalability:
- The retry mechanism is designed to be generic, making it reusable across other parts of the script if needed.

Signed-off-by: Samyak Jain samyak.jn11@gmail.com

In the updated script, we introduced a retry mechanism to enhance the robustness of the build process. The retry logic ensures that temporary infrastructure or network issues do not cause permanent failures. Here’s a summary of what we added: 1. **Retry Wrapper Function (`retry`)**: - A generic function to wrap other functions and enable retrying failed operations. - Configurable retry count (`MAX_RETRIES`) and delay (`RETRY_DELAY`) between attempts. 2. **Integration into Core Functions**: - `buildmeoutput`, `runme`, and `runmeoutput` functions were modified to utilize the retry mechanism. - Commands are retried on failure, and logs are generated for each attempt. 3. **Improved Logging and Error Handling**: - Informative messages are displayed for each retry attempt and final failures. - All attempts are logged, making debugging easier. 4. **Scalability**: - The retry mechanism is designed to be generic, making it reusable across other parts of the script if needed. Signed-off-by: Samyak Jain <samyak.jn11@gmail.com>

kevin commented

2025-01-14 18:40:02 +00:00

Owner

This looks pretty nice for making sure submitting builds doesn't fail as much...

The other place in the past we have seen problems is git pushing the rebuild commit. Would it be difficult to add retry for that if it fails?

This looks pretty nice for making sure submitting builds doesn't fail as much... The other place in the past we have seen problems is git pushing the rebuild commit. Would it be difficult to add retry for that if it fails?

james commented

2025-01-14 20:31:03 +00:00

Member

Couple of minor points:

attempt() is a local function which returns 0 or 1 in buildmeoutput() / runme() and uses retry(), but returns 0 or "result" in runmeoutput() and can't use the retry() function.

retry() also has an attempt variable.

It's probably worth putting some randomization into the sleep() logic, and wait a bit longer than 3 seconds (failure aren't that common, right?) ... so it waits somewhere between 30s and 300s (5m) say. Also means if something temporary happens it doesn't fail 3 times in less than 10s.

But AFAICS, it should be better than nothing as is.

Couple of minor points: attempt() is a local function which returns 0 or 1 in buildmeoutput() / runme() and uses retry(), but returns 0 or "result" in runmeoutput() and can't use the retry() function. retry() also has an attempt variable. It's probably worth putting some randomization into the sleep() logic, and wait a bit longer than 3 seconds (failure aren't that common, right?) ... so it waits somewhere between 30s and 300s (5m) say. Also means if something temporary happens it doesn't fail 3 times in less than 10s. But AFAICS, it should be better than nothing as is.

james commented

2025-01-14 20:33:08 +00:00

Member

To be clear I'm not saying you should do a bunch of work to integrate all the code, if you want to clean up the first point. Just rename one or both functions, and maybe add a comment.

nphilipp commented

2025-01-15 09:44:37 +00:00

Member

If depending on a 3rd party Python package is okay for this script, take a look at backoff. It implements retrying with increasing backoff times, but works as a decorator on functions. You can specify that certain exceptions or return values flag a need for retry, so it should work with the existing code base.

Going on a tangent from James’s comments, the signalling in that script looks wonky to me – it should use exceptions for flagging errors instead of special return values. However, this isn’t a fault of this PR and probably should be addressed outside of it. If depending on a 3rd party Python package is okay for this script, take a look at [backoff](https://pypi.org/project/backoff/). It implements retrying with increasing backoff times, but works as a decorator on functions. You can specify that certain exceptions or return values flag a need for retry, so it should work with the existing code base.

kevin commented

2025-01-15 23:46:40 +00:00

Owner

So... the mass rebuild is ready to fire off now. Should we try and wait to address comments here? Or should we just merge this? Or should we just do the mass rebuild without it?

james commented

2025-01-16 04:31:33 +00:00

Member

I would 100% merge it before running the script tomorrow morning. It can be tidied up later, but it's going to make the mass rebuild less of a pain as is.