aws-samples / aws-samples/aws-transform-custom-samples

scaled-execution-bash: parallel runs crash on macOS, and failed transformations are reported as SUCCESS

Open
#82 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Shell
Stars
37
Forks
18
Avg merge
5d 9h
Merged PRs (30d)
7

Description

Six bugs in `scaled-execution-bash/atx-custom-automation.sh`. Two of them stop a default-configuration run outright on macOS; the other four mean the results the script reports cannot be trusted.

I have a fix for all six, tested end to end. Opening this first per CONTRIBUTING rather than sending the PR straight away.

## 1. Parallel runs die at the default settings on macOS (`new_pids[@]: unbound variable`)

Line 966-968:

```bash
local new_pids=()
for pid in "${pids[@]}"; do
kill -0 "$pid" 2>/dev/null && new_pids+=($pid)
done
pids=("${new_pids[@]}")
```

Under `set -u`, bash 3.2 - which is `/bin/bash` on macOS, and the shebang here is `#!/bin/bash` - treats an empty-array expansion as unbound and aborts. It fires as soon as every in-flight job finishes while repos are still queued, so any run where repo count > `--max-jobs` can die. With 40 repos at the default `--max-jobs 10`, only 10 were processed.

The wait loop 40 lines below already has the right guard (`"${new_pids[@]+"${new_pids[@]}"}"` at line 1011) - this copy just missed it.

## 2. `--resume` crashes once everything is COMPLETED (`filtered_repos[@]: unbound variable`)

Same empty-array class, line 1902. `log_success "All repositories already completed!"` at line 1911 is unreachable. This is mostly hidden today because bug 4 leaves repos stuck in a non-COMPLETED state, so "everything completed" rarely happens.

## 3. Failed transformations are recorded as SUCCESS

Lines 595/598 append `|| true` to the atx pipeline, then line 600 reads `${PIPESTATUS[0]}`:

```bash
"${atx_cmd_args[@]}" 2>&1 | tee -a "$log_file" || true
cmd_exit=${PIPESTATUS[0]}
```

`a | b || true` is one AND-OR list. When the pipeline fails, bash runs `true`, and that resets `PIPESTATUS` to `(0)`. So `cmd_exit` is 0 exactly when atx failed:

```
$ bash -c 'set -euo pipefail; (exit 7) | cat || true; echo ${PIPESTATUS[0]}'
0
```

The only remaining detector is the grep over the nine hardcoded `ATX_ERROR_PATTERNS`, so any failure whose output does not contain one of those strings - a build failure, a crash, an OOM, a timeout - is written to results.txt as SUCCESS. With a stub `atx` that exits 1, a 3-repo run reported `Success: 3 | Failed: 0`.

## 4. Status updates are dropped under contention, which corrupts `--resume`

Line 182:

```bash
mkdir "$lockfile" 2>/dev/null || return 0
```

One attempt, and on failure it returns success without writing anything. `write_with_lock` 40 lines above already retries for 30s - `mark_repo_status` just does not use it.

Measured on 40-repo parallel runs: repos that finished (they have a row in results.txt) but whose `.atx-batch-status` never reached a terminal state - 7, 7 and 11 across three runs. Those repos get re-run on the next `--resume`.

## 5. `--resume` wipes the previous attempt's results

Line 1946 writes the results.txt header unconditionally, and in resume mode `OUTPUT_DIR` resolves to the previous run's directory. A 40-row results.txt became 7 rows after a resume, and the summary then reported `Total Repositories | 7 ... Success Rate | 100%`.

## 6. The summary report aborts before printing (only visible once 3 is fixed)

Line 1275 ends a `while read` body with `[[ "$status" == "FAILED" ]] && printf ...`. When the last row is SUCCESS the AND-list returns 1, the `while` returns 1, and `set -e` kills main() part-way through the summary block - console summary, failed_repos.csv and the closing message never run, and the script exits 1.

This is pre-existing but dormant, because bug 3 means `failed` is almost always 0. Fixing 3 makes it fire on nearly every batch that has a failure, so the two cannot be separated.

## What the fix looks like

Six commits, ~104 lines, one file:

- guard both empty-array expansions the way line 1011 already does
- `set +o pipefail` around the atx pipeline instead of `|| true`, capturing `PIPESTATUS[0]` inside each branch
- give `mark_repo_status` the same retry loop as `write_with_lock`, failing **open** on timeout (returning non-zero there kills the caller under `set -euo pipefail` - I tried it)
- write the results.txt header only when the file does not exist, and drop the rows for repos about to be re-run
- make the FAILED loop return 0

Verified with a stub `atx` binary and local git repos, so no AWS calls and no ATX service access needed:

| | before | after |
|---|---|---|
| 40-repo parallel, lost status rows | 7 / 7 / 11 | 0 / 0 / 0 |
| stub atx exits 1 | `SUCCESS` | `FAILED` |
| resume ladder (40 repos, 10 failing) | results.txt truncated to 7 rows | 40 rows, 30 -> 37 -> 40 SUCCESS, then "All repositories already completed!" |

Happy to send it as one PR, or split if you would rather review the two crashes separately from the reporting fixes. Also happy to leave it if this script is not something you want changes to right now.

Contributor guide

Open the contributing guide

Research direction

Start in scaled-execution-bash/atx-custom-automation.sh, focusing on the cited parallel-job, resume, status-lock, pipeline, and summary-report sections. Reproduce the cases with a stub atx binary and local git repositories; done means parallel runs retain all statuses, failures are reported as FAILED, resume preserves results, completed runs exit cleanly, and summaries finish.

Written by the indexing model from the issue text.

Assessment

Tech stack
bash
Domain
cli, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.