Skip to content

Keep optimizer store's in-memory database alive - #6157

Open
aponcedeleonch wants to merge 1 commit into
mainfrom
fix-optimizer-store-schema-loss
Open

Keep optimizer store's in-memory database alive#6157
aponcedeleonch wants to merge 1 commit into
mainfrom
fix-optimizer-store-schema-loss

Conversation

@aponcedeleonch

Copy link
Copy Markdown
Member

Summary

The optimizer's tool store runs on a shared-cache in-memory SQLite database, which exists only while at least one connection to it is open. When the database/sql pool drops to zero connections SQLite discards the whole database, and nothing recreates it because the schema is executed only in the constructor. Every later session build and find_tool then fails with no such table: llm_capabilities until the pod restarts, while the process stays healthy and /readyz stays green.

The pool does reach zero. modernc.org/sqlite implements context cancellation with sqlite3_interrupt and reports an interrupted connection as unusable from conn.IsValid, so database/sql discards a connection whose statement was canceled rather than returning it to the pool. Because the store is touched only on session build and find_tool, a single pooled connection is the normal steady state for a quiet pod, so one canceled request is enough to destroy the database for the life of the process. An embedding-service outage makes this much more likely, since slow and hanging calls cause clients to give up mid-statement, but the fault is not specific to embeddings: a well-timed client disconnect does the same thing with a perfectly healthy embedder.

What changed:

  • sqliteToolStore now pins a dedicated keepAlive connection for its whole lifetime, acquired with a background context and never used to run statements, so it can never be interrupted and never discarded.
  • Close releases that connection ahead of the pool and tolerates sql.ErrConnDone, preserving the documented "safe to call Close multiple times" contract on types.ToolStore.
  • Added a regression test that interrupts statements mid-flight and asserts the store still serves searches.

Fixes #5889

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

The new test was verified to actually catch the regression, not just pass: with the keep-alive connection released immediately after acquisition, it fails with the exact error from the issue.

FTS5 query failed: SQL logic error: no such table: llm_capabilities_fts (1)

Does this introduce a user-facing change?

Yes. A vMCP pod running with the optimizer enabled could permanently lose its tool index while continuing to report itself healthy, with every find_tool call and every new session failing until the pod was deleted. That no longer happens.

Special notes for reviewers

Two alternatives I did not take:

Scope notes: the store is per-pod in-memory state, so there is nothing to coordinate across replicas. With multiple pods only the poisoned one misbehaves, which is part of why this was hard to spot. The change is also inert when the optimizer is disabled, since resolveOptimizer never builds a store in that case.

Pre-existing failures unrelated to this PR, which reproduce on a clean main at 33e06f4:

  • task test: TestParseGitReference_RefAndSubdir/git://github.com/org/repo and three TestValidateOCIRegistryHost subtests, all in pkg/plugins/pluginsvc. That package has no dependency on the optimizer. All other packages pass, including ./pkg/vmcp/optimizer/... under -race.
  • task lint-fix: one gosec G115 finding in cmd/thv/app/upgrade.go:204, a file this branch does not touch.

Generated with Claude Code

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.55%. Comparing base (b7e824c) to head (0241d08).

Files with missing lines Patch % Lines
.../vmcp/optimizer/internal/toolstore/sqlite_store.go 45.45% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6157      +/-   ##
==========================================
- Coverage   72.56%   72.55%   -0.01%     
==========================================
  Files         736      736              
  Lines       76331    76341      +10     
==========================================
  Hits        55391    55391              
- Misses      17019    17024       +5     
- Partials     3921     3926       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The optimizer's tool store runs on a shared-cache in-memory SQLite
database, which exists only while at least one connection to it is open.
When the database/sql pool drops to zero connections SQLite discards the
whole database, and nothing recreates it because the schema is executed
only in the constructor. Every later session build and find_tool then
fails with "no such table: llm_capabilities" until the pod restarts,
while the process stays healthy and /readyz stays green.

The pool does reach zero. modernc.org/sqlite implements context
cancellation with sqlite3_interrupt and reports an interrupted
connection as unusable from conn.IsValid, so database/sql discards a
connection whose statement was canceled rather than returning it to the
pool. Because the store is touched only on session build and find_tool,
a single pooled connection is the normal steady state, so one canceled
request is enough to destroy the database for the life of the process.
An embedding-service outage makes this far more likely, since slow and
hanging calls cause clients to give up mid-statement, but the fault is
not specific to embeddings.

Pin a dedicated connection for the store's lifetime. It is acquired with
a background context and never used to run statements, so it can never
be interrupted and never discarded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@aponcedeleonch
aponcedeleonch force-pushed the fix-optimizer-store-schema-loss branch from a25e39b to 0241d08 Compare July 31, 2026 12:55
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimizer store never recovers after a transient embedding failure at startup (no such table: llm_capabilities)

2 participants