Merge pull request #75 from entireio/fix/target-push-timeout-batched-retry · Entire
Merge pull request #75 from entireio/fix/target-push-timeout-batched-retry
171d5e9→main·Soph·1mo ago·2 files·+182 added/-12 removed
bootstrap: treat receive-pack timeouts (408/504) as batchable
Changes
2
internal/strategy/bootstrap
Mbootstrap.go+66/-12
Mbootstrap_test.go+116
178 unmodified lines
179
180
181
182
182
183
184
185
186
187
184
185
186
187
188
189
190
191
192
193
194
307 unmodified lines
502
503
504
501
502
503
504
505
506
507
508
509
510
511
512
513
5 unmodified lines
519
520
521
516
522
523
518
524
525
526
527
856 unmodified lines
1384
1385
1386
1381
1387
1388
1389
1390
53 unmodified lines
1444
1445
1446
1447
1448
1449
1450
1451
1452
1453
1454
1455
1456
1457
1458
1459
1460
1461
1462
1463
1464
1465
1466
1467
1468
1469
1470
1471
1472
1473
1474
1475
1476
1477
1478
1479
1480
1481
1482
1483
1484
1485
1486
1487
1488
1489
1490
1491
1492
1493
1494
1495
1496
1497
178 unmodified lines
if pushErr != nil {
autoBatch, ok := autoTargetMaxPackBytes(p, pushErr)
if !ok {
return result, fmt.Errorf("push target refs: %w", pushErr)
return result, fmt.Errorf("push target refs: %w", actionableTargetPushError(p, pushErr))
}
p.log("bootstrap retrying with batched mode after target rejection",
"target_max_pack_bytes", autoBatch)
p.notice(fmt.Sprintf("target rejected pack — switching to batched mode (limit %%s)",
humanBytes(autoBatch)))
reason := "target rejected pack"
if isTargetPushDeadlineError(pushErr) {
reason = "target push timed out"
}
p.log("bootstrap retrying with batched mode after batchable push failure",
"target_max_pack_bytes", autoBatch, "reason", reason)
p.notice(fmt.Sprintf("%%s — switching to batched mode (limit %%s)",
reason, humanBytes(autoBatch)))
p.TargetMaxPack = autoBatch
return executeBatched(ctx, p, plans, result)
}
307 unmodified lines
abortedEarly := observer.Aborted()
if pushErr != nil {
_ = packReader.Close()
// Treat abortedEarly the same as a body-limit error:
// both indicate "this pack is too big for the target",
// just one is detected by the server and one by us.
sizeIssue := abortedEarly || isTargetBodyLimitError(pushErr)
// A pack too big for the target is the unifying signal here,
// whether the server announced it (413 body limit), we
// detected it ourselves (abortedEarly), or the target ran out
// of time receiving it (408/504 deadline). All three are fixed
// the same way: subdivide so each push is smaller and faster.
subdivide := abortedEarly || isBatchableTargetPushError(pushErr)
p.log("bootstrap batch push failed",
"branch", batch.Plan.TargetRef.String(),
"batch", idx+1,
5 unmodified lines
"objects_sent", objectsSent,
"total_objects_in_pack", totalObjects,
"aborted_early", abortedEarly,
"will_subdivide", sizeIssue && len(batch.chain) > 0,
"will_subdivide", subdivide && len(batch.chain) > 0,
"error", pushErr.Error())
if sizeIssue && len(batch.chain) > 0 {
if subdivide && len(batch.chain) > 0 {
parsedLimit := targetBodyLimit(pushErr)
limit := p.TargetMaxPack
if parsedLimit > 0 {
856 unmodified lines
}
func autoTargetMaxPackBytes(p Params, err error) (int64, bool) {
if p.TargetMaxPack > 0 || !isTargetBodyLimitError(err) {
if p.TargetMaxPack > 0 || !isBatchableTargetPushError(err) {
return 0, false
}
if p.SourceService == nil || !p.SourceService.SupportsBootstrapBatch() {
53 unmodified lines
strings.Contains(msg, "http 413")
}
// isTargetPushDeadlineError reports whether err indicates the target cut the
// receive-pack POST short because it ran past a server-side deadline rather
// than because the pack exceeded an announced size limit. GitHub returns 408
// (Request Timeout) when a slow or oversized push outlasts its receive-pack
// wall-clock window — common when relaying a large repo over a slow source
// link, where the upstream read rate throttles the downstream write. Gateways
// fronting other hosts surface the same condition as 504 (Gateway Timeout).
//
// Both are remedied the way a body-limit rejection is: smaller packs each
// finish inside the window, so callers route them into the same batched
// bootstrap retry. Kept distinct from isTargetBodyLimitError because the
// trigger is a timeout, not a size rejection, and there's no body limit to
// parse out of the message.
func isTargetPushDeadlineError(err error) bool {
if err == nil {
return false
}
msg := strings.ToLower(err.Error())
return strings.Contains(msg, "http 408") || strings.Contains(msg, "http 504")
}
// isBatchableTargetPushError reports whether err is a target-side push failure
// that batched bootstrap can work around by sending smaller packs: an explicit
// body-size rejection (413 / "body exceeded size limit") or a receive-pack
// deadline (408 / 504).
func isBatchableTargetPushError(err error) bool {
return isTargetBodyLimitError(err) || isTargetPushDeadlineError(err)
}
// actionableTargetPushError augments a one-shot push failure with guidance
// when the target couldn't receive the pack — too large or too slow — but batched
// bootstrap couldn't take over — which, on the one-shot path, means the source
// can't serve the protocol-v2 fetch filter that checkpointing requires. The
// extra context tells the user why the obvious knob (--target-max-pack-bytes)
// won't help here, instead of leaving a bare "http 408". Returns err unchanged
// for non-batchable failures or when batching is in fact available.
func actionableTargetPushError(p Params, err error) error {
if !isBatchableTargetPushError(err) {
return err
}
if p.SourceService != nil && p.SourceService.SupportsBootstrapBatch() {
return err
}
return fmt.Errorf("%w (target could not receive the pack — too large, or too slow to "+
"receive within its deadline; batched bootstrap could split it into smaller pushes, "+
"but the source does not support the protocol-v2 fetch filter batched bootstrap requires)", err)
}
func targetBodyLimit(err error) int64 {
if err == nil {
return 0
}
func TestIsTargetPushDeadlineError(t *testing.T) {
tests := []struct {
name string
err error
want bool
}{
{name: "nil error", err: nil, want: false},
{
name: "github receive-pack 408",
err: errors.New("push target refs: target receive-pack: post RPC stream body: http 408: https://github.com/o/r.git/git-receive-pack"),
want: true,
},
{
name: "gateway 504",
err: errors.New("target receive-pack: http 504: gateway timeout"),
want: true,
},
{
name: "body limit is not a deadline",
err: errors.New("body exceeded size limit 1048576"),
want: false,
},
{
name: "413 is not a deadline",
err: errors.New("http 413: payload too large"),
want: false,
},
{
name: "unrelated error",
err: errors.New("connection refused"),
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isTargetPushDeadlineError(tt.err); got != tt.want {
t.Errorf("isTargetPushDeadlineError(%%v) = %%v, want %%v", tt.err, got, tt.want)
}
})
}
}
func TestIsBatchableTargetPushError(t *testing.T) {
// Per-status edge cases are covered by TestIsTargetBodyLimitError and
// TestIsTargetPushDeadlineError; this only confirms the OR wires both in.
tests := []struct {
name string
err error
want bool
}{
{name: "body limit", err: errors.New("body exceeded size limit 1048576"), want: true},
{name: "deadline", err: errors.New("http 408: request timeout"), want: true},
{name: "unrelated", err: errors.New("connection refused"), want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isBatchableTargetPushError(tt.err); got != tt.want {
t.Errorf("isBatchableTargetPushError(%%v) = %%v, want %%v", tt.err, got, tt.want)
}
})
}
}
func TestTargetBodyLimit(t *testing.T) {
tests := []struct {
name string
err error
}
}
// noBatchSource is a source that can't serve the protocol-v2 fetch filter
// batched bootstrap needs, so a one-shot push failure has no batched fallback.
type noBatchSource struct{ fakeBootstrapSource }
func (noBatchSource) SupportsBootstrapBatch() bool { return false }
func TestAutoTargetMaxPackBytesTimeoutTriggersBatching(t *testing.T) {
limit, ok := autoTargetMaxPackBytes(
Params{SourceService: fakeBootstrapSource{}},
errorors.New("target receive-pack: http 408: request timeout"),
)
if !ok {
t.Fatal("autoTargetMaxPackBytes(408) = not ok, want batched fallback")
}
if limit != defaultTargetMaxPackBytes {
t.Fatalf("limit = %%d, want default %%d", limit, int64(defaultTargetMaxPackBytes))
}
}
func TestExecuteOneShotTimeoutWithoutBatchSupportIsActionable(t *testing.T) {
mainRef := plumbing.NewBranchReferenceName("main")
mainHash := plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
pushErr := errors.New("target receive-pack: post RPC stream body: http 408: request timeout")
_, err := Execute(context.Background(), Params{
SourceService: noBatchSource{fakeBootstrapSource{
fetchPack: func(_ context.Context, _ gitproto.Conn, _ map[plumbing.ReferenceName]gitproto.DesiredRef, _ map[plumbing.ReferenceName]plumbing.Hash) (io.ReadCloser, error) {
return io.NopCloser(bytes.NewReader([]byte("PACK"))), nil
},
}},
TargetPusher: fakeBootstrapPusher{
pushPack: func(_ context.Context, _ []gitproto.PushCommand, pack io.ReadCloser) error {
_ = pack.Close()
return pushErr
},
},
DesiredRefs: map[plumbing.ReferenceName]planner.DesiredRef{
mainRef: {SourceRef: mainRef, TargetRef: mainRef, SourceHash: mainHash, Kind: planner.RefKindBranch},
},
}, "empty target")
if err == nil {
t.Fatal("Execute() error = nil, want actionable timeout error")
}
if !errors.Is(err, pushErr) {
t.Fatalf("Execute() error does not wrap original push error: %%v", err)
}
if !strings.Contains(err.Error(), "protocol-v2 fetch filter") {
t.Fatalf("Execute() error missing batched-bootstrap guidance: %%v", err)
}
}
func TestExecuteBatchedClosesCheckpointPackOnPushError(t *testing.T) {
mainRef := plumbing.NewBranchReferenceName("main")
hashes := makeLinearCommitChain(t, 1)