diff --git a/core/services/studio/sandbox/runner/runner.go b/core/services/studio/sandbox/runner/runner.go index 5584687d5e..4c3006056f 100644 --- a/core/services/studio/sandbox/runner/runner.go +++ b/core/services/studio/sandbox/runner/runner.go @@ -116,6 +116,11 @@ type entry struct { trk *tracker gone bool hasBuilt bool + // suspendFails counts the failed suspends since the last call, and + // suspendAfter is the earliest time the sweep tries again. Both are + // guarded by mu. + suspendFails int + suspendAfter time.Time // mounts are the folders of the sandbox's view, to name a path of a // commit as the guest sees it. A resumed sandbox has the view it was // made with. @@ -479,6 +484,7 @@ func (m *Manager) Run(ctx context.Context, r Request) (string, error) { end = m.o.Begin(ctx, r.User, r.Conversation, r.Operation) defer end() // a panic of the tool still ends the call } + e.suspendFails, e.suspendAfter = 0, time.Time{} out, runErr := tool.Run(ctx, r.Args) // The call's changes become one change set, whether or not the command // succeeded: a failing script may still have written files. @@ -496,6 +502,14 @@ func (m *Manager) Run(ctx context.Context, r Request) (string, error) { return out, runErr } +// suspendBackoff is the wait after n failed suspends of one sandbox: the +// sweep runs every minute, and a suspend that fails holds the entry for up to +// a minute, so it doubles from two minutes to half an hour. +func suspendBackoff(n int) time.Duration { + d := time.Minute << min(n, 5) + return min(d, 30*time.Minute) +} + // Sweep puts idle sandboxes to sleep and removes the old ones. It is called // once a minute; tests call it by hand. func (m *Manager) Sweep(ctx context.Context) { @@ -518,14 +532,25 @@ func (m *Manager) Sweep(ctx context.Context) { e.gone = true m.destroy(ctx, e) m.forget(e) - case e.sb != nil && idle >= m.o.IdleAfter: + case e.sb != nil && idle >= m.o.IdleAfter && !now.Before(e.suspendAfter): sctx, cancel := context.WithTimeout(ctx, time.Minute) ref, err := e.sb.Suspend(sctx) cancel() - if err != nil { - xlog.Warn("sandbox: could not suspend an idle sandbox", "error", err) - } else { + switch { + case err == nil: e.sb, e.snap = nil, &ref + e.suspendFails = 0 + case errors.Is(err, sandbox.ErrGone): + // The guest stopped by itself: keeping it would send every + // next call of the chat to a dead sandbox. + xlog.Warn("sandbox: an idle sandbox is gone, dropping it", "user", e.user, "conversation", e.conv, "error", err) + m.destroy(ctx, e) + e.suspendFails = 0 + default: + e.suspendFails++ + e.suspendAfter = now.Add(suspendBackoff(e.suspendFails)) + xlog.Warn("sandbox: could not suspend an idle sandbox, trying again later", "user", e.user, "conversation", e.conv, + "failures", e.suspendFails, "retry_after", e.suspendAfter, "error", err) } } e.mu.Unlock() diff --git a/core/services/studio/sandbox/runner/sweep_test.go b/core/services/studio/sandbox/runner/sweep_test.go new file mode 100644 index 0000000000..3ade3af0df --- /dev/null +++ b/core/services/studio/sandbox/runner/sweep_test.go @@ -0,0 +1,113 @@ +//go:build linux || darwin + +package runner_test + +import ( + "context" + "errors" + "sync" + "time" + + "github.com/mudler/LocalAI/core/services/studio/sandbox" + "github.com/mudler/LocalAI/core/services/studio/sandbox/runner" + "github.com/mudler/LocalAI/core/services/studio/sandbox/sbtest" + "github.com/mudler/LocalAI/core/services/studio/workspace" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// flakyBackend makes sandboxes whose Suspend fails with err, and counts the +// sandboxes it made and the suspends it saw. +type flakyBackend struct { + sandbox.Backend + mu sync.Mutex + err error + created int + suspends int +} + +func (b *flakyBackend) Create(ctx context.Context, spec sandbox.Spec) (sandbox.Sandbox, error) { + sb, err := b.Backend.Create(ctx, spec) + if err != nil { + return nil, err + } + b.mu.Lock() + b.created++ + b.mu.Unlock() + return &flakySandbox{Sandbox: sb, b: b}, nil +} + +func (b *flakyBackend) counts() (created, suspends int) { + b.mu.Lock() + defer b.mu.Unlock() + return b.created, b.suspends +} + +type flakySandbox struct { + sandbox.Sandbox + b *flakyBackend +} + +func (s *flakySandbox) Suspend(context.Context) (sandbox.SnapshotRef, error) { + s.b.mu.Lock() + defer s.b.mu.Unlock() + s.b.suspends++ + return sandbox.SnapshotRef{}, s.b.err +} + +var _ = Describe("the sweep of idle sandboxes", func() { + var ( + rig *scopedRig + fb *flakyBackend + m *runner.Manager + now time.Time + ) + BeforeEach(func() { + rig = newScopedRig(workspace.ModeWrite) + fb = &flakyBackend{Backend: sbtest.NewFakeBackend(GinkgoT().TempDir())} + now = time.Now() + var err error + m, err = runner.New(runner.Options{ + Backend: func() (sandbox.Backend, error) { return fb, nil }, + Store: rig.st, + Actor: rig.actor, + View: rig.view, + IdleAfter: time.Minute, + Now: func() time.Time { return now }, + }) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func() { _ = m.Close(context.Background()) }) + rig.m = m + rig.must("echo one") + now = now.Add(2 * time.Minute) + }) + + It("drops a sandbox whose guest is gone, so the next call makes a fresh one", func() { + fb.err = errors.Join(sandbox.ErrGone, errors.New("the guest exited")) + m.Sweep(context.Background()) + Expect(rig.must("echo two")).To(ContainSubstring("two")) + created, _ := fb.counts() + Expect(created).To(Equal(2)) + }) + + for name, cause := range map[string]error{ + "a failed checkpoint": errors.New("remote: no request is being served"), + "a busy sandbox": sandbox.ErrBusy, + } { + It("waits before it suspends again after "+name+", and keeps the sandbox usable", func() { + fb.err = cause + m.Sweep(context.Background()) + now = now.Add(time.Minute) + m.Sweep(context.Background()) + _, suspends := fb.counts() + Expect(suspends).To(Equal(1), "the next sweep a minute later does not try again") + now = now.Add(time.Hour) + m.Sweep(context.Background()) + _, suspends = fb.counts() + Expect(suspends).To(Equal(2), "it tries again after the wait") + Expect(rig.must("echo three")).To(ContainSubstring("three")) + created, _ := fb.counts() + Expect(created).To(Equal(1), "the sandbox that could not sleep still serves") + }) + } +})