-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat: Add workspace isolation support for pipeline status #2353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
LightRAG/lightrag/api/routers/document_routes.py
Lines 2304 to 2311 in d6c5311
| try: | |
| from lightrag.kg.shared_storage import ( | |
| get_namespace_data, | |
| get_all_update_flags_status, | |
| ) | |
| pipeline_status = await get_namespace_data("pipeline_status") | |
The pipeline status endpoint still reads the global "pipeline_status" dictionary even though other parts of the change now isolate pipeline state per workspace (f"{workspace}:pipeline"). When a tenant operates in a non-default workspace, their jobs update a different namespace, so this endpoint will continue to return the idle global state and hide that the workspace is busy. Construct the namespace from rag.workspace and initialize it, similar to the updated delete/cancel handlers, so each tenant sees their own status.
LightRAG/lightrag/api/routers/document_routes.py
Lines 2514 to 2521 in d6c5311
| try: | |
| from lightrag.kg.shared_storage import get_namespace_data | |
| pipeline_status = await get_namespace_data("pipeline_status") | |
| # Check if pipeline is busy | |
| if pipeline_status.get("busy", False): | |
| return DeleteDocByIdResponse( |
The deletion entry point still checks await get_namespace_data("pipeline_status") for a global busy flag while the background task now marks busy inside the workspace-specific namespace (f"{workspace}:pipeline"). If two deletion requests are issued for the same workspace, the first sets busy in workspace:pipeline, but the second request reads the unused global dictionary and proceeds, defeating the lock and allowing concurrent deletions in one workspace. Build the namespace from rag.workspace and reuse the keyed lock to ensure the busy flag is consulted per workspace.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fixed Issues
Both endpoints now follow the same pattern as the other updated handlers:
CI checks should pass now. |
|
To use Codex here, create a Codex account and connect to github. |
Problem:
In multi-tenant scenarios, different workspaces share a single global
pipeline_status namespace, causing pipelines from different tenants to
block each other, severely impacting concurrent processing performance.
Solution:
- Extended get_namespace_data() to recognize workspace-specific pipeline
namespaces with pattern "{workspace}:pipeline" (following GraphDB pattern)
- Added workspace parameter to initialize_pipeline_status() for per-tenant
isolated pipeline namespaces
- Updated all 7 call sites to use workspace-aware locks:
* lightrag.py: process_document_queue(), aremove_document()
* document_routes.py: background_delete_documents(), clear_documents(),
cancel_pipeline(), get_pipeline_status(), delete_documents()
Impact:
- Different workspaces can process documents concurrently without blocking
- Backward compatible: empty workspace defaults to "pipeline_status"
- Maintains fail-fast: uninitialized pipeline raises clear error
- Expected N× performance improvement for N concurrent tenants
Bug fixes:
- Fixed AttributeError by using self.workspace instead of self.global_config
- Fixed pipeline status endpoint to show workspace-specific status
- Fixed delete endpoint to check workspace-specific busy flag
Code changes: 4 files, 141 insertions(+), 28 deletions(-)
Testing: All syntax checks passed, comprehensive workspace isolation tests completed
23ab491 to
2f3620b
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
目前的代码还需要解决对遗留代码的兼容性问题。LightRAG Core的用户的代码一直都是使用 The current codebase still requires addressing compatibility with legacy code. Previously, user code utilizing LightRAG Core initialized the pipeline via |
|
还有另一个兼容问题,
Another compatibility issue exists: the health check endpoint
|
Fixes two compatibility issues in workspace isolation: 1. Problem: lightrag_server.py calls initialize_pipeline_status() without workspace parameter, causing pipeline to initialize in global namespace instead of rag's workspace. Solution: Add set_default_workspace() mechanism in shared_storage. LightRAG.initialize_storages() now sets default workspace, which initialize_pipeline_status() uses when called without parameters. 2. Problem: /health endpoint hardcoded to use "pipeline_status", cannot return workspace-specific status or support frontend workspace selection. Solution: Add LIGHTRAG-WORKSPACE header support. Endpoint now extracts workspace from header or falls back to server default, returning correct workspace-specific pipeline status. Changes: - lightrag/kg/shared_storage.py: Add set/get_default_workspace() - lightrag/lightrag.py: Call set_default_workspace() in initialize_storages() - lightrag/api/lightrag_server.py: Add get_workspace_from_request() helper, update /health endpoint to support LIGHTRAG-WORKSPACE header Testing: - Backward compatibility: Old code works without modification - Multi-instance safety: Explicit workspace passing preserved - /health endpoint: Supports both default and header-specified workspaces Related: HKUDS#2353
已修复兼容性问题 / Compatibility Issues Fixed@danielaskdd 您好!我已经修复了您提出的两个兼容性问题: Hi @danielaskdd! I have fixed both compatibility issues you mentioned: 🔧 问题1:
|
Summary
This PR implements workspace isolation for pipeline status to enable concurrent document processing across multiple tenants without blocking each other.
Problem
In multi-tenant scenarios, different workspaces currently share a single global
pipeline_statusnamespace, causing pipelines from different tenants to block each other. This severely impacts concurrent processing performance.Solution
Core Changes
Extended
get_namespace_data()in shared_storage.py"{workspace}:pipeline"(following GraphDB pattern)"pipeline_status"Added workspace parameter to
initialize_pipeline_status()Updated 5 call sites
lightrag.py:process_document_queue(),aremove_document()document_routes.py:background_delete_documents(),clear_documents(),cancel_pipeline()get_storage_keyed_lock()Key Features
"pipeline_status"PipelineNotInitializedErrorBug Fixes
Fixed
AttributeErrorcaused by accessing non-existentself.global_config:self.workspaceinlightrag.py(2 locations)rag.workspaceindocument_routes.py(3 locations)Testing
All syntax checks passed and comprehensive testing completed:
Impact
Performance
Code Changes
References
utils_graph.py.claude/plan/minimal-workspace-isolation.md.claude/plan/linus-review-optimization.md