Skip to content

Commit ff81ffb

Browse files
committed
feat(demo): spread each change over a sharded file tree
## Summary ### Why? Every demo change wrote exactly one file, and every one of those files went into a single flat `demo/` directory. Two things follow from that, and neither is what the demo wants to show. The directory grows without bound as runs accumulate, until it is the kind of directory nobody wants in a repository. And a pull request that adds one line to one file is not much of a pull request — it exercises none of the handling a real change needs, and reads as a toy in a walkthrough whose whole purpose is to look like the real thing. ### What? A change now writes several files, each at a path sharded into two levels of hex buckets under `demo/` — `demo/c2/91/<tag>-<change>-<file>.txt`. The buckets are the leading bytes of the leaf name's SHA-256, so the spread is uniform without any coordination and stable across runs, and no directory becomes a dumping ground however many times the demo is run. Uniqueness is preserved, and it is load-bearing rather than incidental: the leaf carries the run tag, the change index and the file index, so no two files in a run and no two runs against one repository can name the same path. Independent changes that collided on content would make the run measure conflict handling instead of the throughput it exists to show. Two unrelated changes landing in the same bucket is expected and harmless — the bucket is only a directory, and it is the leaf that has to be distinct. How many files a change touches varies rather than being fixed, so a run does not produce a row of identically shaped pull requests. `-files` sets the floor (default 3, exposed as `FILES` on `make demo-pr`) and the actual count runs a little above it. The variation is derived from the run tag and the change index rather than from a clock, so replaying a tag reproduces the same run — a demo that cannot be reproduced is hard to talk about once something in it goes wrong. Each file is committed separately, so a change arrives as a multi-file, multi-commit pull request. That is closer to a real change, and it means the demo exercises replaying a range of commits rather than always handing the merger a single one. ## Test Plan ✅ `bazel test //service/submitqueue/demo/pr:go_default_test` — five new cases over the two pure functions: paths are unique across every change and file of two runs, the layout is the demo root plus hex buckets plus the leaf, 80 files spread over more than 50 distinct buckets (a layout that put everything in one directory would pass the uniqueness test alone), the file count respects its floor and clamps a non-positive one, and the count both varies across changes and reproduces for a given tag. ✅ Cross-checked the generated layout against an independent recomputation of the same hashes in shell, rather than only against the implementation's own output. Not yet exercised against GitHub: the path and count logic is covered above, but the multi-commit loop against the live API — several sequential commits on one branch, and the last SHA as the head the change URI pins — will first run on the next `make demo-pr`.
1 parent a240b6b commit ff81ffb

4 files changed

Lines changed: 159 additions & 12 deletions

File tree

Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export SQ_PROVIDER_CONFIG_DIR ?= $(REPO_ROOT)/service/submitqueue/demo/provider/
5555
# Defaults for `make land` / `make demo-pr` against the provider demo stack.
5656
DEMO_REPO ?= behinddwalls/sq-demo
5757
COUNT ?= 3
58+
FILES ?= 3
5859
STACKED ?= false
5960
LAND ?= true
6061
WATCH ?= true
@@ -152,10 +153,11 @@ clean-proto: ## Clean generated proto files
152153
@rm -f $(foreach p,$(PROTO_PACKAGES),$(p)/protopb/*.pb.go $(p)/protopb/*.pb.yarpc.go)
153154
@echo "Proto clean complete!"
154155

155-
demo-pr: ## Create N PRs in the demo repo, enqueue each as it is created, and watch (COUNT=3; needs GITHUB_TOKEN)
156+
demo-pr: ## Create N PRs in the demo repo, enqueue each as it is created, and watch (COUNT=3 FILES=3; needs GITHUB_TOKEN)
156157
@$(BAZEL) run //service/submitqueue/demo/pr -- \
157158
-repo $(DEMO_REPO) \
158159
-count $(COUNT) \
160+
-files $(FILES) \
159161
-stacked=$(STACKED) \
160162
-gateway $(GATEWAY_ADDR) \
161163
-queue $(QUEUE) \

doc/howto/PROVIDER-E2E.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ Opening pull requests by hand gets old fast. `demo-pr` creates them, enqueues th
9696
```bash
9797
make demo-pr # 3 independent PRs, each enqueued as it is created
9898
make demo-pr COUNT=8 # more traffic
99+
make demo-pr FILES=8 # wider changes, more files per PR
99100
make demo-pr STACKED=true # one stack, enqueued as a single request
100101
make demo-pr LAND=false # create only, print the land command
101102
```
@@ -120,7 +121,9 @@ The trail is only as detailed as what the pipeline reports, which today is `acce
120121

121122
`STACKED=true` is the exception to the overlap: one request carries the whole chain, so it can only go in once every pull request in it exists. That is the atomic-stack path — the whole set reaches `main` in a single push, and the table shows it as the single row it is.
122123

123-
It talks to GitHub over the REST API with the same `GITHUB_TOKEN`, so it needs no clone and no git binary. Each run tags its branches with a timestamp so repeated runs do not collide, and each change edits its own file so independent changes do not conflict by accident.
124+
It talks to GitHub over the REST API with the same `GITHUB_TOKEN`, so it needs no clone and no git binary. Each run tags its branches with a timestamp so repeated runs do not collide, and every file a change writes is at a path no other change uses, so independent changes do not conflict by accident.
125+
126+
A change touches several files rather than one, each committed separately, so it arrives as a multi-file, multi-commit pull request — closer to a real change, and enough to exercise replaying a range of commits. `FILES` sets the floor (default 3); the actual count varies a little above it, derived from the run tag so replaying a tag reproduces the same run. Paths are sharded into two levels of hex buckets under `demo/` (`demo/c2/91/<tag>-<change>-<file>.txt`), which keeps the tree from degenerating into one enormous directory as runs accumulate.
124127

125128
The command exits non-zero if any request settles anywhere other than `landed`, so it works in a script. Piped to a file it prints a fresh table whenever a request moves — and not when only the clock did — instead of redrawing in place.
126129

service/submitqueue/demo/pr/main.go

Lines changed: 71 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ package main
4747
import (
4848
"bytes"
4949
"context"
50+
"crypto/sha256"
5051
"encoding/base64"
5152
"encoding/json"
5253
"flag"
@@ -106,6 +107,7 @@ type config struct {
106107
repo string
107108
base string
108109
count int
110+
files int
109111
stacked bool
110112
prefix string
111113
land bool
@@ -123,6 +125,7 @@ func parseFlags() config {
123125
flag.StringVar(&c.repo, "repo", "behinddwalls/sq-demo", "scratch repository as owner/name")
124126
flag.StringVar(&c.base, "base", "main", "branch the changes target")
125127
flag.IntVar(&c.count, "count", 3, "how many pull requests to create")
128+
flag.IntVar(&c.files, "files", 3, "fewest files each pull request touches; the actual count varies a little above it")
126129
flag.BoolVar(&c.stacked, "stacked", false, "chain the pull requests and enqueue them as one stack")
127130
flag.StringVar(&c.prefix, "prefix", "demo", "branch name prefix")
128131
flag.BoolVar(&c.land, "land", true, "enqueue each pull request as it is created")
@@ -302,6 +305,51 @@ func (rw *row) stage() string {
302305
return absent
303306
}
304307

308+
// shardDirs is how many nested bucket directories a path carries under the demo
309+
// root. Two levels of 256 buckets spread a run's files widely enough that no
310+
// directory becomes a dumping ground, while staying shallow enough to read in a
311+
// diff.
312+
const shardDirs = 2
313+
314+
// changeFilePath returns the repository path for one file of one change.
315+
//
316+
// The leaf name carries the run tag, the change index and the file index, which
317+
// is what makes it unique: no two files in a run, and no two runs against the
318+
// same repository, can ever name the same path. That uniqueness is load-bearing
319+
// — see createAndEnqueue.
320+
//
321+
// The directories are the leading bytes of the leaf's SHA-256, so files land in
322+
// buckets that are uniform without any coordination and stable across runs. Two
323+
// unrelated changes sharing a bucket is expected and harmless: the bucket is
324+
// only a directory, and it is the leaf that has to be distinct.
325+
func changeFilePath(tag string, change, file int) string {
326+
leaf := fmt.Sprintf("%s-%d-%d.txt", tag, change, file)
327+
sum := sha256.Sum256([]byte(leaf))
328+
329+
parts := make([]string, 0, shardDirs+2)
330+
parts = append(parts, "demo")
331+
for i := 0; i < shardDirs; i++ {
332+
parts = append(parts, fmt.Sprintf("%02x", sum[i]))
333+
}
334+
parts = append(parts, leaf)
335+
return strings.Join(parts, "/")
336+
}
337+
338+
// changeFileCount returns how many files a change touches: at least min, varied
339+
// a little so a run does not produce a row of identically shaped pull requests.
340+
//
341+
// The variation is derived from the run tag and the change index rather than
342+
// from a clock or a global source of randomness, so replaying a tag reproduces
343+
// the same run. A demo that cannot be reproduced is hard to talk about when
344+
// something in it goes wrong.
345+
func changeFileCount(tag string, change, min int) int {
346+
if min < 1 {
347+
min = 1
348+
}
349+
sum := sha256.Sum256([]byte(fmt.Sprintf("%s#%d", tag, change)))
350+
return min + int(sum[0]%4)
351+
}
352+
305353
// createAndEnqueue opens the pull requests and puts them on the queue, filling
306354
// in the tracker's rows as it goes and reporting each step beneath the table.
307355
//
@@ -311,10 +359,13 @@ func (rw *row) stage() string {
311359
// carries the whole chain, so it can only be submitted once the chain is
312360
// complete.
313361
//
314-
// Each change edits its own file. Independent changes would otherwise collide
315-
// on content and the run would measure conflict handling rather than the
316-
// throughput it is trying to show; a caller wanting a conflict can make one
317-
// deliberately.
362+
// Every file a change writes is its own, at a path no other change uses.
363+
// Independent changes would otherwise collide on content and the run would
364+
// measure conflict handling rather than the throughput it is trying to show; a
365+
// caller wanting a conflict can make one deliberately. Each change spreads
366+
// several files across the sharded tree, so it arrives as a multi-file, multi-
367+
// commit pull request rather than a single-line edit — which is both closer to
368+
// a real change and enough to exercise replaying a range of commits.
318369
func createAndEnqueue(
319370
ctx context.Context,
320371
gh *githubClient,
@@ -340,12 +391,22 @@ func createAndEnqueue(
340391
return nil, fmt.Errorf("create branch %s: %w", branch, err)
341392
}
342393

343-
path := fmt.Sprintf("demo/%s-%d.txt", tag, i)
344-
body := fmt.Sprintf("change %d of run %s\n", i, tag)
345-
t.note("committing %s", path)
346-
headSHA, err := gh.commitFile(ctx, branch, path, body, fmt.Sprintf("demo change %d (run %s)", i, tag))
347-
if err != nil {
348-
return nil, fmt.Errorf("commit to %s: %w", branch, err)
394+
// Each file is its own commit, so the pull request arrives as a range of
395+
// commits rather than a single edit. The last one is the head the change
396+
// URI pins.
397+
var headSHA string
398+
fileCount := changeFileCount(tag, i, cfg.files)
399+
for k := 1; k <= fileCount; k++ {
400+
path := changeFilePath(tag, i, k)
401+
body := fmt.Sprintf("change %d of run %s\nfile %d of %d\n", i, tag, k, fileCount)
402+
t.note("committing %s (%d/%d)", path, k, fileCount)
403+
404+
message := fmt.Sprintf("demo change %d (run %s): file %d of %d", i, tag, k, fileCount)
405+
sha, err := gh.commitFile(ctx, branch, path, body, message)
406+
if err != nil {
407+
return nil, fmt.Errorf("commit %s to %s: %w", path, branch, err)
408+
}
409+
headSHA = sha
349410
}
350411

351412
t.note("opening pull request for %s", branch)

service/submitqueue/demo/pr/main_test.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,3 +656,84 @@ func head(s string, n int) string {
656656
}
657657
return s[:n]
658658
}
659+
660+
func TestChangeFilePath_IsUniquePerFileAcrossChangesAndRuns(t *testing.T) {
661+
// Uniqueness is the property the whole layout rests on: two changes writing
662+
// the same path would collide on content, and the run would measure conflict
663+
// handling instead of throughput.
664+
seen := make(map[string]string)
665+
for _, tag := range []string{"0810-1203", "0810-1204"} {
666+
for change := 1; change <= 20; change++ {
667+
for file := 1; file <= 8; file++ {
668+
path := changeFilePath(tag, change, file)
669+
owner := fmt.Sprintf("%s/%d/%d", tag, change, file)
670+
if prev, ok := seen[path]; ok {
671+
t.Fatalf("path %s produced for both %s and %s", path, prev, owner)
672+
}
673+
seen[path] = owner
674+
}
675+
}
676+
}
677+
}
678+
679+
func TestChangeFilePath_ShardsUnderTheDemoRoot(t *testing.T) {
680+
path := changeFilePath("0810-1203", 1, 1)
681+
682+
parts := strings.Split(path, "/")
683+
require.Len(t, parts, shardDirs+2, "demo root, %d bucket dirs, and the leaf", shardDirs)
684+
assert.Equal(t, "demo", parts[0])
685+
for _, bucket := range parts[1 : len(parts)-1] {
686+
assert.Len(t, bucket, 2, "each bucket is one hex byte")
687+
assert.Regexp(t, "^[0-9a-f]{2}$", bucket)
688+
}
689+
assert.Equal(t, "0810-1203-1-1.txt", parts[len(parts)-1])
690+
}
691+
692+
func TestChangeFilePath_SpreadsAcrossManyBuckets(t *testing.T) {
693+
// A layout that puts everything in one directory would satisfy the
694+
// uniqueness test above while defeating the point of sharding.
695+
buckets := make(map[string]struct{})
696+
for change := 1; change <= 20; change++ {
697+
for file := 1; file <= 4; file++ {
698+
parts := strings.Split(changeFilePath("0810-1203", change, file), "/")
699+
buckets[strings.Join(parts[1:len(parts)-1], "/")] = struct{}{}
700+
}
701+
}
702+
assert.Greater(t, len(buckets), 50, "80 files should land in many distinct buckets")
703+
}
704+
705+
func TestChangeFileCount(t *testing.T) {
706+
tests := []struct {
707+
name string
708+
min int
709+
}{
710+
{name: "default minimum", min: 3},
711+
{name: "single file floor", min: 1},
712+
{name: "non-positive is clamped", min: 0},
713+
{name: "negative is clamped", min: -5},
714+
}
715+
for _, tt := range tests {
716+
t.Run(tt.name, func(t *testing.T) {
717+
floor := tt.min
718+
if floor < 1 {
719+
floor = 1
720+
}
721+
for change := 1; change <= 50; change++ {
722+
got := changeFileCount("0810-1203", change, tt.min)
723+
assert.GreaterOrEqual(t, got, floor)
724+
assert.LessOrEqual(t, got, floor+3)
725+
}
726+
})
727+
}
728+
}
729+
730+
func TestChangeFileCount_VariesButIsReproducible(t *testing.T) {
731+
counts := make(map[int]struct{})
732+
for change := 1; change <= 30; change++ {
733+
got := changeFileCount("0810-1203", change, 3)
734+
counts[got] = struct{}{}
735+
assert.Equal(t, got, changeFileCount("0810-1203", change, 3),
736+
"replaying a tag must reproduce the run")
737+
}
738+
assert.Greater(t, len(counts), 1, "the count should vary across changes, not be constant")
739+
}

0 commit comments

Comments
 (0)