Commit 9c31ca2462
Verified · cmc
Layout: unified · split
.gitbay/wiki/Admin.org +3 −1
| @@ -218,7 +218,9 @@ push=. | |||
| 218 | at most =pack_concurrency= − 1 slots when it is above 1, so a signed-in | 218 | at most =pack_concurrency= − 1 slots when it is above 1, so a signed-in |
| 219 | client (SSH key, bearer token or web session) can always get the last. | 219 | client (SSH key, bearer token or web session) can always get the last. |
| 220 | Past that an SSH client gets "the server is busy…" and exit 1, HTTP | 220 | Past that an SSH client gets "the server is busy…" and exit 1, HTTP |
| 221 | gets 503 with =Retry-After: 30=, git:// an =ERR= line. An SSH client | 221 | gets 503 with =Retry-After: 30=, git:// an =ERR= line; the daemon |
| 222 | logs a =pack limit= warning naming the transport and whether the | ||
| 223 | client was signed in, at most once a minute per transport. An SSH client | ||
| 222 | that disconnects while queued leaves the queue; an HTTP or git:// one | 224 | that disconnects while queued leaves the queue; an HTTP or git:// one |
| 223 | keeps its place until the wait runs out. A running clone is killed | 225 | keeps its place until the wait runs out. A running clone is killed |
| 224 | when its client disconnects, or when no write to the client completes | 226 | when its client disconnects, or when no write to the client completes |
CHANGELOG.org +3
| @@ -240,6 +240,9 @@ missing, =gitbayd admin backup --verify <archive>= names it, and | |||
| 240 | - Anonymous clones are counted per IPv4 address or IPv6 /64, and | 240 | - Anonymous clones are counted per IPv4 address or IPv6 /64, and |
| 241 | together hold at most =pack_concurrency= − 1 slots, so a signed-in | 241 | together hold at most =pack_concurrency= − 1 slots, so a signed-in |
| 242 | client can always get the last one (#262). | 242 | client can always get the last one (#262). |
| 243 | - A request turned away by the pack limit logs a warning naming the | ||
| 244 | transport and whether the client was signed in, never its address, | ||
| 245 | at most once a minute per transport (#262). | ||
| 243 | - Web archive downloads (=/{owner}/{repo}/archive/{ref}.tar.gz=) take | 246 | - Web archive downloads (=/{owner}/{repo}/archive/{ref}.tar.gz=) take |
| 244 | a pack slot, answer 503 with =Retry-After: 30= when none is free, and | 247 | a pack slot, answer 503 with =Retry-After: 30= when none is free, and |
| 245 | are killed when the client leaves or stops reading (#262). | 248 | are killed when the client leaves or stops reading (#262). |
internal/gitd/gitd.go +3 −1
| @@ -72,8 +72,10 @@ func (s *Server) handle(conn net.Conn) { | |||
| 72 | 72 | ||
| 73 | // A nil done: a queued client that leaves, or a restart, does not | 73 | // A nil done: a queued client that leaves, or a restart, does not |
| 74 | // end the wait; only the limiter's wait does. | 74 | // end the wait; only the limiter's wait does. |
| 75 | release, err := s.packs.Acquire(nil, principal(conn.RemoteAddr())) | 75 | p := principal(conn.RemoteAddr()) |
| 76 | release, err := s.packs.Acquire(nil, p) | ||
| 76 | if err != nil { | 77 | if err != nil { |
| 78 | s.packs.Refused("git", p, err) | ||
| 77 | writeErr(conn, err.Error()) | 79 | writeErr(conn, err.Error()) |
| 78 | return | 80 | return |
| 79 | } | 81 | } |
internal/httpd/smart.go +3 −1
| @@ -174,8 +174,10 @@ func (s *Server) uploadPack(w http.ResponseWriter, r *http.Request) { | |||
| 174 | // to it completed for packlimit.StallDeadline — and finish, called once | 174 | // to it completed for packlimit.StallDeadline — and finish, called once |
| 175 | // git has exited, releases the slot. | 175 | // git has exited, releases the slot. |
| 176 | func (s *Server) packSlot(w http.ResponseWriter, r *http.Request) (out io.Writer, kill <-chan struct{}, finish func(), ok bool) { | 176 | func (s *Server) packSlot(w http.ResponseWriter, r *http.Request) (out io.Writer, kill <-chan struct{}, finish func(), ok bool) { |
| 177 | release, err := s.packs.Acquire(s.until(r), s.packPrincipal(r)) | 177 | principal := s.packPrincipal(r) |
| 178 | release, err := s.packs.Acquire(s.until(r), principal) | ||
| 178 | if err != nil { | 179 | if err != nil { |
| 180 | s.packs.Refused("http", principal, err) | ||
| 179 | msg := "the server is restarting; try again in a minute" | 181 | msg := "the server is restarting; try again in a minute" |
| 180 | if errors.Is(err, packlimit.ErrBusy) { | 182 | if errors.Is(err, packlimit.ErrBusy) { |
| 181 | msg = "the server is busy: it is at its limit of concurrent clones and fetches; try again in a minute" | 183 | msg = "the server is busy: it is at its limit of concurrent clones and fetches; try again in a minute" |
internal/packlimit/packlimit.go +32 −4
| @@ -9,6 +9,7 @@ package packlimit | |||
| 9 | 9 | ||
| 10 | import ( | 10 | import ( |
| 11 | "errors" | 11 | "errors" |
| 12 | "log/slog" | ||
| 12 | "net/netip" | 13 | "net/netip" |
| 13 | "strings" | 14 | "strings" |
| 14 | "sync" | 15 | "sync" |
| @@ -33,9 +34,10 @@ type Limiter struct { | |||
| 33 | running int | 34 | running int |
| 34 | classHeld int | 35 | classHeld int |
| 35 | queued int | 36 | queued int |
| 36 | held map[string]int // running, per principal | 37 | held map[string]int // running, per principal |
| 37 | waiting map[string]int // queued, per principal | 38 | waiting map[string]int // queued, per principal |
| 38 | changed chan struct{} // closed and replaced on every release | 39 | changed chan struct{} // closed and replaced on every release |
| 40 | warned map[string]time.Time // last refusal logged, per transport | ||
| 39 | } | 41 | } |
| 40 | 42 | ||
| 41 | // New returns a limiter, or nil — no limit — when max is not positive. | 43 | // New returns a limiter, or nil — no limit — when max is not positive. |
| @@ -44,7 +46,33 @@ func New(max, per, queue int, wait time.Duration) *Limiter { | |||
| 44 | return nil | 46 | return nil |
| 45 | } | 47 | } |
| 46 | return &Limiter{max: max, per: per, queue: queue, wait: wait, | 48 | return &Limiter{max: max, per: per, queue: queue, wait: wait, |
| 47 | held: map[string]int{}, waiting: map[string]int{}, changed: make(chan struct{})} | 49 | held: map[string]int{}, waiting: map[string]int{}, changed: make(chan struct{}), |
| 50 | warned: map[string]time.Time{}} | ||
| 51 | } | ||
| 52 | |||
| 53 | // Refused logs that a request on transport was turned away with err, at | ||
| 54 | // most once a minute per transport. It names the principal's class | ||
| 55 | // (user or ip), never the principal: an address is personal data. | ||
| 56 | func (l *Limiter) Refused(transport, principal string, err error) { | ||
| 57 | if l == nil { | ||
| 58 | return | ||
| 59 | } | ||
| 60 | now := time.Now() | ||
| 61 | l.mu.Lock() | ||
| 62 | last, seen := l.warned[transport] | ||
| 63 | if seen && now.Sub(last) < time.Minute { | ||
| 64 | l.mu.Unlock() | ||
| 65 | return | ||
| 66 | } | ||
| 67 | l.warned[transport] = now | ||
| 68 | l.mu.Unlock() | ||
| 69 | class, _, _ := strings.Cut(principal, ":") | ||
| 70 | reason := "busy" | ||
| 71 | if errors.Is(err, ErrGone) { | ||
| 72 | reason = "gone" | ||
| 73 | } | ||
| 74 | slog.Warn("pack limit: request turned away (logged at most once a minute per transport)", | ||
| 75 | "transport", transport, "class", class, "reason", reason) | ||
| 48 | } | 76 | } |
| 49 | 77 | ||
| 50 | // CapClass caps the slots that principals starting with prefix may hold | 78 | // CapClass caps the slots that principals starting with prefix may hold |
internal/packlimit/packlimit_test.go +31
| @@ -1,8 +1,11 @@ | |||
| 1 | package packlimit | 1 | package packlimit |
| 2 | 2 | ||
| 3 | import ( | 3 | import ( |
| 4 | "bytes" | ||
| 4 | "errors" | 5 | "errors" |
| 6 | "log/slog" | ||
| 5 | "math" | 7 | "math" |
| 8 | "strings" | ||
| 6 | "testing" | 9 | "testing" |
| 7 | "time" | 10 | "time" |
| 8 | ) | 11 | ) |
| @@ -282,3 +285,31 @@ func TestAddrPrincipal(t *testing.T) { | |||
| 282 | } | 285 | } |
| 283 | } | 286 | } |
| 284 | } | 287 | } |
| 288 | |||
| 289 | // A refusal is logged once a minute per transport, with the principal's | ||
| 290 | // class and never its address. | ||
| 291 | func TestRefusedLogsOncePerTransport(t *testing.T) { | ||
| 292 | var buf bytes.Buffer | ||
| 293 | old := slog.Default() | ||
| 294 | slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil))) | ||
| 295 | t.Cleanup(func() { slog.SetDefault(old) }) | ||
| 296 | |||
| 297 | l := New(1, 0, 0, time.Second) | ||
| 298 | l.Refused("http", "ip:192.0.2.7", ErrBusy) | ||
| 299 | l.Refused("http", "ip:192.0.2.8", ErrBusy) | ||
| 300 | l.Refused("ssh", "user:4", ErrGone) | ||
| 301 | out := buf.String() | ||
| 302 | if n := strings.Count(out, "\n"); n != 2 { | ||
| 303 | t.Fatalf("%d lines, want 2:\n%s", n, out) | ||
| 304 | } | ||
| 305 | for _, want := range []string{"transport=http class=ip reason=busy", "transport=ssh class=user reason=gone"} { | ||
| 306 | if !strings.Contains(out, want) { | ||
| 307 | t.Errorf("missing %q in:\n%s", want, out) | ||
| 308 | } | ||
| 309 | } | ||
| 310 | if strings.Contains(out, "192.0.2") || strings.Contains(out, "user:4") { | ||
| 311 | t.Fatalf("principal logged:\n%s", out) | ||
| 312 | } | ||
| 313 | var none *Limiter | ||
| 314 | none.Refused("git", "ip:x", ErrBusy) | ||
| 315 | } | ||
internal/sshd/sshd.go +5 −1
| @@ -608,7 +608,11 @@ func runGit(cfg config.Config, st *store.Store, packs *packlimit.Limiter, user s | |||
| 608 | // Pack generation shares one budget with smart HTTP and git://. | 608 | // Pack generation shares one budget with smart HTTP and git://. |
| 609 | // receive-pack stays outside it: its post-receive runs after the | 609 | // receive-pack stays outside it: its post-receive runs after the |
| 610 | // client has its report, and must not be queued or killed. | 610 | // client has its report, and must not be queued or killed. |
| 611 | release, err := packs.Acquire(done, "user:"+strconv.FormatInt(user.ID, 10)) | 611 | principal := "user:" + strconv.FormatInt(user.ID, 10) |
| 612 | release, err := packs.Acquire(done, principal) | ||
| 613 | if err != nil { | ||
| 614 | packs.Refused("ssh", principal, err) | ||
| 615 | } | ||
| 612 | if errors.Is(err, packlimit.ErrBusy) { | 616 | if errors.Is(err, packlimit.ErrBusy) { |
| 613 | fmt.Fprintln(stderr, "the server is busy: it is at its limit of concurrent clones and fetches; try again in a minute") | 617 | fmt.Fprintln(stderr, "the server is busy: it is at its limit of concurrent clones and fetches; try again in a minute") |
| 614 | return protocol.ExitFailure | 618 | return protocol.ExitFailure |