phraser: the startup wait is a config field, and it is tested (V-323)
The last untested arm of startLlamaProc was `case <-time.After(60 * time.Second)`, and it could not be tested as written — a test would have had to wait a real minute. It is Config.StartupTimeout now, defaulted to 60s in DefaultConfig and surfaced as phraser.startup_timeout, so the box can raise it: a cold 1.7B loading off a spinning disk can outrun 60s and that failed the boot with nothing to turn. The test asserts the child is dead, not just that the error came back. That arm is the one most likely to leak, because the child is alive and busy loading a model rather than already gone — so the fake records its pid before it hangs. startLlamaProc 90.9% → 95.7%, package 76.9% → 77.3%.
This commit is contained in:
+9
-8
@@ -246,14 +246,15 @@ func run(args []string) error {
|
||||
phr = phraser.NewStub()
|
||||
if cfg.Phraser != nil {
|
||||
pc := phraser.Config{
|
||||
ModelPath: cfg.Phraser.ModelPath,
|
||||
BinPath: cfg.Phraser.BinPath,
|
||||
Listen: cfg.Phraser.Listen,
|
||||
NGpuLayers: cfg.Phraser.NGpuLayers,
|
||||
NCtx: cfg.Phraser.NCtx,
|
||||
Timeout: time.Duration(cfg.Phraser.Timeout),
|
||||
LLMNudges: cfg.Phraser.LLMNudges,
|
||||
ContextBlock: contextBlockFn(cfg, time.Now),
|
||||
ModelPath: cfg.Phraser.ModelPath,
|
||||
BinPath: cfg.Phraser.BinPath,
|
||||
Listen: cfg.Phraser.Listen,
|
||||
NGpuLayers: cfg.Phraser.NGpuLayers,
|
||||
NCtx: cfg.Phraser.NCtx,
|
||||
Timeout: time.Duration(cfg.Phraser.Timeout),
|
||||
StartupTimeout: time.Duration(cfg.Phraser.StartupTimeout),
|
||||
LLMNudges: cfg.Phraser.LLMNudges,
|
||||
ContextBlock: contextBlockFn(cfg, time.Now),
|
||||
}
|
||||
if pc.BinPath == "" {
|
||||
pc.BinPath = "llama-server"
|
||||
|
||||
@@ -1233,6 +1233,13 @@ type PhraserConfig struct {
|
||||
NCtx int `json:"n_ctx,omitempty"`
|
||||
Timeout Duration `json:"timeout,omitempty"`
|
||||
|
||||
// StartupTimeout — how long the daemon waits for llama-server to print its
|
||||
// listen line at boot. Zero ⇒ the phraser's 60s default.
|
||||
//
|
||||
// It is here because a cold 1.7B loading off a spinning disk can outrun 60s,
|
||||
// and that failed the boot with no way to raise it (Vikunja #323).
|
||||
StartupTimeout Duration `json:"startup_timeout,omitempty"`
|
||||
|
||||
// LLMNudges — let the model word nudges again. Off by default: nudges are
|
||||
// worded from hand-written Russian templates now (the model broke the
|
||||
// persona and invented units). Chat, query and reminder phrasing always go
|
||||
|
||||
@@ -101,8 +101,21 @@ type Config struct {
|
||||
// ever fights the grammar, the fix should be a config flip on the
|
||||
// deploy box, not a code change and a rebuild.
|
||||
NoGrammar bool
|
||||
|
||||
// StartupTimeout — how long to wait for llama-server to print its listen
|
||||
// line before giving up, killing the child and returning an error.
|
||||
//
|
||||
// A field rather than a constant because the box may legitimately want
|
||||
// longer: a cold 1.7B loading off a spinning disk can outrun the 60s
|
||||
// default, and that used to fail the boot with no way to raise it
|
||||
// (Vikunja #323). Zero ⇒ defaultStartupTimeout.
|
||||
StartupTimeout time.Duration
|
||||
}
|
||||
|
||||
// defaultStartupTimeout — the wait DefaultConfig sets and the zero value falls
|
||||
// back to, so a Config built by hand still has a ceiling.
|
||||
const defaultStartupTimeout = 60 * time.Second
|
||||
|
||||
func DefaultConfig(modelPath string) Config {
|
||||
return Config{
|
||||
ModelPath: modelPath,
|
||||
@@ -111,6 +124,8 @@ func DefaultConfig(modelPath string) Config {
|
||||
NGpuLayers: -1,
|
||||
NCtx: 2048,
|
||||
Timeout: 30 * time.Second,
|
||||
|
||||
StartupTimeout: defaultStartupTimeout,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -278,6 +293,10 @@ func startLlamaProc(ctx context.Context, cfg Config) (*llamaProc, error) {
|
||||
}
|
||||
}()
|
||||
|
||||
startupTimeout := cfg.StartupTimeout
|
||||
if startupTimeout <= 0 {
|
||||
startupTimeout = defaultStartupTimeout
|
||||
}
|
||||
select {
|
||||
case addr := <-portCh:
|
||||
p.base = addr
|
||||
@@ -290,10 +309,12 @@ func startLlamaProc(ctx context.Context, cfg Config) (*llamaProc, error) {
|
||||
_ = cmd.Process.Kill()
|
||||
_ = cmd.Wait()
|
||||
return nil, ctx.Err()
|
||||
case <-time.After(60 * time.Second):
|
||||
case <-time.After(startupTimeout):
|
||||
// The arm most likely to leak: the child is still loading a model, so
|
||||
// it is alive and busy rather than dead. Kill and reap before the error.
|
||||
_ = cmd.Process.Kill()
|
||||
_ = cmd.Wait()
|
||||
return nil, fmt.Errorf("llm: server did not start within 60s")
|
||||
return nil, fmt.Errorf("llm: server did not start within %s", startupTimeout)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -105,6 +105,35 @@ exit 1`)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("startup timeout", func(t *testing.T) {
|
||||
// The 60s wait was a literal in the select, so this arm could only be
|
||||
// tested by waiting a real minute (Vikunja #323). It is Config now.
|
||||
//
|
||||
// The child records its pid, because startLlamaProc returns nil on
|
||||
// every failure arm and this is the arm where the child is alive and
|
||||
// busy loading a model rather than already dead.
|
||||
pidFile := filepath.Join(t.TempDir(), "pid")
|
||||
bin := fakeLlama(t, `echo $$ > `+pidFile+`
|
||||
while : ; do sleep 1 ; done`)
|
||||
cfg := testCfg(bin)
|
||||
cfg.StartupTimeout = 50 * time.Millisecond
|
||||
_, err := startLlamaProc(context.Background(), cfg)
|
||||
if err == nil || !strings.Contains(err.Error(), "did not start within") {
|
||||
t.Fatalf("err = %v, want the startup-timeout arm", err)
|
||||
}
|
||||
raw, readErr := os.ReadFile(pidFile)
|
||||
if readErr != nil {
|
||||
t.Fatalf("the child never ran: %v", readErr)
|
||||
}
|
||||
pid, convErr := strconv.Atoi(strings.TrimSpace(string(raw)))
|
||||
if convErr != nil {
|
||||
t.Fatalf("pid file %q: %v", raw, convErr)
|
||||
}
|
||||
if err := syscall.Kill(pid, 0); err == nil {
|
||||
t.Errorf("child %d survived the startup timeout", pid)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("context cancelled during startup", func(t *testing.T) {
|
||||
// Never prints the listen line and never exits: only ctx can end this.
|
||||
bin := fakeLlama(t, `while : ; do sleep 1 ; done`)
|
||||
|
||||
Reference in New Issue
Block a user