Skip to content

Optimize glob walkdir - #3013

Merged
qin-ctx merged 17 commits into
volcengine:mainfrom
baojun-zhang:optimize-glob-walkdir
Jul 6, 2026
Merged

Optimize glob walkdir#3013
qin-ctx merged 17 commits into
volcengine:mainfrom
baojun-zhang:optimize-glob-walkdir

Conversation

@baojun-zhang

@baojun-zhang baojun-zhang commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR moves glob from Python-side tree() + PurePath.match() filtering into RAGFS as a first-class paged backend operation.

Before this change, VikingFS.glob() fetched a large tree from Python and then applied PurePath.match() as the final matcher. That made glob expensive for large backends and also exposed Python PurePath.match() suffix/basename semantics, which are not aligned with standard glob behavior.

After this change, glob_directory() is implemented in RAGFS and exposed through the Python binding / AGFS async client. Matching is performed in Rust with globset against query-root-relative paths. This intentionally changes glob semantics from Python PurePath.match() to standard globset semantics.

This PR also optimizes backend-specific glob implementations:

  • LocalFS uses ignore::WalkBuilder instead of custom DFS walker/cursor code.
  • S3FS implements paged glob traversal directly on top of S3 ListObjectsV2 pages, avoiding full tree materialization.
  • Multi-write wrappers delegate glob to the primary backend when possible, avoiding fallback to the expensive trait default path.

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Added FileSystem::glob_directory() to RAGFS with GlobPage, GlobEntry, opaque continuation tokens, page size validation, and shared globset helpers.
  • Exposed paged glob through ragfs-python, openviking/pyagfs, and VikingFS.glob(), replacing Python-side tree() + PurePath.match() filtering.
  • Changed glob semantics from Python PurePath.match() suffix matching to standard globset full query-root-relative path matching.
  • Updated callers and tests from legacy basename patterns to standard recursive patterns, e.g. *.md -> **/*.md, * -> **/*, *.jsonl -> **/*.jsonl.
  • Replaced LocalFS custom glob walker/cursor logic with ignore::WalkBuilder, while keeping DFS traversal and adding static max-depth pruning for patterns without **, such as 200*.
  • Added S3FS paged glob implementation using ListObjectsV2 page scanning, S3GlobToken, buffered page entries, S3 continuation tokens, and cross-page DFS ancestor de-duplication.
  • Added MultiWriteWrappedFS::glob_directory() so glob can delegate to optimized backend implementations instead of falling back to default full-tree traversal.
  • Added tests covering standard glob semantics, token scope validation, backend paging, visibility filtering, URI alias preservation, LocalFS pruning, and S3 paginated tree/glob stitching behavior.
  • change /api/v1/search/glob default node_limit from None to 256
  • add server test covering omitted node_limit behavior
  • update zh/en API docs to match the new default

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

Glob semantics change

This PR intentionally replaces Python PurePath.match() semantics with Rust globset standard glob semantics.

Important examples:

  • Old PurePath.match() behavior:

    • PurePath("a/b.md").match("*.md") == True
    • PurePath("a.md").match("**/*.md") == False
  • New globset behavior:

    • *.md matches only root-level *.md under the query root.
    • **/*.md matches Markdown files at any depth, including root-level files.
    • Multi-segment patterns are anchored at the query root.

Migration rule:

  • Use **/*.md, **/*.py, **/*.jsonl, or **/* when recursive matching is intended.
  • Use single-segment patterns such as 200* only when matching root-level entries under the query path is intended.

LocalFS glob behavior

LocalFS now uses ignore::WalkBuilder. This does not turn traversal into BFS; traversal remains DFS.

Performance improvements come from:

  • removing custom walker/cursor code,
  • using mature walkdir-style traversal,
  • using precompiled globset matching,
  • avoiding Python PurePath.match() filtering,
  • pruning traversal depth when the pattern cannot match deeper paths.

For example, 200* has no / and no **, so LocalFS limits traversal to depth 1.

S3FS glob behavior

S3FS now performs glob directly over S3 listing pages instead of materializing a full tree first.

The S3 glob token tracks:

  • S3 scan continuation token,
  • buffered matched entries from the current listing page,
  • page offset within buffered entries,
  • last emitted relative path parts for cross-page ancestor de-duplication.

This allows S3 glob pagination to continue scanning from the previous S3 page without rebuilding the whole tree for every request.

@qin-ctx qin-ctx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这次 review 结论是 REQUEST_CHANGES。后端 glob 下推和分页方向整体合理,但 HTTP glob 默认改为 256 后,Python SDK / 高层 client 没有同步暴露 node_limit,既有 SDK 调用会被静默截断;另外英文 filesystem 文档误把 grep 的 node_limit 默认写成 256。

Comment thread openviking/server/routers/search.py
Comment thread docs/en/api/03-filesystem.md Outdated
@qin-ctx
qin-ctx merged commit 6a33ebb into volcengine:main Jul 6, 2026
11 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants