Skip to content

Commit 9f8a3ef

Browse files
authored
feat(demo): add BURST mode for a simultaneous enqueue of independent changes (#615)
## Summary Add -burst (Makefile BURST=false) which, for independent changes, creates every change first and only then fires all the Land calls together, so the requests reach the queue in one burst instead of being enqueued as each is created. It works for every provider: burst runs each source through the same two phases, so fake creates instantly, github creates concurrently, git creates serially on its single work tree — and in all three the enqueues arrive together once creation is done. Burst does not make creation faster; with the git source it is still serialized on one work tree. It only separates creation from enqueuing so the publishes are not spread across the creation phase. The default is unchanged — each change lands the moment it exists, so the queue starts working during creation. A small lander seam lets the two-phase ordering be tested without a live gateway. ## Test Plan ✅ New unit test asserts nothing is enqueued until every change is created, provider-agnostically (a `lander` seam + a recording source stand in for any provider); shape banner covered; existing `//service/submitqueue/demo/requests` tests green. ## Stack 1. @ #615 1. #616
1 parent 0892fcf commit 9f8a3ef

4 files changed

Lines changed: 146 additions & 4 deletions

File tree

Makefile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ FOLDERS ?= 0
7878
FILES ?= 3
7979
CONCURRENCY ?= 5
8080
STACKED ?= false
81+
# BURST=true creates every change first, then enqueues them all at once instead
82+
# of as each is created. Independent changes only; a stack always lands as one.
83+
BURST ?= false
8184
SINCE ?= 1h
8285
LIMIT ?= 50
8386
LAND ?= true
@@ -240,7 +243,7 @@ clean-proto: ## Clean generated proto files
240243
@rm -f $(foreach p,$(PROTO_PACKAGES),$(p)/protopb/*.pb.go $(p)/protopb/*.pb.yarpc.go)
241244
@echo "Proto clean complete!"
242245

243-
demo-requests: ## Create N changes, enqueue each as it is created, and watch (PROVIDER=fake|git|github COUNT=3 FOLDERS=0 FILES=3 CONCURRENCY=5)
246+
demo-requests: ## Create N changes, enqueue each as it is created, and watch (PROVIDER=fake|git|github COUNT=3 FOLDERS=0 FILES=3 CONCURRENCY=5 BURST=false)
244247
@set -e; $(resolve_gateway_addr); $(resolve_provider); \
245248
$(BAZEL) run //service/submitqueue/demo/requests -- \
246249
-provider $$provider \
@@ -251,6 +254,7 @@ demo-requests: ## Create N changes, enqueue each as it is created, and watch (PR
251254
-files $(FILES) \
252255
-concurrency $(CONCURRENCY) \
253256
-stacked=$(STACKED) \
257+
-burst=$(BURST) \
254258
-addr $$addr \
255259
-queue $(QUEUE) \
256260
-strategy $(STRATEGY) \

service/submitqueue/demo/requests/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,12 @@ go_test(
4646
"SUBMITQUEUE_TEST_GIT": "$(location @git//:git)",
4747
},
4848
deps = [
49+
"//api/base/mergestrategy/protopb:go_default_library",
4950
"//platform/base/change/git:go_default_library",
5051
"//platform/fakemarker:go_default_library",
5152
"//platform/git/exec:go_default_library",
5253
"//platform/git/exectest:go_default_library",
54+
"//submitqueue/client:go_default_library",
5355
"@com_github_stretchr_testify//assert:go_default_library",
5456
"@com_github_stretchr_testify//require:go_default_library",
5557
],

service/submitqueue/demo/requests/main.go

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ type config struct {
9292
files int
9393
concurrency int
9494
stacked bool
95+
burst bool
9596
prefix string
9697
land bool
9798
watch bool
@@ -120,6 +121,8 @@ func parseFlags() config {
120121
flag.IntVar(&c.concurrency, "concurrency", 5,
121122
"how many changes to create at once; a stack ignores it, being sequential by nature, and -provider git serializes its git commands")
122123
flag.BoolVar(&c.stacked, "stacked", false, "chain the changes and enqueue them as one stack")
124+
flag.BoolVar(&c.burst, "burst", false,
125+
"create every change first, then enqueue them all at once instead of as each is created; independent changes only")
123126
flag.StringVar(&c.prefix, "prefix", "demo", "branch name prefix")
124127
flag.BoolVar(&c.land, "land", true, "enqueue each change as it is created")
125128
flag.BoolVar(&c.watch, "watch", true, "watch the requests until they all settle")
@@ -242,6 +245,9 @@ func shape(cfg config) string {
242245
if cfg.stacked {
243246
return "stacked, enqueued as one request once the chain exists"
244247
}
248+
if cfg.burst && cfg.land {
249+
return fmt.Sprintf("independent, created %d at a time, then all enqueued at once", cfg.concurrency)
250+
}
245251
if cfg.concurrency > 1 {
246252
return fmt.Sprintf("independent, %d at a time, each enqueued as soon as it is created", cfg.concurrency)
247253
}
@@ -432,6 +438,12 @@ func changeFileCount(tag string, change, min int) int {
432438
return min + int(sum[0]%4)
433439
}
434440

441+
// lander enqueues a request. *client.Client is the real one; a test supplies
442+
// its own to observe when each change is enqueued relative to when it is created.
443+
type lander interface {
444+
Land(ctx context.Context, queue string, uris []string, strategy mergestrategypb.Strategy) (string, error)
445+
}
446+
435447
// createAndEnqueue creates the changes and puts them on the queue, filling
436448
// in the tracker's rows as it goes and reporting each step beneath the table.
437449
//
@@ -451,7 +463,7 @@ func changeFileCount(tag string, change, min int) int {
451463
func createAndEnqueue(
452464
ctx context.Context,
453465
src changeSource,
454-
sq *client.Client,
466+
sq lander,
455467
cfg config,
456468
strategy mergestrategypb.Strategy,
457469
tag, baseSHA string,
@@ -480,12 +492,16 @@ func createAndEnqueue(
480492
func createIndependent(
481493
ctx context.Context,
482494
src changeSource,
483-
sq *client.Client,
495+
sq lander,
484496
cfg config,
485497
strategy mergestrategypb.Strategy,
486498
tag, baseSHA string,
487499
t *client.Tracker,
488500
) ([]change, error) {
501+
if cfg.burst && cfg.land {
502+
return createBurst(ctx, src, sq, cfg, strategy, tag, baseSHA, t)
503+
}
504+
489505
rows := t.Rows()
490506
// Indexed rather than appended: the workers finish in whatever order the
491507
// provider answers them, and the caller still wants the run's own order.
@@ -521,6 +537,62 @@ func createIndependent(
521537
return created, nil
522538
}
523539

540+
// createBurst creates every change first and only then enqueues them, firing
541+
// all the Land calls together so the requests arrive at the queue in one burst.
542+
//
543+
// The default path lands each change the moment it exists, so the queue starts
544+
// working while later changes are still being created. Burst trades that early
545+
// overlap for a simultaneous arrival: useful for watching the queue admit a
546+
// large batch at once. It does not make creation faster — with -provider git the
547+
// creation phase is still serialized on a single work tree — it only separates
548+
// creation from enqueuing so the enqueues are not spread across it.
549+
func createBurst(
550+
ctx context.Context,
551+
src changeSource,
552+
sq lander,
553+
cfg config,
554+
strategy mergestrategypb.Strategy,
555+
tag, baseSHA string,
556+
t *client.Tracker,
557+
) ([]change, error) {
558+
rows := t.Rows()
559+
created := make([]change, cfg.count)
560+
561+
create, createCtx := errgroup.WithContext(ctx)
562+
create.SetLimit(cfg.concurrency)
563+
for i := 1; i <= cfg.count; i++ {
564+
create.Go(func() error {
565+
c, err := createOne(createCtx, src, cfg, tag, baseSHA, cfg.base, i, t, rows[i-1])
566+
if err != nil {
567+
return err
568+
}
569+
created[i-1] = c
570+
return nil
571+
})
572+
}
573+
if err := create.Wait(); err != nil {
574+
return nil, err
575+
}
576+
577+
t.Note("enqueuing %d changes at once", cfg.count)
578+
land, landCtx := errgroup.WithContext(ctx)
579+
land.SetLimit(cfg.concurrency)
580+
for i := 1; i <= cfg.count; i++ {
581+
land.Go(func() error {
582+
sqid, err := sq.Land(landCtx, cfg.queue, urisOf([]change{created[i-1]}), strategy)
583+
if err != nil {
584+
return err
585+
}
586+
t.Update(func() { rows[i-1].SQID, rows[i-1].Submitted = sqid, time.Now() })
587+
return nil
588+
})
589+
}
590+
if err := land.Wait(); err != nil {
591+
return nil, err
592+
}
593+
return created, nil
594+
}
595+
524596
// createStack creates the changes one after another, each based on the one
525597
// before it, and submits the whole chain as a single request.
526598
//
@@ -530,7 +602,7 @@ func createIndependent(
530602
func createStack(
531603
ctx context.Context,
532604
src changeSource,
533-
sq *client.Client,
605+
sq lander,
534606
cfg config,
535607
strategy mergestrategypb.Strategy,
536608
tag, baseSHA string,

service/submitqueue/demo/requests/main_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,18 @@
1515
package main
1616

1717
import (
18+
"context"
1819
"fmt"
1920
"strings"
21+
"sync"
22+
"sync/atomic"
2023
"testing"
2124

2225
"github.com/stretchr/testify/assert"
2326
"github.com/stretchr/testify/require"
27+
28+
mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb"
29+
"github.com/uber/submitqueue/submitqueue/client"
2430
)
2531

2632
func TestChangeFilePath_IsUniquePerFileAcrossChangesAndRuns(t *testing.T) {
@@ -206,4 +212,62 @@ func TestShapeReportsConcurrency(t *testing.T) {
206212
"one at a time is just sequential; saying so adds nothing")
207213
assert.Contains(t, shape(config{count: 10, concurrency: 5, stacked: true}), "stacked",
208214
"a stack is sequential whatever the limit says")
215+
assert.Contains(t, shape(config{count: 10, concurrency: 5, land: true, burst: true}), "all enqueued at once")
216+
assert.NotContains(t, shape(config{count: 10, concurrency: 5, burst: true, land: false}), "at once",
217+
"burst is a landing decision; with nothing to land there is no burst")
218+
}
219+
220+
// recordingSource counts how many changes have been created, so a test can
221+
// observe creation relative to enqueuing. It creates through fakeSource, so it
222+
// stands in for any provider — the two-phase orchestration under test is the
223+
// same whichever one is wired.
224+
type recordingSource struct {
225+
created *int64
226+
}
227+
228+
func (recordingSource) baseSHA(context.Context, string) (string, error) { return "base", nil }
229+
230+
func (r recordingSource) open(ctx context.Context, spec changeSpec) (openedChange, error) {
231+
atomic.AddInt64(r.created, 1)
232+
return fakeSource{}.open(ctx, spec)
233+
}
234+
235+
// landerFunc adapts a function to the lander interface.
236+
type landerFunc func(context.Context, string, []string, mergestrategypb.Strategy) (string, error)
237+
238+
func (f landerFunc) Land(ctx context.Context, queue string, uris []string, s mergestrategypb.Strategy) (string, error) {
239+
return f(ctx, queue, uris, s)
240+
}
241+
242+
// TestCreateBurst_EnqueuesOnlyAfterEveryChangeIsCreated is the property -burst
243+
// exists for: no request reaches the queue until the last change has been
244+
// created, so they all arrive together. It is provider-independent — burst runs
245+
// every source through the same two phases.
246+
func TestCreateBurst_EnqueuesOnlyAfterEveryChangeIsCreated(t *testing.T) {
247+
const count = 8
248+
cfg := config{count: count, concurrency: 4, files: 3, folders: 3, land: true, burst: true, queue: "q", prefix: "demo"}
249+
250+
var created int64
251+
src := recordingSource{created: &created}
252+
253+
var mu sync.Mutex
254+
createdWhenLanded := make([]int64, 0, count)
255+
lander := landerFunc(func(context.Context, string, []string, mergestrategypb.Strategy) (string, error) {
256+
mu.Lock()
257+
createdWhenLanded = append(createdWhenLanded, atomic.LoadInt64(&created))
258+
mu.Unlock()
259+
return "sqid", nil
260+
})
261+
262+
tracker := client.NewTracker(client.NewRows(count))
263+
got, err := createBurst(context.Background(), src, lander, cfg,
264+
mergestrategypb.Strategy_SQUASH_REBASE, "run", "base", tracker)
265+
require.NoError(t, err)
266+
require.Len(t, got, count)
267+
268+
require.Len(t, createdWhenLanded, count, "every change is enqueued exactly once")
269+
for _, seen := range createdWhenLanded {
270+
assert.Equal(t, int64(count), seen,
271+
"burst enqueues nothing until every change has been created")
272+
}
209273
}

0 commit comments

Comments
 (0)