-
Notifications
You must be signed in to change notification settings - Fork 21
Race detection is nineteen minutes, and seventeen of them are one package #777
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
Merged
+345
−9
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| #!/usr/bin/env bash | ||
| # race-shard-union-test.sh — the union check's own tests. | ||
| # | ||
| # The union check is the reason it is safe to shard this job at all, so it | ||
| # is worth knowing it fails. Each case below is a way the shards could | ||
| # silently stop covering the suite; all of them must be refused. | ||
| set -euo pipefail | ||
| cd "$(dirname "$0")/.." | ||
| union=scripts/race-shard-union.sh | ||
|
|
||
| work=$(mktemp -d) | ||
| trap 'rm -rf "$work"' EXIT | ||
|
|
||
| plant() { | ||
| rm -rf "${work:?}"/* | ||
| for i in 1 2 3 4; do | ||
| mkdir -p "$work/shard-$i" | ||
| printf 'pkg/a\tTestOne\npkg/a\tTestTwo\npkg/b\tTestThree\npkg/b\tTestFour\n' > "$work/shard-$i/all.txt" | ||
| done | ||
| printf 'pkg/a\tTestOne\n' > "$work/shard-1/shard.1.txt" | ||
| printf 'pkg/a\tTestTwo\n' > "$work/shard-2/shard.2.txt" | ||
| printf 'pkg/b\tTestThree\n' > "$work/shard-3/shard.3.txt" | ||
| printf 'pkg/b\tTestFour\n' > "$work/shard-4/shard.4.txt" | ||
| } | ||
|
|
||
| refuses() { | ||
| local what="$1" | ||
| if "$union" "$work" 4 >/dev/null 2>&1; then | ||
| echo "FAIL: the union check accepted $what" >&2 | ||
| exit 1 | ||
| fi | ||
| echo "ok - refuses $what" | ||
| } | ||
|
|
||
| plant | ||
| "$union" "$work" 4 >/dev/null || { echo "FAIL: the union check refused a complete, disjoint split" >&2; exit 1; } | ||
| echo "ok - accepts a complete, disjoint split" | ||
|
|
||
| plant; : > "$work/shard-3/shard.3.txt" | ||
| refuses "a shard assigned nothing" | ||
|
|
||
| plant; rm "$work/shard-2/shard.2.txt" | ||
| refuses "a shard that recorded no assignment" | ||
|
|
||
| plant; printf 'pkg/a\tTestOne\n' >> "$work/shard-3/shard.3.txt" | ||
| refuses "the same test assigned to two shards" | ||
|
|
||
| plant; printf 'pkg/b\tTestFive\n' >> "$work/shard-1/all.txt" | ||
| refuses "a test in the enumeration and in no shard" | ||
|
|
||
| plant; printf 'pkg/b\tTestFive\n' >> "$work/shard-2/all.txt" | ||
| refuses "shards that enumerated different sets of tests" | ||
|
|
||
| plant; printf 'pkg/z\tTestGhost\n' >> "$work/shard-4/shard.4.txt" | ||
| refuses "a test assigned that the repository does not have" | ||
|
|
||
| plant; : > "$work/shard-1/all.txt" | ||
| refuses "a shard whose enumeration is empty" | ||
|
|
||
| # The aggregate reads the shards through uploaded artifacts, so a report | ||
| # that never arrives looks exactly like a shard with nothing to say. It | ||
| # must not: an aggregator that cannot read a shard's report would have an | ||
| # opinion about it anyway. | ||
| plant; rm -rf "$work/shard-2" | ||
| refuses "a shard whose report never arrived" | ||
|
|
||
| plant; mv "$work/shard-3" "$work/race-shard-3" | ||
| refuses "a report that arrived under a name the checker does not read" | ||
|
|
||
| # The check must not depend on how seq reads a backwards range. macOS ships | ||
| # seq, but BSD seq treats "first larger than last" as counting down, and the | ||
| # overlap loop ends on exactly that range. With seq, this case compared the | ||
| # last shard against itself and refused every correct split on macOS. | ||
| plant | ||
| bsd=$work/bsdbin | ||
| mkdir -p "$bsd" | ||
| cat > "$bsd/seq" <<'SEQ' | ||
| #!/usr/bin/env bash | ||
| # BSD seq(1): "If first is larger than last the default incr is -1." | ||
| first=$1; last=$2 | ||
| if [ "$first" -gt "$last" ]; then | ||
| for ((n = first; n >= last; n--)); do echo "$n"; done | ||
| else | ||
| for ((n = first; n <= last; n++)); do echo "$n"; done | ||
| fi | ||
| SEQ | ||
| chmod +x "$bsd/seq" | ||
| PATH="$bsd:$PATH" "$union" "$work" 4 >/dev/null || { | ||
| echo "FAIL: the union check refused a complete, disjoint split where seq counts down over a backwards range, as BSD seq does on macOS" >&2 | ||
| exit 1 | ||
| } | ||
| echo "ok - accepts a complete, disjoint split under BSD seq semantics" | ||
|
|
||
| echo "All union checks behaved." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| #!/usr/bin/env bash | ||
| # race-shard-union.sh - the aggregate job's check that the shards between | ||
| # them ran every test, once. | ||
| # | ||
| # This is the point of sharding the race job rather than a nicety attached to | ||
| # it. A split that silently stops running some tests reports a fast green and | ||
| # measures nothing, which is the defect this repository has spent a lot of | ||
| # effort closing elsewhere. So every one of these is a failure, not a warning: | ||
| # | ||
| # - a shard that enumerated a different set of tests than its siblings, | ||
| # which means they did not all see the same repository | ||
| # - the same test assigned to two shards, which means the split is wrong | ||
| # even where it looks complete | ||
| # - a test in no shard at all, which is the one that matters | ||
| # | ||
| # race-shard-union.sh <directory of shard records> <number of shards> | ||
| set -euo pipefail | ||
|
|
||
| dir="${1:?directory holding the shard records}" | ||
| total="${2:?number of shards}" | ||
|
|
||
| fail() { echo "SHARD COVERAGE FAILED: $*" >&2; exit 1; } | ||
|
|
||
| sorted() { LC_ALL=C sort -u "$1"; } | ||
|
|
||
| # Compared as sorted files rather than hashed: cmp is POSIX and md5sum is | ||
| # not on macOS, and this check is in make check, which people run locally. | ||
| # | ||
| # The loops count in the shell rather than through seq for the same reason. | ||
| # macOS does ship seq, but BSD seq reads "first larger than last" as a | ||
| # request to count down, so `seq 5 4` prints "5 4" where GNU seq prints | ||
| # nothing. The inner loop below ends on exactly that range, so on macOS | ||
| # every correct run was refused, comparing the last shard against itself. | ||
| for ((i = 1; i <= total; i++)); do | ||
| [ -s "$dir/shard-$i/all.txt" ] || fail "shard $i recorded no enumeration; either it did not get far enough to have one, or its report did not reach here" | ||
| sorted "$dir/shard-$i/all.txt" > "$dir/all.$i.sorted" | ||
| if [ "$i" != 1 ] && ! cmp -s "$dir/all.1.sorted" "$dir/all.$i.sorted"; then | ||
| fail "shard $i enumerated a different set of tests than shard 1, so the shards did not all see the same repository" | ||
| fi | ||
| done | ||
|
|
||
| for ((i = 1; i <= total; i++)); do | ||
| [ -s "$dir/shard-$i/shard.$i.txt" ] || fail "shard $i was assigned no tests" | ||
| for ((j = i + 1; j <= total; j++)); do | ||
| overlap=$(LC_ALL=C comm -12 <(sorted "$dir/shard-$i/shard.$i.txt") <(sorted "$dir/shard-$j/shard.$j.txt")) | ||
| [ -z "$overlap" ] || fail "shards $i and $j were both assigned $(echo "$overlap" | wc -l) test(s), e.g. $(echo "$overlap" | head -1)" | ||
| done | ||
| done | ||
|
|
||
| cat "$dir"/shard-*/shard.*.txt | LC_ALL=C sort -u > "$dir/union.txt" | ||
| cp "$dir/all.1.sorted" "$dir/expected.txt" | ||
|
|
||
| missing=$(LC_ALL=C comm -23 "$dir/expected.txt" "$dir/union.txt") | ||
| [ -z "$missing" ] || fail "$(echo "$missing" | wc -l) test(s) were in no shard and so did not run, e.g. $(echo "$missing" | head -1)" | ||
|
|
||
| extra=$(LC_ALL=C comm -13 "$dir/expected.txt" "$dir/union.txt") | ||
| [ -z "$extra" ] || fail "$(echo "$extra" | wc -l) test(s) were assigned that the repository does not have, e.g. $(echo "$extra" | head -1)" | ||
|
|
||
| echo "shard coverage OK: $(wc -l < "$dir/expected.txt") tests across $total shards, disjoint and complete" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| #!/usr/bin/env bash | ||
| # race-shard.sh - run one shard of the race-detector suite. | ||
| # | ||
| # Race detection was one job of nineteen minutes, and seventeen of them were | ||
| # one package: internal/connector is 1031s of a 1061s critical path. Sharding | ||
| # by package therefore buys nothing — whichever shard gets that package still | ||
| # takes seventeen minutes — so the unit here is the test, not the package. | ||
| # | ||
| # The hazard that comes with that is the one this repository keeps finding: | ||
| # a shard scheme that silently stops running some tests is green and measures | ||
| # nothing. Two checks answer it. This script refuses to report success unless | ||
| # every test it was assigned actually ran, and race-shard-union.sh, in the | ||
| # aggregate job, refuses unless the shards between them covered the lot. | ||
| # | ||
| # race-shard.sh <index> <total> <output directory> | ||
| set -euo pipefail | ||
|
|
||
| index="${1:?shard index, 1-based}" | ||
| total="${2:?number of shards}" | ||
| out="${3:?directory to write the shard record into}" | ||
| mkdir -p "$out" | ||
|
|
||
| all="$out/all.txt" | ||
| plan="$out/shard.$index.txt" | ||
|
|
||
| # -json, not a bare -list: a plain listing prints test names with no package, | ||
| # and six names in this repository exist in more than one. The union check | ||
| # downstream compares package-and-test pairs, so the attribution has to be | ||
| # unambiguous here. | ||
| go test -tags dev -list '.*' -json ./... | | ||
| python3 -c ' | ||
| import sys, json | ||
| for line in sys.stdin: | ||
| try: | ||
| event = json.loads(line) | ||
| except ValueError: | ||
| continue | ||
| if event.get("Action") != "output": | ||
| continue | ||
| name = event.get("Output", "").strip() | ||
| # Benchmarks are listed but -run does not run them without -bench, so | ||
| # assigning one would be assigning work no shard can do. The race job | ||
| # has never run them; it does not start now. | ||
| if name and " " not in name and name.startswith(("Test", "Fuzz", "Example")): | ||
| print(event["Package"] + "\t" + name) | ||
| ' | LC_ALL=C sort -u > "$all" | ||
|
|
||
| [ -s "$all" ] || { echo "enumerated no tests at all" >&2; exit 1; } | ||
| awk -v i="$index" -v n="$total" 'NR % n == i % n' "$all" > "$plan" | ||
| [ -s "$plan" ] || { echo "shard $index of $total was assigned no tests" >&2; exit 1; } | ||
|
|
||
| echo "shard $index of $total: $(wc -l < "$plan") of $(wc -l < "$all") tests" | ||
|
|
||
| # One invocation over ./... keeps go test's own package parallelism. A name | ||
| # that exists in two packages runs in both, which costs eight extra tests | ||
| # across the repository and keeps the command line to a single regex. | ||
| regex="^($(cut -f2 "$plan" | LC_ALL=C sort -u | paste -sd'|'))$" | ||
|
|
||
| set +e | ||
| go test -tags dev -race -timeout 20m -json -run "$regex" ./... | tee "$out/events.$index.json" | | ||
| python3 -c ' | ||
| import sys, json | ||
| for line in sys.stdin: | ||
| try: | ||
| event = json.loads(line) | ||
| except ValueError: | ||
| sys.stdout.write(line) | ||
| continue | ||
| if event.get("Action") == "output": | ||
| sys.stdout.write(event.get("Output", "")) | ||
| ' | ||
| status=${PIPESTATUS[0]} | ||
| set -e | ||
|
|
||
| # A -run regex that matches nothing exits 0 with "no tests to run". That is | ||
| # exactly the shape of failure this scheme has to rule out, so what ran is | ||
| # compared against what was assigned. | ||
| python3 - "$plan" "$out/events.$index.json" <<'PY' | ||
| import json, sys | ||
|
|
||
| plan, events = sys.argv[1], sys.argv[2] | ||
|
|
||
| assigned = set() | ||
| with open(plan) as handle: | ||
| for line in handle: | ||
| package, _, name = line.rstrip("\n").partition("\t") | ||
| if name: | ||
| assigned.add((package, name)) | ||
|
|
||
| ran = set() | ||
| with open(events) as handle: | ||
| for line in handle: | ||
| try: | ||
| event = json.loads(line) | ||
| except ValueError: | ||
| continue | ||
| name = event.get("Test", "") | ||
| if event.get("Action") == "run" and name and "/" not in name: | ||
| ran.add((event["Package"], name)) | ||
|
|
||
| missing = assigned - ran | ||
| if missing: | ||
| print(f"SHARD RAN {len(assigned) - len(missing)} OF ITS {len(assigned)} TESTS", file=sys.stderr) | ||
| for package, name in sorted(missing)[:20]: | ||
| print(f" did not run: {package}\t{name}", file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| print(f"shard ran all {len(assigned)} tests it was assigned") | ||
| PY | ||
|
|
||
| exit "$status" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.