Skip to content

Commit bda86fb

Browse files
committed
fix(index): paginate per-profile rebuild/clear beyond search-page caps
GetToolsByServer capped a single Bleve search at 10000 hits and DeleteAll at 100000, so a server with >10k indexed tools or an index with >100k docs would silently drop the overflow during a per-profile rebuild/clear, leaving the profile index inconsistent with the shared index. Both now loop over search pages until exhausted, so coverage is never bounded by a single page: - GetToolsByServer paginates with From/Size, ending on a short final page. - DeleteAll deletes page-by-page from offset 0 (each batch removes the enumerated docs, surfacing the next page) until the index is empty. Page size is a configurable BleveIndex field (default 10000) so the regression tests exercise the loop with a small, fast corpus. Related #756. Follow-up to MCP-3240 Codex review (MCP-3307).
1 parent b011cda commit bda86fb

2 files changed

Lines changed: 157 additions & 34 deletions

File tree

internal/index/bleve.go

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,19 @@ import (
1414
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
1515
)
1616

17+
// defaultSearchPageSize is the page size used when paginating full-coverage
18+
// scans (GetToolsByServer, DeleteAll). It is a generous upper bound on the
19+
// number of docs in a single page; pagination loops over as many pages as
20+
// needed, so total coverage is never bounded by this value (MCP-3319).
21+
const defaultSearchPageSize = 10000
22+
1723
// BleveIndex wraps Bleve index operations
1824
type BleveIndex struct {
1925
index bleve.Index
2026
logger *zap.Logger
27+
// searchPageSize bounds a single search page during paginated full scans.
28+
// Defaults to defaultSearchPageSize; overridable in tests.
29+
searchPageSize int
2130
}
2231

2332
// ToolDocument represents a tool document in the index
@@ -61,8 +70,9 @@ func newBleveIndexAt(indexPath string, logger *zap.Logger) (*BleveIndex, error)
6170
}
6271

6372
return &BleveIndex{
64-
index: index,
65-
logger: logger,
73+
index: index,
74+
logger: logger,
75+
searchPageSize: defaultSearchPageSize,
6676
}, nil
6777
}
6878

@@ -304,21 +314,29 @@ func (b *BleveIndex) GetDocumentCount() (uint64, error) {
304314
// its on-disk directory.
305315
func (b *BleveIndex) DeleteAll() error {
306316
query := bleve.NewMatchAllQuery()
307-
searchReq := bleve.NewSearchRequest(query)
308-
searchReq.Size = 100000 // generous upper bound on indexed tools
309-
searchResult, err := b.index.Search(searchReq)
310-
if err != nil {
311-
return fmt.Errorf("failed to enumerate documents for delete-all: %w", err)
312-
}
317+
// Delete in pages until the index is empty. Each iteration enumerates a
318+
// page from offset 0 and deletes exactly those docs, so the next search
319+
// surfaces the following page — no From offset bookkeeping, and coverage is
320+
// not bounded by a single search page (MCP-3319).
321+
for {
322+
searchReq := bleve.NewSearchRequest(query)
323+
searchReq.Size = b.searchPageSize
324+
searchResult, err := b.index.Search(searchReq)
325+
if err != nil {
326+
return fmt.Errorf("failed to enumerate documents for delete-all: %w", err)
327+
}
328+
if len(searchResult.Hits) == 0 {
329+
return nil
330+
}
313331

314-
batch := b.index.NewBatch()
315-
for _, hit := range searchResult.Hits {
316-
batch.Delete(hit.ID)
317-
}
318-
if batch.Size() == 0 {
319-
return nil
332+
batch := b.index.NewBatch()
333+
for _, hit := range searchResult.Hits {
334+
batch.Delete(hit.ID)
335+
}
336+
if err := b.index.Batch(batch); err != nil {
337+
return fmt.Errorf("failed to delete documents for delete-all: %w", err)
338+
}
320339
}
321-
return b.index.Batch(batch)
322340
}
323341

324342
// Batch operations for efficiency
@@ -382,30 +400,40 @@ func (b *BleveIndex) GetToolsByServer(serverName string) ([]*config.ToolMetadata
382400
query := bleve.NewTermQuery(serverName)
383401
query.SetField("server_name")
384402

385-
// Create search request with high limit to get all tools
386-
searchReq := bleve.NewSearchRequest(query)
387-
searchReq.Size = 10000 // Maximum tools per server
388-
searchReq.Fields = []string{"tool_name", "full_tool_name", "server_name", "description", "params_json", "output_schema_json", "hash"}
403+
fields := []string{"tool_name", "full_tool_name", "server_name", "description", "params_json", "output_schema_json", "hash"}
389404

390405
b.logger.Debug("Querying tools by server", zap.String("server", serverName))
391406

392-
searchResult, err := b.index.Search(searchReq)
393-
if err != nil {
394-
return nil, fmt.Errorf("failed to query tools by server: %w", err)
395-
}
396-
397-
// Convert results to ToolMetadata
407+
// Paginate so a server exposing more than one search page of tools is fully
408+
// covered — a single capped search would silently drop the overflow
409+
// (MCP-3319). A short final page (fewer hits than the page size) ends the loop.
398410
var tools []*config.ToolMetadata
399-
for _, hit := range searchResult.Hits {
400-
toolMeta := &config.ToolMetadata{
401-
Name: getStringField(hit.Fields, "full_tool_name"),
402-
ServerName: getStringField(hit.Fields, "server_name"),
403-
Description: getStringField(hit.Fields, "description"),
404-
ParamsJSON: getStringField(hit.Fields, "params_json"),
405-
OutputSchemaJSON: getStringField(hit.Fields, "output_schema_json"),
406-
Hash: getStringField(hit.Fields, "hash"),
411+
for from := 0; ; from += b.searchPageSize {
412+
searchReq := bleve.NewSearchRequest(query)
413+
searchReq.From = from
414+
searchReq.Size = b.searchPageSize
415+
searchReq.Fields = fields
416+
417+
searchResult, err := b.index.Search(searchReq)
418+
if err != nil {
419+
return nil, fmt.Errorf("failed to query tools by server: %w", err)
420+
}
421+
422+
for _, hit := range searchResult.Hits {
423+
toolMeta := &config.ToolMetadata{
424+
Name: getStringField(hit.Fields, "full_tool_name"),
425+
ServerName: getStringField(hit.Fields, "server_name"),
426+
Description: getStringField(hit.Fields, "description"),
427+
ParamsJSON: getStringField(hit.Fields, "params_json"),
428+
OutputSchemaJSON: getStringField(hit.Fields, "output_schema_json"),
429+
Hash: getStringField(hit.Fields, "hash"),
430+
}
431+
tools = append(tools, toolMeta)
432+
}
433+
434+
if len(searchResult.Hits) < b.searchPageSize {
435+
break
407436
}
408-
tools = append(tools, toolMeta)
409437
}
410438

411439
b.logger.Debug("Found tools for server",

internal/index/bleve_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package index
22

33
import (
4+
"fmt"
45
"os"
56
"testing"
67
"time"
@@ -506,3 +507,97 @@ func createTestDeFiLlamaTools() []*config.ToolMetadata {
506507

507508
return tools
508509
}
510+
511+
// TestBleveIndex_GetToolsByServer_BeyondPageCap verifies that GetToolsByServer
512+
// returns every tool of a server even when the server exposes more tools than a
513+
// single search page can hold. Regression for MCP-3319: the prior single-search
514+
// implementation hard-capped at 10000 hits, silently dropping the overflow.
515+
func TestBleveIndex_GetToolsByServer_BeyondPageCap(t *testing.T) {
516+
tmpDir, err := os.MkdirTemp("", "bleve_test_*")
517+
require.NoError(t, err)
518+
defer os.RemoveAll(tmpDir)
519+
520+
logger := zap.NewNop()
521+
bleveIndex, err := NewBleveIndex(tmpDir, logger)
522+
require.NoError(t, err)
523+
defer bleveIndex.Close()
524+
525+
// Shrink the page size so the test can exercise the pagination loop with a
526+
// small, fast corpus instead of indexing >10k docs.
527+
bleveIndex.searchPageSize = 3
528+
529+
// Index 7 tools for one server (spans 3 pages: 3 + 3 + 1) plus an unrelated
530+
// server to confirm the term filter is preserved across pages.
531+
const total = 7
532+
tools := make([]*config.ToolMetadata, 0, total+1)
533+
for i := 0; i < total; i++ {
534+
tools = append(tools, &config.ToolMetadata{
535+
Name: fmt.Sprintf("paged:tool_%02d", i),
536+
ServerName: "paged",
537+
Description: fmt.Sprintf("Paged tool %d", i),
538+
Hash: fmt.Sprintf("hash_%02d", i),
539+
})
540+
}
541+
tools = append(tools, &config.ToolMetadata{
542+
Name: "other:tool_x",
543+
ServerName: "other",
544+
Description: "Other tool",
545+
Hash: "hash_other",
546+
})
547+
548+
require.NoError(t, bleveIndex.BatchIndex(tools))
549+
550+
got, err := bleveIndex.GetToolsByServer("paged")
551+
require.NoError(t, err)
552+
assert.Len(t, got, total, "all tools across every page must be returned")
553+
554+
names := make(map[string]bool, len(got))
555+
for _, tm := range got {
556+
assert.Equal(t, "paged", tm.ServerName)
557+
names[tm.Name] = true
558+
}
559+
for i := 0; i < total; i++ {
560+
assert.True(t, names[fmt.Sprintf("paged:tool_%02d", i)],
561+
"tool_%02d missing from paginated result", i)
562+
}
563+
}
564+
565+
// TestBleveIndex_DeleteAll_BeyondPageCap verifies that DeleteAll clears every
566+
// document even when the index holds more docs than a single search page.
567+
// Regression for MCP-3319: the prior single-search implementation deleted only
568+
// the first 100000 docs, leaving stale docs beyond the cap.
569+
func TestBleveIndex_DeleteAll_BeyondPageCap(t *testing.T) {
570+
tmpDir, err := os.MkdirTemp("", "bleve_test_*")
571+
require.NoError(t, err)
572+
defer os.RemoveAll(tmpDir)
573+
574+
logger := zap.NewNop()
575+
bleveIndex, err := NewBleveIndex(tmpDir, logger)
576+
require.NoError(t, err)
577+
defer bleveIndex.Close()
578+
579+
bleveIndex.searchPageSize = 3
580+
581+
// Index 10 docs (spans multiple pages of size 3).
582+
const total = 10
583+
tools := make([]*config.ToolMetadata, 0, total)
584+
for i := 0; i < total; i++ {
585+
tools = append(tools, &config.ToolMetadata{
586+
Name: fmt.Sprintf("srv:tool_%02d", i),
587+
ServerName: "srv",
588+
Description: fmt.Sprintf("Tool %d", i),
589+
Hash: fmt.Sprintf("h_%02d", i),
590+
})
591+
}
592+
require.NoError(t, bleveIndex.BatchIndex(tools))
593+
594+
count, err := bleveIndex.GetDocumentCount()
595+
require.NoError(t, err)
596+
require.Equal(t, uint64(total), count)
597+
598+
require.NoError(t, bleveIndex.DeleteAll())
599+
600+
count, err = bleveIndex.GetDocumentCount()
601+
require.NoError(t, err)
602+
assert.Equal(t, uint64(0), count, "DeleteAll must remove every document across all pages")
603+
}

0 commit comments

Comments
 (0)