fix(submit): 分块上传的预算检查复用一分钟内读过的 blob 大小，存储读不到时回 503 让面板自动重试 (d2730c6e) · Commits · FelisMC / Felis · GitLab

docs/openapi.yaml

+8 −2

Changes for docs/openapi.yaml: 8 added lines, 2 removed lines.
Original line number	Diff line number	Diff line
		@@ -5884,7 +5884,11 @@ paths:
		the same context cap (400) and storage budget (403) as a single upload.
		A part that breaks off is cut back off, so the staged bytes are always a
		prefix of the file. One request per upload at a time (409 upload_busy).
		Staged bytes untouched for 24 hours are deleted.
		Staged bytes untouched for 24 hours are deleted. The budget check reads
		blob sizes remembered for up to a minute; when a size has to be read and
		the uploads store does not answer, the answer is 503
		uploads_store_unavailable with Retry-After, and the same part can be
		sent again.
		x-felis-face: [external]
		x-felis-tier: app
		security: [{ sessionCookie: [] }]
		@@ -5938,7 +5942,9 @@ paths:
		way, and deletes the staged copy. Holds the same per-user upload
		cooldown (429) and writes the same submission.upload audit event.
		Nothing staged is 400. After a failure the staged bytes stay, for a
		retry.
		retry. The budget is checked against every blob's size read from the
		uploads store; a store that does not answer is 503
		uploads_store_unavailable with Retry-After.
		x-felis-face: [external]
		x-felis-tier: app
		security: [{ sessionCookie: [] }]

internal/api/api_test.go

+1 −0

Changes for internal/api/api_test.go: 1 added line, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -824,6 +824,7 @@ func (f *fakeRepo) SetAllowlistWake(_ context.Context, n, uuid string, canWake b
		}
		return ErrNotFound
		}

		// RequestRetire and CancelRetire mirror PGRepo's: the first request time is
		// kept, a deletion stays a deletion, and only an admin cancels a deletion.
		func (f *fakeRepo) RequestRetire(_ context.Context, n string, del bool) (RetireState, error) {

internal/api/submissions.go

+14 −1

Changes for internal/api/submissions.go: 14 added lines, 1 removed line.
Original line number	Diff line number	Diff line
		@@ -9,6 +9,7 @@ import (
		"log"
		"net/http"
		"strconv"
		"time"

		"felis.lolicon.best/internal/submit"
		)
		@@ -502,6 +503,9 @@ func (a API) handleDeleteSubmission(w http.ResponseWriter, r http.Request) {
		// errSubmissionsUnavailable is returned when the approval lane is not configured
		// on this api instance (a nil Submissions service), so the admin/app boundary is
		// still exercised before the subsystem is wired in.
		// uploadsStoreRetry is the Retry-After on uploads_store_unavailable.
		const uploadsStoreRetry = 5 * time.Second

		var errSubmissionsUnavailable = newError(http.StatusServiceUnavailable, "submissions_unavailable",
		"modpack submission subsystem is not configured")

		@@ -511,7 +515,10 @@ var errSubmissionsUnavailable = newError(http.StatusServiceUnavailable, "submiss
		// allowance is 403 (the same status the server-resource quota answers with), a
		// full uploads store (every user's uploads together at their cap, or the volume
		// short of free space) is 507, and an unconfigured upload transport is 503 (the store this deployment set has no
		// implemented transport — an honest "not available here", not a client error). Everything
		// implemented transport — an honest "not available here", not a client error). A
		// blob store that did not answer the budget check is 503 uploads_store_unavailable
		// with Retry-After: nothing was written and the same request can be sent again.
		// Everything
		// else — including a build.ErrInvalid raised by the pre-CAS build.Validate (a
		// platform registry/context MISCONFIGURATION, never client input, since every
		// build input is platform-derived) and a post-CAS Submit hand-off failure — is a
		@@ -541,6 +548,12 @@ func writeSubmitError(w http.ResponseWriter, r *http.Request, err error) {
		case errors.Is(err, submit.ErrUploadsUnavailable):
		writeError(w, r, newError(http.StatusServiceUnavailable, "uploads_unavailable",
		"modpack upload transport is not configured"))
		case errors.Is(err, submit.ErrStoreUnavailable):
		// The budget could not be checked; nothing was written. The panel's upload
		// loop sends the same request again.
		log.Printf("api: %s %s: %v", r.Method, r.URL.Path, err)
		writeError(w, r, newError(http.StatusServiceUnavailable, "uploads_store_unavailable",
		"the uploads store did not answer; send the request again").retryAfter(uploadsStoreRetry))
		case errors.Is(err, submit.ErrUploadBusy):
		writeError(w, r, newError(http.StatusConflict, "upload_busy",
		"another request is still writing this upload; read where it stands and continue from there"))

internal/api/submissions_chunked_test.go

+12 −5

Changes for internal/api/submissions_chunked_test.go: 12 added lines, 5 removed lines.
Original line number	Diff line number	Diff line
		@@ -60,12 +60,16 @@ func TestContextUploadErrors(t *testing.T) {
		code int
		want string
		msg string
		// retry is the Retry-After the answer must carry, if any.
		retry string
		}{
		{"offset mismatch", &submit.OffsetMismatchError{Received: 12}, 409, "upload_offset_mismatch", "the upload holds 12 bytes"},
		{"busy", submit.ErrUploadBusy, 409, "upload_busy", ""},
		{"part too large", fmt.Errorf("submit: write upload part: %w", submit.ErrPartTooLarge), 413, "part_too_large", ""},
		{"not owned", submit.ErrNotFound, 404, "not_found", ""},
		{"no part store", submit.ErrUploadsUnavailable, 503, "uploads_unavailable", ""},
		{"offset mismatch", &submit.OffsetMismatchError{Received: 12}, 409, "upload_offset_mismatch", "the upload holds 12 bytes", ""},
		{"busy", submit.ErrUploadBusy, 409, "upload_busy", "", ""},
		{"part too large", fmt.Errorf("submit: write upload part: %w", submit.ErrPartTooLarge), 413, "part_too_large", "", ""},
		{"not owned", submit.ErrNotFound, 404, "not_found", "", ""},
		{"no part store", submit.ErrUploadsUnavailable, 503, "uploads_unavailable", "", ""},
		{"store did not answer", fmt.Errorf("%w: size of the context of sub-3: dial tcp: i/o timeout", submit.ErrStoreUnavailable),
		503, "uploads_store_unavailable", "send the request again", "5"},
		} {
		fs := &fakeSubmissions{chunkErr: tc.err}
		w := do(appSubAPI(fs).ExternalHandler(), "PUT", "/api/v1/me/submissions/sub-9/context/upload?offset=12", "abcd", nil)
		@@ -75,6 +79,9 @@ func TestContextUploadErrors(t *testing.T) {
		if tc.msg != "" && !strings.Contains(w.Body.String(), tc.msg) {
		t.Errorf("%s: body %s does not say %q", tc.name, w.Body.String(), tc.msg)
		}
		if got := w.Header().Get("Retry-After"); got != tc.retry {
		t.Errorf("%s: Retry-After = %q, want %q", tc.name, got, tc.retry)
		}
		}
		}

internal/submit/sizes_test.go

0 → 100644

+175 −0

Changes for internal/submit/sizes_test.go: 175 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		package submit

		import (
		"context"
		"errors"
		"io"
		"os"
		"path/filepath"
		"strings"
		"testing"
		"time"
		)

		// countingBlobs counts the Size reads per id.
		type countingBlobs struct {
		*fakeBlobs
		sizes map[string]int
		}

		func (c *countingBlobs) Size(ctx context.Context, id string) (int64, bool, error) {
		c.sizes[id]++
		return c.fakeBlobs.Size(ctx, id)
		}

		// putThenFail stores the bytes and still fails, like an object-store upload that
		// completed but whose answer was lost.
		type putThenFail struct{ *fakeBlobs }

		func (p putThenFail) Put(ctx context.Context, id string, r io.Reader) (int64, error) {
		if _, err := p.fakeBlobs.Put(ctx, id, r); err != nil {
		return 0, err
		}
		return 0, errors.New("put: connection reset")
		}

		// The budget check ahead of each part reads every other blob's size once per
		// blobSizeTTL; the one on completion reads them all from the store.
		func TestPartBudgetRemembersBlobSizes(t *testing.T) {
		m, _, fb, a := newChunkedManager(t)
		ctx := context.Background()
		clock := testNow
		m.Now = func() time.Time { return clock }
		b, err := m.Create(ctx, CreateRequest{DisplayName: "B", SubmittedBy: "user-2"})
		if err != nil {
		t.Fatal(err)
		}
		fb.stored[b.ID] = []byte("xyz")
		cb := &countingBlobs{fakeBlobs: fb, sizes: map[string]int{}}
		m.Blobs = cb

		sendPart(t, m, a, 0, "\x1f\x8b\x08\x00")
		sendPart(t, m, a, 4, "abcd")
		if n := cb.sizes[b.ID]; n != 1 {
		t.Fatalf("two parts read the other blob's size %d times, want once", n)
		}
		clock = clock.Add(blobSizeTTL)
		sendPart(t, m, a, 8, "ef")
		if n := cb.sizes[b.ID]; n != 2 {
		t.Fatalf("a part once the TTL ran out: %d reads in all, want 2", n)
		}
		if _, err := m.CompleteUpload(ctx, a, "user-1"); err != nil {
		t.Fatal(err)
		}
		if n := cb.sizes[b.ID]; n != 3 {
		t.Fatalf("completion within the TTL: %d reads in all, want 3 (it reads the store)", n)
		}
		}

		// What this Manager stores or deletes counts in the next part's budget at once,
		// without waiting out the TTL.
		func TestPartBudgetSeesThisManagersOwnWritesAtOnce(t *testing.T) {
		m, _, _, a := newChunkedManager(t)
		ctx := context.Background()
		m.MaxStoredBytesPerUser = 10
		m.PartMaxBytes = 16
		b, err := m.Create(ctx, CreateRequest{DisplayName: "B", SubmittedBy: "user-1"})
		if err != nil {
		t.Fatal(err)
		}
		// The first part remembers B as holding nothing.
		sendPart(t, m, a, 0, "\x1f\x8b")
		if _, err := m.UploadContext(ctx, b.ID, "user-1", strings.NewReader("\x1f\x8b\x08\x00ab")); err != nil {
		t.Fatal(err)
		}
		if _, err := m.UploadPart(ctx, a, "user-1", 2, strings.NewReader("cde")); !errors.Is(err, ErrQuotaExceeded) {
		t.Fatalf("5 bytes staged beside B's 6 under a 10-byte budget = %v, want ErrQuotaExceeded", err)
		}
		if _, err := m.Withdraw(ctx, b.ID, "user-1"); err != nil {
		t.Fatal(err)
		}
		// Its row is gone, so nothing counts it; nor is its size kept, or the
		// remembered sizes would grow with every submission ever deleted.
		if _, kept := m.sizes[b.ID]; kept {
		t.Fatal("a withdrawn submission's blob size is still remembered")
		}
		if got := sendPart(t, m, a, 2, "cdefgh"); got != 8 {
		t.Fatalf("staged %d after B was withdrawn, want 8", got)
		}
		}

		func TestPartBudgetSeesAReapedBlobAtOnce(t *testing.T) {
		m, st, _, a := newChunkedManager(t)
		ctx := context.Background()
		m.MaxStoredBytesPerUser = 10
		m.PartMaxBytes = 16
		b, err := m.Create(ctx, CreateRequest{DisplayName: "B", SubmittedBy: "user-1"})
		if err != nil {
		t.Fatal(err)
		}
		if _, err := m.UploadContext(ctx, b.ID, "user-1", strings.NewReader("\x1f\x8b\x08\x00ab")); err != nil {
		t.Fatal(err)
		}
		reviewed := testNow.Add(-48 * time.Hour)
		st.subs[b.ID].Status = StatusRejected
		st.subs[b.ID].ReviewedAt = &reviewed
		if n, err := m.ReapRejected(ctx, 24*time.Hour); n != 1 \|\| err != nil {
		t.Fatalf("ReapRejected = %d, %v; want 1, nil", n, err)
		}
		if got := sendPart(t, m, a, 0, "\x1f\x8b\x08\x00abcd"); got != 8 {
		t.Fatalf("staged %d after B's blob was reaped, want 8", got)
		}
		}

		// A Put that failed may still have replaced the blob, so its size is read again.
		func TestPartBudgetRereadsABlobAfterAFailedPut(t *testing.T) {
		m, _, fb, a := newChunkedManager(t)
		ctx := context.Background()
		m.MaxStoredBytesPerUser = 10
		m.PartMaxBytes = 16
		b, err := m.Create(ctx, CreateRequest{DisplayName: "B", SubmittedBy: "user-1"})
		if err != nil {
		t.Fatal(err)
		}
		if _, err := m.UploadContext(ctx, b.ID, "user-1", strings.NewReader("\x1f\x8b\x08\x00ab")); err != nil {
		t.Fatal(err)
		}
		m.Blobs = putThenFail{fb}
		if _, err := m.UploadContext(ctx, b.ID, "user-1", strings.NewReader("\x1f\x8b")); err == nil {
		t.Fatal("setup: the failing Put succeeded")
		}
		m.Blobs = fb
		if got := sendPart(t, m, a, 0, "\x1f\x8b\x08\x00abcd"); got != 8 {
		t.Fatalf("staged %d beside B's 2 stored bytes, want 8", got)
		}
		}

		// A size that cannot be read fails the check as ErrStoreUnavailable, which the
		// API answers 503 for the client to send again, never as a bare error (500).
		func TestBudgetReadFailuresAreRetryable(t *testing.T) {
		m, _, fb, a := newChunkedManager(t)
		ctx := context.Background()
		if _, err := m.Create(ctx, CreateRequest{DisplayName: "B", SubmittedBy: "user-2"}); err != nil {
		t.Fatal(err)
		}
		fb.sizeErr = errors.New("dial tcp: i/o timeout")
		if _, err := m.UploadPart(ctx, a, "user-1", 0, strings.NewReader("\x1f\x8b\x08\x00")); !errors.Is(err, ErrStoreUnavailable) {
		t.Fatalf("a part with the blob store down = %v, want ErrStoreUnavailable", err)
		}
		if _, err := m.UploadContext(ctx, a, "user-1", strings.NewReader("\x1f\x8b\x08\x00")); !errors.Is(err, ErrStoreUnavailable) {
		t.Fatalf("an upload with the blob store down = %v, want ErrStoreUnavailable", err)
		}
		if _, ok := fb.stored[a]; ok {
		t.Fatal("an upload whose budget could not be checked was stored")
		}

		fb.sizeErr = nil
		notDir := filepath.Join(t.TempDir(), "parts")
		if err := os.WriteFile(notDir, nil, 0o600); err != nil {
		t.Fatal(err)
		}
		m.Parts = &PartStore{Dir: notDir}
		if _, err := m.UploadContext(ctx, a, "user-1", strings.NewReader("\x1f\x8b\x08\x00")); !errors.Is(err, ErrStoreUnavailable) {
		t.Fatalf("an upload with the staging directory unreadable = %v, want ErrStoreUnavailable", err)
		}
		}