This repository was archived by the owner on Oct 10, 2025. It is now read-only.
Add HNSW parallel loading support with thread-safe lifecycle management #6051
+897
−86
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes a bug where OverflowFile::checkpoint() unconditionally allocated a header page even when no data had been written, causing PrimaryKeyIndexStorageInfo corruption and database reopen failures. **Root cause:** When creating a VectorIndex without inserting data, OverflowFile::checkpoint() allocated a page unnecessarily, setting overflowHeaderPage to a valid page index instead of INVALID_PAGE_IDX. **Fix:** Skip checkpoint when headerChanged == false, following the same design pattern as NodeTable and RelTable. The headerChanged flag is only set to true when actual string data (>12 bytes) is written via OverflowFileHandle::setStringOverflow(). **Test coverage:** Added overflow_file_checkpoint_test.cpp with 5 test cases documenting the bug scenario and verifying correct behavior.
- Add catalogEntry parameter to index_load_func_t signature - Update PrimaryKeyIndex::load() to accept catalogEntry (unused) - Update IndexHolder::load() to pass catalogEntry to load functions - Prevents recursive catalog lock acquisition during parallel loading - All index types now have consistent load() signature
- OnDiskHNSWIndex::load() now uses catalogEntry instead of catalog lookup - FTSIndex::load() now uses catalogEntry instead of catalog lookup - Implement parallel loading in vector_extension with thread pool - Add cancellation support for background index loading - Eliminates catalog access during index loading
- Add vectorIndexLoaderThread to Database for proper thread ownership - Implement startVectorIndexLoader() and joinVectorIndexLoaderThread() - Add callback API for vector index loading completion notification - Add cancellation support with vectorIndexLoadCancelled flag - Add backgroundThreadStartMutex to prevent TOCTOU race - Move extension autoload after WAL recovery completes - Ensures clean shutdown without use-after-free
- Serialize/deserialize lastTimestamp in DatabaseHeader - Restore TransactionManager state after checkpoint recovery - Move extension autoload after WAL recovery completes - Prevents timestamp conflicts when reopening database - Maintains backward compatibility with older database files
- Add null check for catalog entries after version chain traversal - Add MAX_ITERATIONS limit to prevent infinite loops in version chains - Fix catalog entry validation in getEntries() - Add null guard in transaction catalog entry creation
- Add vector_parallel_loading_test to CMake - Change dlopen to RTLD_GLOBAL for proper symbol resolution - Add getExtensionCatalogTransaction() and getExtensionCheckpointTransaction() - Fix extension function registration to use proper transaction contexts
- Update OnDiskHNSWIndex::load() signature in header - Update FTSIndex::load() signature in header - Consistency with implementation changes
- FTSExtension now passes catalogEntry to index.load() - Fix includes for extension.h in FTS and HNSW implementations - Update extension CMakeLists for proper library dependencies
- Mark notifyVectorIndexLoadComplete as KUZU_API - Mark startVectorIndexLoader as KUZU_API - Mark TransactionManager public methods as KUZU_API - Required for proper symbol export in shared libraries
This commit ensures HNSW indexes are loaded synchronously during WAL replay (recovery) to prevent race conditions where WAL records access indexes before background loading completes. Changes: - Add isRecoveryInProgress flag to DatabaseLifeCycleManager - VectorExtension detects recovery mode and loads indexes synchronously - Extract loadHNSWIndexesSync() function for both sync and async paths - Add index loaded assertions in NodeTable insert/delete operations - Add parallel_loading_test to verify recovery behavior Fixes race condition where NodeInsertionRecord/NodeDeletionRecord could access unloaded indexes during WAL replay.
This commit adds synchronization to ensure vector indexes are loaded before INSERT and COPY FROM operations that require index access. Changes: - VectorExtension: Add static-link detection for synchronous loading in test environments - Binder: Wait for vector index loading in bindInsertNode() - Binder: Wait for vector index loading in bindCopyNodeFrom() - Add 30-second timeout with 100ms polling interval - Graceful error handling if loading fails or times out This prevents race conditions where INSERT/COPY operations execute before background index loading completes in dynamic-load scenarios.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements parallel loading support for HNSW indexes in the vector extension, addressing critical thread safety and lifecycle management issues.
Key Changes
Thread-Safe HNSW Index Loading
DatabaseLifeCycleManagerto prevent TOCTOU race conditionsCatalog Entry Propagation
Index::load()interface to acceptCatalogEntry*parameterTransaction Timestamp Persistence
lastTimestamppersistence in database headerIndex Loading Synchronization
Bug Fixes
Test Coverage
parallel_loading_test.cppfor HNSW parallel loading scenariosoverflow_file_checkpoint_test.cppfor checkpoint edge casesRelated Issues