Repository: ollama/ollama
Pull request: Fix/issue 3736: When runners are closing or expiring. Scheduler is getting dirty VRAM size readings.

Issue: When the Ollama `Scheduler` requests a runner to stop (kill), the `Scheduler` reads the available VRAM and gets a size that includes the terminating runner. This results in offloading to the CPU and slower execution. Each time a new model is swapped in, the new runner reads the previous runner's memory allocation. This affects the new runner's VRAM allocation estimate.
Fix: When stopping a runner, wait for the process to exit so that the memory is free before `Scheduler` checks the amount of VRAM available.

Issue: After a runner finishes a request, an expiration timer is assigned based on the session duration. Subsequent requests will renew the expiration timer after each request has finished. If a request happens to take too long and the timer fires, the runner will be scheduled to be unloaded.  If concurrently, a new pending may get an incorrect measure of VRAM, resulting in offloading to the CPU and slower execution.  Runners are expiring in the middle of heavy use, which results in the same model closing and reloading.  The reloading gets a dirty VRAM measurement because the previous runner is not fully closed before the new runner is created. The concurrency of the pending and completed Go routines.  The pending continues on the unloaded event, which can be "any" unloaded event.  
Fix: Clear the timer so it does not fire when reusing runners.  Only assign the timer when the runner has finished.  Clear any assigned timers when closing runners.  An active runner should not have an expire timer.  

Changed files:
filename: .gitignore
patch: @@ -11,4 +11,5 @@ ggml-metal.metal
 .idea
 test_data
 *.crt
-llm/build
\ No newline at end of file
+llm/build
+__debug_bin*
\ No newline at end of file

filename: llm/server.go
patch: @@ -300,12 +300,6 @@ func NewLlamaServer(gpus gpu.GpuInfoList, model string, ggml *GGML, adapters, pr
 			continue
 		}
 
-		// reap subprocess when it exits
-		go func() {
-			// Exit status managed via getServerStatus
-			_ = s.cmd.Wait()
-		}()
-
 		// TODO - make sure this is all wired up correctly
 		// if err = s.WaitUntilRunning(); err != nil {
 		// 	slog.Error("error starting llama server", "server", servers[i], "error", err)
@@ -899,7 +893,13 @@ func (s *llmServer) Detokenize(ctx context.Context, tokens []int) (string, error
 func (s *llmServer) Close() error {
 	if s.cmd != nil {
 		slog.Debug("stopping llama server")
-		return s.cmd.Process.Kill()
+		if err := s.cmd.Process.Kill(); err != nil {
+			return err
+		}
+
+		_ = s.cmd.Wait()
+
+		slog.Debug("llama server stopped")
 	}
 
 	return nil

filename: server/sched.go
patch: @@ -250,6 +250,7 @@ func (s *Scheduler) processCompleted(ctx context.Context) {
 						defer runner.refMu.Unlock()
 						if runner.expireTimer != nil {
 							runner.expireTimer.Stop()
+							runner.expireTimer = nil
 						}
 						s.expiredCh <- runner
 					})
@@ -296,6 +297,10 @@ func (pending *LlmRequest) useLoadedRunner(runner *runnerRef, finished chan *Llm
 	runner.refMu.Lock()
 	defer runner.refMu.Unlock()
 	runner.refCount++
+	if runner.expireTimer != nil {
+		runner.expireTimer.Stop()
+		runner.expireTimer = nil
+	}
 	runner.sessionDuration = pending.sessionDuration
 	pending.successCh <- runner
 	go func() {
@@ -426,6 +431,10 @@ type runnerRef struct {
 
 // The refMu must already be held when calling unload
 func (runner *runnerRef) unload() {
+	if runner.expireTimer != nil {
+		runner.expireTimer.Stop()
+		runner.expireTimer = nil
+	}
 	if runner.llama != nil {
 		runner.llama.Close()
 	}