-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: implement Qdrant memory optimization with constants in @roo-code/types #6370
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
Conversation
…e/types - Create comprehensive Qdrant configuration constants in @roo-code/types package - Add memory optimization config fields to codebase-index schema - Configure Qdrant to use on-disk storage for vectors and HNSW indexes by default - Add memory-mapped file support for segments larger than 50k vectors - Keep HNSW search parameter (ef) at 128 for testing purposes - Update all affected files to use constants from types package - Update all test files to handle new configuration parameters Fixes #6262
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.
I have reviewed this PR implementing Qdrant memory optimization. Overall, this is an excellent implementation that addresses issue #6262 effectively. The code quality is high, with good separation of concerns and comprehensive test coverage.
Review Summary
Critical Issues (Must Fix):
-
Missing memory optimization fields in config snapshot - In
src/services/code-index/config-manager.tsline 183, thePreviousConfigSnapshotcreation is missing the memory optimization fields (useOnDiskStorage, memoryMapThreshold, hnswEfSearch) that are defined in the interface. This could cause restart detection issues when memory optimization settings change. -
Unused import in config interface - In
src/services/code-index/interfaces/config.tsline 3,QdrantMemoryOptimizationConfigis imported but not used in the interface definitions, suggesting potential inconsistency in type usage.
Important Suggestions (Should Consider):
-
Test coverage for memory optimization behavior - The tests in
src/services/code-index/vector-store/__tests__/qdrant-client.spec.tsverify that memory optimization config is passed to collection creation but do not test the actual memory optimization behavior or edge cases (e.g., what happens when useOnDiskStorage is false, or when memoryMapThreshold is exceeded). -
Constants organization and documentation - The
QDRANT_QUANTIZATION_CONFIG_DEFAULTSinpackages/types/src/qdrant.tsare only used in search operations. Could these be better organized or documented for their specific use case to improve maintainability?
Minor Improvements (Nice to Have):
-
Enhanced documentation with examples - The new constants are well-documented, but could benefit from examples showing the memory impact of different settings to help developers understand the trade-offs.
-
Type safety improvement - Consider making the
QdrantMemoryOptimizationConfiginterface more strict by requiring at least one property to be defined, preventing empty configuration objects.
Positive Aspects
- ✅ Excellent implementation of requirements from issue #6262
- ✅ Comprehensive test coverage updates across all affected files
- ✅ Good separation of concerns with constants properly placed in types package
- ✅ Proper backward compatibility maintained
- ✅ HNSW ef parameter correctly kept at 128 as requested for testing
- ✅ Clean integration with existing configuration management
- ✅ Well-structured constants with clear documentation
The implementation should significantly reduce Qdrant memory consumption as intended. The architecture is sound and the code follows established patterns in the codebase.
|
@roomote-agent remove the new settings that you added, these memory optimizations will always be on |
|
Acknowledged! I'll remove the new settings that were added and make the memory optimizations always on. Working on this now. |
- Remove memory optimization settings from codebase-index schema - Remove memory optimization properties from config-manager - Update qdrant-client to always use memory optimization defaults - Update service-factory to remove memory optimization config passing - Update tests to remove memory optimization configuration expectations Memory optimizations are now always enabled as requested by @daniel-lxs
|
✅ Done! I've removed the configurable memory optimization settings as requested. The memory optimizations are now always enabled. Changes Made:
Result:Memory optimizations (on-disk storage, memory-mapped files, optimized HNSW settings) are now always enabled using the constants from ✅ All tests pass (374/374 in code-index service) The Qdrant memory optimizations will now always be active, reducing RAM consumption from ~1.5GB to much lower levels while maintaining search performance. |
- Remove useOnDiskStorage, memoryMapThreshold, and hnswEfSearch from CodeIndexConfig - Remove same fields from PreviousConfigSnapshot type - Remove unused QdrantMemoryOptimizationConfig import and interface - Update comment in service-factory.ts for clarity - Add documentation to clarify QDRANT_QUANTIZATION_CONFIG_DEFAULTS is for search operations Memory optimizations are now always enabled via constants as originally requested.
This PR implements Qdrant memory optimization to address issue #6262 by reducing RAM consumption when loading project indexes.
Changes Made
🏗️ Architecture
@roo-code/typespackage🧠 Memory Optimization Features
📁 Files Modified
packages/types/src/qdrant.ts- New constants file with memory optimization defaultspackages/types/src/codebase-index.ts- Added memory optimization schema fieldssrc/services/code-index/config-manager.ts- Updated to use constants from types packagesrc/services/code-index/vector-store/qdrant-client.ts- Implemented memory optimization configsrc/services/code-index/service-factory.ts- Updated to pass memory optimization config🧪 Testing
Impact
This implementation should significantly reduce Qdrant memory consumption from ~1.5GB to much lower levels by storing vectors and indexes on disk instead of in RAM, while maintaining search performance through memory-mapped files and optimized HNSW configuration.
Fixes #6262
Important
Implements Qdrant memory optimization by storing vectors and indexes on disk, using memory-mapped files, and updating configuration and tests.
qdrant-client.ts.efparameter at 128 for testing.qdrant.ts.codebase-index.tsto include memory optimization schema fields.config-manager.tsandservice-factory.ts.qdrant-client.spec.tsto cover new configuration parameters.This description was created by
for 33c22f7. You can customize this summary. It will automatically update as commits are pushed.