From 85aded68932141ecfa5b57ff03735890fe2f7823 Mon Sep 17 00:00:00 2001 From: hemant-endee Date: Wed, 25 Mar 2026 17:18:36 +0530 Subject: [PATCH 01/16] Rebuild index with new config (#136) * Rebuild index with new config * fix 1 * index name in get stattu api correction * docs changes * Rebuild Status Persistence --- docs/rebuild.md | 124 ++++++++++++++++++++ src/core/ndd.hpp | 272 ++++++++++++++++++++++++++++++++++++++++++- src/core/rebuild.hpp | 130 +++++++++++++++++++++ src/main.cpp | 95 +++++++++++++++ 4 files changed, 618 insertions(+), 3 deletions(-) create mode 100644 docs/rebuild.md create mode 100644 src/core/rebuild.hpp diff --git a/docs/rebuild.md b/docs/rebuild.md new file mode 100644 index 0000000000..be6b35640f --- /dev/null +++ b/docs/rebuild.md @@ -0,0 +1,124 @@ +# Index Rebuild + +Rebuild allows you to reconstruct an HNSW index graph with new configuration parameters (M, ef_construction) without re-uploading vector data. All vectors are re-indexed from MDBX storage — only the graph structure is rebuilt. + +## API Endpoints + +| Method | Endpoint | Description | +|--------|----------|-------------| +| POST | `/api/v1/index/{name}/rebuild` | Start async rebuild | +| GET | `/api/v1/index/{name}/rebuild/status` | Check rebuild progress | + +--- + +## Start Rebuild + +**POST** `/api/v1/index/{name}/rebuild` + +All parameters are optional. Omitted parameters retain their current values. + +```json +{ + "M": 32, + "ef_con": 256 +} +``` + +**Parameters:** + +| Parameter | Type | Description | +|-----------|------|-------------| +| `M` | int | HNSW graph connectivity (4–512) | +| `ef_con` | int | Construction-time search quality (8–4096) | + +**Response 202:** +```json +{ + "status": "rebuilding", + "previous_config": { "M": 16, "ef_con": 128 }, + "new_config": { "M": 32, "ef_con": 256 }, + "total_vectors": 50000 +} +``` + +**Errors:** + +| Code | Condition | +|------|-----------| +| 400 | No changes specified, invalid parameters, or attempted to change `precision`/`space_type` | +| 404 | Index not found | +| 409 | Rebuild or backup already in progress for this user | + +--- + +## Check Progress + +**GET** `/api/v1/index/{name}/rebuild/status` + +**Status values:** + +| Status | Meaning | +|--------|---------| +| `idle` | No rebuild has run for this index (or querying a different index) | +| `in_progress` | Rebuild is currently running | +| `completed` | Rebuild finished successfully | +| `failed` | Rebuild failed (see `error` field) | + +**In progress:** +```json +{ + "status": "in_progress", + "vectors_processed": 45000, + "total_vectors": 100000, + "percent_complete": 45.0, + "started_at": "2026-03-25T10:30:00Z" +} +``` + +**Completed:** +```json +{ + "status": "completed", + "vectors_processed": 100000, + "total_vectors": 100000, + "percent_complete": 100.0, + "started_at": "2026-03-25T10:30:00Z", + "completed_at": "2026-03-25T10:32:15Z" +} +``` + +**Failed:** +```json +{ + "status": "failed", + "vectors_processed": 45000, + "total_vectors": 100000, + "percent_complete": 45.0, + "started_at": "2026-03-25T10:30:00Z", + "completed_at": "2026-03-25T10:31:05Z", + "error": "Out of memory" +} +``` + +Status is per-index. The `completed`/`failed` state persists until the next rebuild is started for that user. + +--- + +## Restrictions + +The following parameters **cannot** be changed via rebuild (returns 400): +- `precision` (quantization level) +- `space_type` + + +--- + +## Behavior + +- **All vectors are re-indexed** from MDBX storage into a new HNSW graph with the updated configuration. +- **Search continues** during rebuild — queries use the old index until the rebuild completes. +- **Write operations** (insert, delete, update) will block and timeout while the rebuild is running, same as during backup. +- **One rebuild at a time per user** — cannot start a rebuild on any index while another rebuild is in progress for the same user. Also cannot run concurrently with a backup. +- **Periodic checkpoints** — the in-progress graph is saved to a temp file at regular intervals. +- **On completion**, the new graph replaces `default.idx`. All temporary and intermediate files are cleaned up. +- **On server restart** during an incomplete rebuild, the old index loads normally. Temp files are cleaned up automatically. The rebuild must be restarted manually. diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index 55f6e5bc57..3f89592790 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -197,6 +197,7 @@ struct PersistenceConfig { }; #include "../storage/backup_store.hpp" +#include "rebuild.hpp" class IndexManager { private: @@ -220,8 +221,10 @@ class IndexManager { std::thread autosave_thread_; std::atomic running_{true}; BackupStore backup_store_; - void executeBackupJob(const std::string& index_id, const std::string& backup_name, - std::stop_token st); + Rebuild rebuild_; + void executeBackupJob(const std::string& index_id, const std::string& backup_name); + void executeRebuildJob(const std::string& index_id, const std::string& username, + size_t new_M, size_t new_ef_con); std::unique_ptr createWAL(const std::string& index_id) { const std::string wal_dir = data_dir_ + "/" + index_id; @@ -581,6 +584,7 @@ class IndexManager { backup_store_(data_dir) { std::filesystem::create_directories(data_dir); metadata_manager_ = std::make_unique(data_dir); + rebuild_.cleanupTempFiles(data_dir); // Start the autosave thread autosave_thread_ = std::thread(&IndexManager::autosaveLoop, this); } @@ -1925,9 +1929,59 @@ class IndexManager { return backup_store_.validateBackupName(backup_name); } +<<<<<<< HEAD std::pair uploadBackup(const std::string& backup_name, const std::string& username, const std::string& file_content); +======= + // Metadata access + std::optional getMetadata(const std::string& index_id) { + return metadata_manager_->getMetadata(index_id); + } + + // Index stats (safe to call from routes) + size_t getElementCount(const std::string& index_id) { + auto& entry = getIndexEntry(index_id); + return entry.alg->getElementsCount(); + } + + + // ========== Rebuild operations ========== + + // Orchestration method (defined below after class) + std::pair rebuildIndexAsync(const std::string& index_id, + size_t new_M, + size_t new_ef_con); + + bool hasActiveRebuild(const std::string& username) const { + return rebuild_.hasActiveRebuild(username); + } + + nlohmann::json getRebuildProgress(const std::string& username, + const std::string& index_id) const { + auto state = rebuild_.getActiveRebuild(username); + if (state && state->index_id == index_id) { + size_t processed = state->vectors_processed.load(); + size_t total = state->total_vectors.load(); + double percent = total > 0 ? (100.0 * processed / total) : 0.0; + nlohmann::json result = { + {"status", state->status}, + {"vectors_processed", processed}, + {"total_vectors", total}, + {"percent_complete", percent}, + {"started_at", Rebuild::formatTime(state->started_at)} + }; + if (state->status == "completed" || state->status == "failed") { + result["completed_at"] = Rebuild::formatTime(state->completed_at); + } + if (state->status == "failed" && !state->error_message.empty()) { + result["error"] = state->error_message; + } + return result; + } + return {{"status", "idle"}}; + } +>>>>>>> e66b946 (Rebuild index with new config (#136)) }; // ========== IndexManager backup implementations ========== @@ -2221,6 +2275,7 @@ inline std::pair IndexManager::createBackupAsync(const std::s return {true, backup_name}; } + inline std::pair IndexManager::uploadBackup(const std::string& backup_name, const std::string& username, const std::string& file_content) { std::string user_backup_dir = backup_store_.getUserBackupDir(username); std::filesystem::create_directories(user_backup_dir); @@ -2280,4 +2335,215 @@ inline std::pair IndexManager::uploadBackup(const std::string backup_store_.writeBackupJson(username, backup_db); return {true, "Backup uploaded successfully"}; -} \ No newline at end of file +} + +// ========== IndexManager rebuild implementations ========== + +inline std::pair IndexManager::rebuildIndexAsync(const std::string& index_id, + size_t new_M, + size_t new_ef_con) { + // Validate index exists + auto meta = metadata_manager_->getMetadata(index_id); + if (!meta) { + return {false, "Index not found"}; + } + + // Extract username for backup check + std::string username; + size_t pos = index_id.find('/'); + if (pos != std::string::npos) { + username = index_id.substr(0, pos); + } else { + return {false, "Invalid index ID format"}; + } + + // Check for active backup or rebuild + if (backup_store_.hasActiveBackup(username)) { + return {false, "Backup already in progress for user: " + username}; + } + if (rebuild_.hasActiveRebuild(username)) { + return {false, "Rebuild already in progress for user: " + username}; + } + + // Load entry to get current element count + auto& entry = getIndexEntry(index_id); + size_t current_count = entry.alg->getElementsCount(); + + // Ensure at least one parameter differs + if (new_M == meta->M && new_ef_con == meta->ef_con) { + return {false, "No configuration changes specified"}; + } + + // Set active rebuild state (per-user, one rebuild at a time) + rebuild_.setActiveRebuild(username, index_id, current_count); + + // Spawn background thread (same pattern as createBackupAsync) + std::thread([this, index_id, username, new_M, new_ef_con]() { + executeRebuildJob(index_id, username, new_M, new_ef_con); + }).detach(); + + LOG_INFO(2050, index_id, "Rebuild started: M=" << new_M + << " ef_con=" << new_ef_con); + + return {true, "Rebuild started"}; +} + +inline void IndexManager::executeRebuildJob(const std::string& index_id, + const std::string& username, + size_t new_M, size_t new_ef_con) { + std::string base_path = data_dir_ + "/" + index_id; + std::string temp_path = rebuild_.getTempPath(base_path); + std::string timestamped_path = rebuild_.getTimestampedPath(base_path); + std::string vector_storage_dir = base_path + "/vectors"; + std::string index_path = vector_storage_dir + "/" + settings::DEFAULT_SUBINDEX + ".idx"; + + try { + auto& entry = getIndexEntry(index_id); + + // Hold operation_mutex for entire rebuild — writes timeout, searches continue + std::lock_guard operation_lock(entry.operation_mutex); + + // Phase 1 — Save current state + saveIndexInternal(entry); + + // Read current config from the existing HNSW graph + auto space_type = entry.alg->getSpaceType(); + size_t dim = entry.alg->getDimension(); + auto quant_level = entry.alg->getQuantLevel(); + int32_t checksum = entry.alg->getChecksum(); + size_t max_elements = entry.alg->getMaxElements(); + + // Phase 2 — Build new HNSW (same max_elements as current index) + auto new_alg = std::make_unique>( + max_elements, space_type, dim, new_M, new_ef_con, + settings::RANDOM_SEED, quant_level, checksum); + + // Set vector fetcher BEFORE adding vectors — searchBaseLayer during + // graph construction needs this to compute distances for base-layer-only + // nodes (base layer doesn't store vector data inline) + new_alg->setVectorFetcher([vs = entry.vector_storage](ndd::idInt label, uint8_t* buffer) { + return vs->get_vector(label, buffer); + }); + + new_alg->setVectorFetcherBatch([vs = entry.vector_storage](const ndd::idInt* labels, + uint8_t* buffers, + bool* success, + size_t count) -> size_t { + return vs->get_vectors_batch_into(labels, buffers, success, count); + }); + + // Iterate VectorStore and re-insert all vectors + auto cursor = entry.vector_storage->getCursor(); + const size_t batch_size = settings::RECOVERY_BATCH_SIZE; + size_t total_processed = 0; + size_t batches_since_checkpoint = 0; + constexpr size_t CHECKPOINT_INTERVAL = 5; // Save temp every 5 batches + + while (cursor.hasNext()) { + // Collect batch + std::vector>> batch; + batch.reserve(batch_size); + while (cursor.hasNext() && batch.size() < batch_size) { + auto [label, vec_bytes] = cursor.next(); + if (!vec_bytes.empty()) { + batch.emplace_back(label, std::move(vec_bytes)); + } + } + + if (batch.empty()) { + break; + } + + // Multi-threaded insert (same pattern as addVectors and recoverIndex) + size_t num_threads = std::min(settings::NUM_RECOVERY_THREADS, batch.size()); + std::atomic next{0}; + std::vector threads; + + for (size_t t = 0; t < num_threads; ++t) { + threads.emplace_back([&]() { + size_t i; + while ((i = next.fetch_add(1)) < batch.size()) { + const auto& [label, vec_bytes] = batch[i]; + new_alg->addPoint(vec_bytes.data(), label); + } + }); + } + + for (auto& th : threads) { + th.join(); + } + + total_processed += batch.size(); + + // Update progress + auto state = rebuild_.getActiveRebuild(username); + if (state) { + state->vectors_processed.store(total_processed); + } + + // Periodic checkpoint save + batches_since_checkpoint++; + if (batches_since_checkpoint >= CHECKPOINT_INTERVAL) { + new_alg->saveIndex(temp_path); + batches_since_checkpoint = 0; + } + } + + // Phase 3 — Save final + Copy + Swap + + // Save new graph to timestamped file + new_alg->saveIndex(timestamped_path); + + // Copy to canonical name (overwrites old default.idx on disk) + std::filesystem::copy_file(timestamped_path, index_path, + std::filesystem::copy_options::overwrite_existing); + + // Load fresh from disk + swap pointer (reloadIndex pattern) + auto fresh_alg = std::make_unique>(index_path, 0); + + fresh_alg->setVectorFetcher([vs = entry.vector_storage](ndd::idInt label, uint8_t* buffer) { + return vs->get_vector(label, buffer); + }); + + fresh_alg->setVectorFetcherBatch([vs = entry.vector_storage](const ndd::idInt* labels, + uint8_t* buffers, + bool* success, + size_t count) -> size_t { + return vs->get_vectors_batch_into(labels, buffers, success, count); + }); + + entry.alg = std::move(fresh_alg); + + // Delete temp checkpoint and timestamped file + if (std::filesystem::exists(temp_path)) { + std::filesystem::remove(temp_path); + } + if (std::filesystem::exists(timestamped_path)) { + std::filesystem::remove(timestamped_path); + } + + // Update metadata with new config + auto meta = metadata_manager_->getMetadata(index_id); + if (meta) { + meta->M = new_M; + meta->ef_con = new_ef_con; + meta->total_elements = entry.alg->getElementsCount(); + metadata_manager_->storeMetadata(index_id, *meta); + } + + entry.markUpdated(); + entry.updated = false; // We just saved the new graph + + LOG_INFO(2051, index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); + rebuild_.completeActiveRebuild(username); + + } catch (const std::exception& e) { + LOG_ERROR(2052, index_id, "Rebuild failed: " << e.what()); + + // Cleanup temp file on error + if (std::filesystem::exists(temp_path)) { + std::filesystem::remove(temp_path); + } + rebuild_.failActiveRebuild(username, e.what()); + } +} diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp new file mode 100644 index 0000000000..27777fa571 --- /dev/null +++ b/src/core/rebuild.hpp @@ -0,0 +1,130 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "settings.hpp" +#include "log.hpp" + +struct ActiveRebuild { + std::string index_id; + std::string status{"in_progress"}; // "in_progress", "completed", "failed" + std::string error_message; + std::atomic vectors_processed{0}; + std::atomic total_vectors{0}; + std::chrono::system_clock::time_point started_at; + std::chrono::system_clock::time_point completed_at; +}; + +class Rebuild { +private: + // Keyed by username — one rebuild per user at a time + std::unordered_map> active_rebuilds_; + mutable std::mutex rebuild_state_mutex_; + + static std::string timeToISO8601(std::chrono::system_clock::time_point tp) { + auto time_t_val = std::chrono::system_clock::to_time_t(tp); + std::tm tm_val{}; + gmtime_r(&time_t_val, &tm_val); + std::ostringstream oss; + oss << std::put_time(&tm_val, "%Y-%m-%dT%H:%M:%SZ"); + return oss.str(); + } + +public: + Rebuild() = default; + + // Lifecycle — cleanup temp files from interrupted rebuilds on startup + void cleanupTempFiles(const std::string& data_dir) { + if (!std::filesystem::exists(data_dir)) { + return; + } + try { + std::string temp_filename = std::string(settings::DEFAULT_SUBINDEX) + ".idx.temp"; + for (const auto& entry : std::filesystem::recursive_directory_iterator(data_dir)) { + if (entry.is_regular_file() && + entry.path().filename().string() == temp_filename) { + std::filesystem::remove(entry.path()); + } + } + } catch (const std::exception&) { + // Silently ignore cleanup errors on startup + } + } + + // State tracking — per user + + void setActiveRebuild(const std::string& username, const std::string& index_id, + size_t total_vectors) { + std::lock_guard lock(rebuild_state_mutex_); + auto state = std::make_shared(); + state->index_id = index_id; + state->status = "in_progress"; + state->total_vectors.store(total_vectors); + state->vectors_processed.store(0); + state->started_at = std::chrono::system_clock::now(); + active_rebuilds_[username] = state; + } + + void completeActiveRebuild(const std::string& username) { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + it->second->status = "completed"; + it->second->completed_at = std::chrono::system_clock::now(); + } + } + + void failActiveRebuild(const std::string& username, const std::string& error) { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + it->second->status = "failed"; + it->second->error_message = error; + it->second->completed_at = std::chrono::system_clock::now(); + } + } + + bool hasActiveRebuild(const std::string& username) const { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + // Only "in_progress" blocks a new rebuild + return it != active_rebuilds_.end() && it->second->status == "in_progress"; + } + + std::shared_ptr getActiveRebuild(const std::string& username) const { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + return it->second; + } + return nullptr; + } + + // Format state as JSON fields + static std::string formatTime(std::chrono::system_clock::time_point tp) { + return timeToISO8601(tp); + } + + // Path helpers + + static std::string getTempPath(const std::string& index_dir) { + return index_dir + "/vectors/" + settings::DEFAULT_SUBINDEX + ".idx.temp"; + } + + static std::string getTimestampedPath(const std::string& index_dir) { + auto ts = std::to_string( + std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch() + ).count() + ); + return index_dir + "/vectors/" + settings::DEFAULT_SUBINDEX + ".idx." + ts; + } +}; diff --git a/src/main.cpp b/src/main.cpp index 4654a54c20..7821451ae8 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -692,6 +692,101 @@ int main(int argc, char** argv) { } }); + // ========== Rebuild operations ========== + + // Start index rebuild + CROW_ROUTE(app, "/api/v1/index//rebuild") + .CROW_MIDDLEWARES(app, AuthMiddleware) + .methods("POST"_method)([&index_manager, &app](const crow::request& req, + const std::string& index_name) { + auto& ctx = app.get_context(req); + std::string index_id = ctx.username + "/" + index_name; + + auto body = crow::json::load(req.body); + if (!body) { + return json_error(400, "Invalid JSON"); + } + + // Reject parameters that cannot be changed via rebuild + if (body.has("precision")) { + return json_error(400, "precision cannot be changed via rebuild"); + } + if (body.has("space_type")) { + return json_error(400, "space_type cannot be changed via rebuild"); + } + + // Get current metadata for defaults + auto meta = index_manager.getMetadata(index_id); + if (!meta) { + return json_error(404, "Index not found"); + } + + // Parse parameters with current values as defaults + size_t new_M = body.has("M") ? (size_t)body["M"].i() : meta->M; + size_t new_ef_con = body.has("ef_con") ? (size_t)body["ef_con"].i() : meta->ef_con; + + // Validate M + if (new_M < settings::MIN_M || new_M > settings::MAX_M) { + return json_error(400, + "M must be between " + std::to_string(settings::MIN_M) + + " and " + std::to_string(settings::MAX_M)); + } + + // Validate ef_con + if (new_ef_con < settings::MIN_EF_CONSTRUCT || new_ef_con > settings::MAX_EF_CONSTRUCT) { + return json_error(400, + "ef_con must be between " + std::to_string(settings::MIN_EF_CONSTRUCT) + + " and " + std::to_string(settings::MAX_EF_CONSTRUCT)); + } + + // Get actual vector count for response + size_t actual_element_count = 0; + try { + actual_element_count = index_manager.getElementCount(index_id); + } catch (...) {} + + try { + auto [success, message] = index_manager.rebuildIndexAsync( + index_id, new_M, new_ef_con); + + if (!success) { + int code = (message.find("already in progress") != std::string::npos) ? 409 : 400; + return json_error(code, message); + } + + crow::json::wvalue response; + response["status"] = "rebuilding"; + response["previous_config"]["M"] = meta->M; + response["previous_config"]["ef_con"] = meta->ef_con; + response["new_config"]["M"] = new_M; + response["new_config"]["ef_con"] = new_ef_con; + response["total_vectors"] = actual_element_count; + return crow::response(202, response.dump()); + } catch (const std::exception& e) { + return json_error_500(ctx.username, index_name, req.url, e.what()); + } + }); + + // Get rebuild status + CROW_ROUTE(app, "/api/v1/index//rebuild/status") + .CROW_MIDDLEWARES(app, AuthMiddleware) + .methods("GET"_method)([&index_manager, &app](const crow::request& req, + const std::string& index_name) { + auto& ctx = app.get_context(req); + std::string index_id = ctx.username + "/" + index_name; + + try { + auto progress = index_manager.getRebuildProgress(ctx.username, index_id); + crow::response res; + res.code = 200; + res.set_header("Content-Type", "application/json"); + res.body = progress.dump(); + return res; + } catch (const std::exception& e) { + return json_error_500(ctx.username, index_name, req.url, e.what()); + } + }); + // List indexes for current user CROW_ROUTE(app, "/api/v1/index/list") .CROW_MIDDLEWARES(app, AuthMiddleware) From bb2f3d6db53f36f59997c9b0d7f231c67c4dbcce Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Wed, 8 Apr 2026 16:40:22 +0530 Subject: [PATCH 02/16] using jthread with stop token --- src/core/ndd.hpp | 32 ++++++++++++++++++++------------ src/core/rebuild.hpp | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index 3f89592790..046254da7e 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -222,9 +222,9 @@ class IndexManager { std::atomic running_{true}; BackupStore backup_store_; Rebuild rebuild_; - void executeBackupJob(const std::string& index_id, const std::string& backup_name); + void executeBackupJob(const std::string& index_id, const std::string& backup_name, std::stop_token st); void executeRebuildJob(const std::string& index_id, const std::string& username, - size_t new_M, size_t new_ef_con); + size_t new_M, size_t new_ef_con, std::stop_token st); std::unique_ptr createWAL(const std::string& index_id) { const std::string wal_dir = data_dir_ + "/" + index_id; @@ -593,9 +593,10 @@ class IndexManager { // Signal all threads to stop (running_ is checked by autosave and backup threads) running_ = false; - // Join background backup threads before destroying members - // (prevents use-after-free when detached threads outlive IndexManager) + // Join background backup and rebuild threads before destroying members + // (prevents use-after-free when threads outlive IndexManager) backup_store_.joinAllThreads(); + rebuild_.joinAllThreads(); /** * Don't wait for autosave thread to exit. @@ -2374,13 +2375,11 @@ inline std::pair IndexManager::rebuildIndexAsync(const std::s return {false, "No configuration changes specified"}; } - // Set active rebuild state (per-user, one rebuild at a time) - rebuild_.setActiveRebuild(username, index_id, current_count); - - // Spawn background thread (same pattern as createBackupAsync) - std::thread([this, index_id, username, new_M, new_ef_con]() { - executeRebuildJob(index_id, username, new_M, new_ef_con); - }).detach(); + // Set active rebuild state and spawn jthread (same pattern as createBackupAsync) + std::jthread t([this, index_id, username, new_M, new_ef_con](std::stop_token st) { + executeRebuildJob(index_id, username, new_M, new_ef_con, st); + }); + rebuild_.setActiveRebuild(username, index_id, current_count, std::move(t)); LOG_INFO(2050, index_id, "Rebuild started: M=" << new_M << " ef_con=" << new_ef_con); @@ -2390,7 +2389,8 @@ inline std::pair IndexManager::rebuildIndexAsync(const std::s inline void IndexManager::executeRebuildJob(const std::string& index_id, const std::string& username, - size_t new_M, size_t new_ef_con) { + size_t new_M, size_t new_ef_con, + std::stop_token st) { std::string base_path = data_dir_ + "/" + index_id; std::string temp_path = rebuild_.getTempPath(base_path); std::string timestamped_path = rebuild_.getTimestampedPath(base_path); @@ -2440,6 +2440,14 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, constexpr size_t CHECKPOINT_INTERVAL = 5; // Save temp every 5 batches while (cursor.hasNext()) { + if (st.stop_requested()) { + if (std::filesystem::exists(temp_path)) { + std::filesystem::remove(temp_path); + } + rebuild_.failActiveRebuild(username, "Rebuild interrupted by server shutdown"); + return; + } + // Collect batch std::vector>> batch; batch.reserve(batch_size); diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index 27777fa571..98d10192f4 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -5,10 +5,12 @@ #include #include #include +#include #include #include #include #include +#include #include "settings.hpp" #include "log.hpp" @@ -21,6 +23,7 @@ struct ActiveRebuild { std::atomic total_vectors{0}; std::chrono::system_clock::time_point started_at; std::chrono::system_clock::time_point completed_at; + std::jthread thread; // jthread: built-in stop_token + auto-join on destruction }; class Rebuild { @@ -62,7 +65,7 @@ class Rebuild { // State tracking — per user void setActiveRebuild(const std::string& username, const std::string& index_id, - size_t total_vectors) { + size_t total_vectors, std::jthread&& thread) { std::lock_guard lock(rebuild_state_mutex_); auto state = std::make_shared(); state->index_id = index_id; @@ -70,6 +73,7 @@ class Rebuild { state->total_vectors.store(total_vectors); state->vectors_processed.store(0); state->started_at = std::chrono::system_clock::now(); + state->thread = std::move(thread); active_rebuilds_[username] = state; } @@ -77,6 +81,10 @@ class Rebuild { std::lock_guard lock(rebuild_state_mutex_); auto it = active_rebuilds_.find(username); if (it != active_rebuilds_.end()) { + // Called from within the thread — detach so the jthread dtor doesn't join us + if (it->second->thread.joinable()) { + it->second->thread.detach(); + } it->second->status = "completed"; it->second->completed_at = std::chrono::system_clock::now(); } @@ -86,6 +94,10 @@ class Rebuild { std::lock_guard lock(rebuild_state_mutex_); auto it = active_rebuilds_.find(username); if (it != active_rebuilds_.end()) { + // Called from within the thread — detach so the jthread dtor doesn't join us + if (it->second->thread.joinable()) { + it->second->thread.detach(); + } it->second->status = "failed"; it->second->error_message = error; it->second->completed_at = std::chrono::system_clock::now(); @@ -99,6 +111,28 @@ class Rebuild { return it != active_rebuilds_.end() && it->second->status == "in_progress"; } + // Join all in-progress rebuild threads on shutdown. Mirrors BackupStore::joinAllThreads: + // move threads out under lock, request_stop + join outside lock to avoid deadlock + // (finishing threads call completeActiveRebuild which also locks rebuild_state_mutex_). + void joinAllThreads() { + std::vector threads_to_join; + { + std::lock_guard lock(rebuild_state_mutex_); + for (auto& [username, state] : active_rebuilds_) { + if (state->thread.joinable()) { + threads_to_join.push_back(std::move(state->thread)); + } + } + active_rebuilds_.clear(); + } + for (auto& t : threads_to_join) { + t.request_stop(); + if (t.joinable()) { + t.join(); + } + } + } + std::shared_ptr getActiveRebuild(const std::string& username) const { std::lock_guard lock(rebuild_state_mutex_); auto it = active_rebuilds_.find(username); From cbaa7445e57b0a77860f32348253857867d61c83 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Thu, 9 Apr 2026 14:20:31 +0530 Subject: [PATCH 03/16] =?UTF-8?q?Shared=20parallel=20addPoint=20utility=20?= =?UTF-8?q?function=20=E2=80=94=20static=20chunk=20partition(same=20as=20a?= =?UTF-8?q?ddVectors)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/core/ndd.hpp | 134 ++++++++++++++++++++--------------------------- 1 file changed, 57 insertions(+), 77 deletions(-) diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index 046254da7e..95043d0b2d 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -226,6 +226,31 @@ class IndexManager { void executeRebuildJob(const std::string& index_id, const std::string& username, size_t new_M, size_t new_ef_con, std::stop_token st); + // Shared parallel addPoint utility — static chunk partition (same as addVectors). + // ProcessFn signature: void(size_t index) + template + static void parallelAddPoints(size_t count, size_t max_threads, ProcessFn&& process) { + if (count == 0) return; + size_t num_threads = std::min(max_threads, count); + const size_t chunk_size = (count + num_threads - 1) / num_threads; // Ceiling division + std::vector threads; + threads.reserve(num_threads); + + for (size_t t = 0; t < num_threads; ++t) { + threads.emplace_back([&, t]() { + size_t start_idx = t * chunk_size; + size_t end_idx = std::min(start_idx + chunk_size, count); + for (size_t i = start_idx; i < end_idx; ++i) { + process(i); + } + }); + } + + for (auto& th : threads) { + th.join(); + } + } + std::unique_ptr createWAL(const std::string& index_id) { const std::string wal_dir = data_dir_ + "/" + index_id; return std::make_unique(wal_dir, index_id); @@ -1113,47 +1138,15 @@ class IndexManager { logInsertsAndUpdates(entry, numeric_ids); // Add to HNSW index in parallel using pre-quantized data from QuantVectorObject - size_t available_threads = settings::NUM_PARALLEL_INSERTS; - const size_t num_threads = (available_threads < quantized_vectors.size()) - ? available_threads - : quantized_vectors.size(); - std::vector threads; - const size_t chunk_size = - (quantized_vectors.size() + num_threads - 1) / num_threads; // Ceiling division - - threads.reserve(num_threads); - for(size_t t = 0; t < num_threads; t++) { - threads.emplace_back([&, t]() { - // Calculate start and end indices for this thread - size_t start_idx = t * chunk_size; - size_t end_idx = (start_idx + chunk_size < quantized_vectors.size()) - ? (start_idx + chunk_size) - : quantized_vectors.size(); - - // Process assigned chunk of vectors - for(size_t i = start_idx; i < end_idx; i++) { - const auto& quant_vec_obj = quantized_vectors[i]; - - // Use pre-quantized data directly from QuantVectorObject - no conversion - // needed! - const uint8_t* vector_data = quant_vec_obj.quant_vector.data(); - - // Add to HNSW index using pre-quantized raw bytes - if(numeric_ids[i].second) { - // If it's a new ID, add it to the index - entry.alg->addPoint(vector_data, numeric_ids[i].first); - } else { - // If it's an update, add it to the index - entry.alg->addPoint(vector_data, numeric_ids[i].first); - } + parallelAddPoints(quantized_vectors.size(), settings::NUM_PARALLEL_INSERTS, + [&](size_t i) { + const uint8_t* vector_data = quantized_vectors[i].quant_vector.data(); + if(numeric_ids[i].second) { + entry.alg->addPoint(vector_data, numeric_ids[i].first); + } else { + entry.alg->addPoint(vector_data, numeric_ids[i].first); } }); - } - - // Wait for all threads to complete - for(auto& thread : threads) { - thread.join(); - } entry.markDirty(); @@ -1942,8 +1935,8 @@ class IndexManager { // Index stats (safe to call from routes) size_t getElementCount(const std::string& index_id) { - auto& entry = getIndexEntry(index_id); - return entry.alg->getElementsCount(); + auto entry = getIndexEntry(index_id); + return entry->alg->getElementsCount(); } @@ -2367,8 +2360,8 @@ inline std::pair IndexManager::rebuildIndexAsync(const std::s } // Load entry to get current element count - auto& entry = getIndexEntry(index_id); - size_t current_count = entry.alg->getElementsCount(); + auto entry = getIndexEntry(index_id); + size_t current_count = entry->alg->getElementsCount(); // Ensure at least one parameter differs if (new_M == meta->M && new_ef_con == meta->ef_con) { @@ -2398,20 +2391,20 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, std::string index_path = vector_storage_dir + "/" + settings::DEFAULT_SUBINDEX + ".idx"; try { - auto& entry = getIndexEntry(index_id); + auto entry = getIndexEntry(index_id); // Hold operation_mutex for entire rebuild — writes timeout, searches continue - std::lock_guard operation_lock(entry.operation_mutex); + std::unique_lock operation_lock(entry->operation_mutex); // Phase 1 — Save current state - saveIndexInternal(entry); + saveIndexInternal(*entry); // Read current config from the existing HNSW graph - auto space_type = entry.alg->getSpaceType(); - size_t dim = entry.alg->getDimension(); - auto quant_level = entry.alg->getQuantLevel(); - int32_t checksum = entry.alg->getChecksum(); - size_t max_elements = entry.alg->getMaxElements(); + auto space_type = entry->alg->getSpaceType(); + size_t dim = entry->alg->getDimension(); + auto quant_level = entry->alg->getQuantLevel(); + int32_t checksum = entry->alg->getChecksum(); + size_t max_elements = entry->alg->getMaxElements(); // Phase 2 — Build new HNSW (same max_elements as current index) auto new_alg = std::make_unique>( @@ -2421,11 +2414,11 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, // Set vector fetcher BEFORE adding vectors — searchBaseLayer during // graph construction needs this to compute distances for base-layer-only // nodes (base layer doesn't store vector data inline) - new_alg->setVectorFetcher([vs = entry.vector_storage](ndd::idInt label, uint8_t* buffer) { + new_alg->setVectorFetcher([vs = entry->vector_storage](ndd::idInt label, uint8_t* buffer) { return vs->get_vector(label, buffer); }); - new_alg->setVectorFetcherBatch([vs = entry.vector_storage](const ndd::idInt* labels, + new_alg->setVectorFetcherBatch([vs = entry->vector_storage](const ndd::idInt* labels, uint8_t* buffers, bool* success, size_t count) -> size_t { @@ -2433,7 +2426,7 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, }); // Iterate VectorStore and re-insert all vectors - auto cursor = entry.vector_storage->getCursor(); + auto cursor = entry->vector_storage->getCursor(); const size_t batch_size = settings::RECOVERY_BATCH_SIZE; size_t total_processed = 0; size_t batches_since_checkpoint = 0; @@ -2462,24 +2455,12 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, break; } - // Multi-threaded insert (same pattern as addVectors and recoverIndex) - size_t num_threads = std::min(settings::NUM_RECOVERY_THREADS, batch.size()); - std::atomic next{0}; - std::vector threads; - - for (size_t t = 0; t < num_threads; ++t) { - threads.emplace_back([&]() { - size_t i; - while ((i = next.fetch_add(1)) < batch.size()) { - const auto& [label, vec_bytes] = batch[i]; - new_alg->addPoint(vec_bytes.data(), label); - } + // Multi-threaded insert (shared utility with addVectors) + parallelAddPoints(batch.size(), settings::NUM_PARALLEL_INSERTS, + [&](size_t i) { + const auto& [label, vec_bytes] = batch[i]; + new_alg->addPoint(vec_bytes.data(), label); }); - } - - for (auto& th : threads) { - th.join(); - } total_processed += batch.size(); @@ -2509,18 +2490,18 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, // Load fresh from disk + swap pointer (reloadIndex pattern) auto fresh_alg = std::make_unique>(index_path, 0); - fresh_alg->setVectorFetcher([vs = entry.vector_storage](ndd::idInt label, uint8_t* buffer) { + fresh_alg->setVectorFetcher([vs = entry->vector_storage](ndd::idInt label, uint8_t* buffer) { return vs->get_vector(label, buffer); }); - fresh_alg->setVectorFetcherBatch([vs = entry.vector_storage](const ndd::idInt* labels, + fresh_alg->setVectorFetcherBatch([vs = entry->vector_storage](const ndd::idInt* labels, uint8_t* buffers, bool* success, size_t count) -> size_t { return vs->get_vectors_batch_into(labels, buffers, success, count); }); - entry.alg = std::move(fresh_alg); + entry->alg = std::move(fresh_alg); // Delete temp checkpoint and timestamped file if (std::filesystem::exists(temp_path)) { @@ -2535,12 +2516,11 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, if (meta) { meta->M = new_M; meta->ef_con = new_ef_con; - meta->total_elements = entry.alg->getElementsCount(); + meta->total_elements = entry->alg->getElementsCount(); metadata_manager_->storeMetadata(index_id, *meta); } - entry.markUpdated(); - entry.updated = false; // We just saved the new graph + entry->is_dirty = false; // We just saved the new graph LOG_INFO(2051, index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); rebuild_.completeActiveRebuild(username); From cb9f73d18c465cbcb4263268f6456b005bfbde4d Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Tue, 21 Apr 2026 15:28:16 +0530 Subject: [PATCH 04/16] comments resolved --- src/core/ndd.hpp | 110 ++++++++++++++++++------------------------- src/core/rebuild.hpp | 43 ++++++++++++++++- src/main.cpp | 19 ++++---- 3 files changed, 96 insertions(+), 76 deletions(-) diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index 95043d0b2d..b0ad08c9ac 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -251,6 +251,19 @@ class IndexManager { } } + // Wires vector fetchers on an HNSW graph. Must be called before addPoint — searchBaseLayer + // during graph construction needs fetchers to compute distances for base-layer-only nodes. + static void wireVectorFetchers(hnswlib::HierarchicalNSW* alg, + std::shared_ptr vs) { + alg->setVectorFetcher([vs](ndd::idInt label, uint8_t* buffer) { + return vs->get_vector(label, buffer); + }); + alg->setVectorFetcherBatch([vs](const ndd::idInt* labels, uint8_t* buffers, + bool* success, size_t count) -> size_t { + return vs->get_vectors_batch_into(labels, buffers, success, count); + }); + } + std::unique_ptr createWAL(const std::string& index_id) { const std::string wal_dir = data_dir_ + "/" + index_id; return std::make_unique(wal_dir, index_id); @@ -1933,7 +1946,7 @@ class IndexManager { return metadata_manager_->getMetadata(index_id); } - // Index stats (safe to call from routes) + // Reads live count from the in-memory HNSW graph; meta->total_elements can be stale between saves. size_t getElementCount(const std::string& index_id) { auto entry = getIndexEntry(index_id); return entry->alg->getElementsCount(); @@ -1943,9 +1956,9 @@ class IndexManager { // ========== Rebuild operations ========== // Orchestration method (defined below after class) - std::pair rebuildIndexAsync(const std::string& index_id, - size_t new_M, - size_t new_ef_con); + RebuildResult rebuildIndexAsync(const std::string& index_id, + size_t new_M, + size_t new_ef_con); bool hasActiveRebuild(const std::string& username) const { return rebuild_.hasActiveRebuild(username); @@ -1953,27 +1966,7 @@ class IndexManager { nlohmann::json getRebuildProgress(const std::string& username, const std::string& index_id) const { - auto state = rebuild_.getActiveRebuild(username); - if (state && state->index_id == index_id) { - size_t processed = state->vectors_processed.load(); - size_t total = state->total_vectors.load(); - double percent = total > 0 ? (100.0 * processed / total) : 0.0; - nlohmann::json result = { - {"status", state->status}, - {"vectors_processed", processed}, - {"total_vectors", total}, - {"percent_complete", percent}, - {"started_at", Rebuild::formatTime(state->started_at)} - }; - if (state->status == "completed" || state->status == "failed") { - result["completed_at"] = Rebuild::formatTime(state->completed_at); - } - if (state->status == "failed" && !state->error_message.empty()) { - result["error"] = state->error_message; - } - return result; - } - return {{"status", "idle"}}; + return rebuild_.getProgress(username, index_id); } >>>>>>> e66b946 (Rebuild index with new config (#136)) }; @@ -2333,13 +2326,13 @@ inline std::pair IndexManager::uploadBackup(const std::string // ========== IndexManager rebuild implementations ========== -inline std::pair IndexManager::rebuildIndexAsync(const std::string& index_id, - size_t new_M, - size_t new_ef_con) { +inline RebuildResult IndexManager::rebuildIndexAsync(const std::string& index_id, + size_t new_M, + size_t new_ef_con) { // Validate index exists auto meta = metadata_manager_->getMetadata(index_id); if (!meta) { - return {false, "Index not found"}; + return {false, 404, "Index not found"}; } // Extract username for backup check @@ -2348,15 +2341,15 @@ inline std::pair IndexManager::rebuildIndexAsync(const std::s if (pos != std::string::npos) { username = index_id.substr(0, pos); } else { - return {false, "Invalid index ID format"}; + return {false, 400, "Invalid index ID format"}; } // Check for active backup or rebuild if (backup_store_.hasActiveBackup(username)) { - return {false, "Backup already in progress for user: " + username}; + return {false, 409, "Backup already in progress for user: " + username}; } if (rebuild_.hasActiveRebuild(username)) { - return {false, "Rebuild already in progress for user: " + username}; + return {false, 409, "Rebuild already in progress for user: " + username}; } // Load entry to get current element count @@ -2365,21 +2358,30 @@ inline std::pair IndexManager::rebuildIndexAsync(const std::s // Ensure at least one parameter differs if (new_M == meta->M && new_ef_con == meta->ef_con) { - return {false, "No configuration changes specified"}; + return {false, 400, "No configuration changes specified"}; } - // Set active rebuild state and spawn jthread (same pattern as createBackupAsync) + // Register state FIRST with empty thread — hasActiveRebuild() now returns true immediately, + // blocking any concurrent rebuild requests before the thread is even spawned. + rebuild_.setActiveRebuild(username, index_id, current_count, std::jthread{}); + + // THEN spawn thread std::jthread t([this, index_id, username, new_M, new_ef_con](std::stop_token st) { executeRebuildJob(index_id, username, new_M, new_ef_con, st); }); - rebuild_.setActiveRebuild(username, index_id, current_count, std::move(t)); + + // Move real thread into the already-registered state + rebuild_.attachRebuildThread(username, std::move(t)); LOG_INFO(2050, index_id, "Rebuild started: M=" << new_M << " ef_con=" << new_ef_con); - return {true, "Rebuild started"}; + return {true, 202, "Rebuild started"}; } +// executeRebuildJob lives in IndexManager (not Rebuild) because it needs direct access to +// CacheEntry, parallelAddPoints, and saveIndexInternal. Rebuild is a state-tracker only — +// moving execution here would create a circular dependency with IndexManager. inline void IndexManager::executeRebuildJob(const std::string& index_id, const std::string& username, size_t new_M, size_t new_ef_con, @@ -2411,19 +2413,8 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, max_elements, space_type, dim, new_M, new_ef_con, settings::RANDOM_SEED, quant_level, checksum); - // Set vector fetcher BEFORE adding vectors — searchBaseLayer during - // graph construction needs this to compute distances for base-layer-only - // nodes (base layer doesn't store vector data inline) - new_alg->setVectorFetcher([vs = entry->vector_storage](ndd::idInt label, uint8_t* buffer) { - return vs->get_vector(label, buffer); - }); - - new_alg->setVectorFetcherBatch([vs = entry->vector_storage](const ndd::idInt* labels, - uint8_t* buffers, - bool* success, - size_t count) -> size_t { - return vs->get_vectors_batch_into(labels, buffers, success, count); - }); + // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes + wireVectorFetchers(new_alg.get(), entry->vector_storage); // Iterate VectorStore and re-insert all vectors auto cursor = entry->vector_storage->getCursor(); @@ -2487,23 +2478,16 @@ inline void IndexManager::executeRebuildJob(const std::string& index_id, std::filesystem::copy_file(timestamped_path, index_path, std::filesystem::copy_options::overwrite_existing); - // Load fresh from disk + swap pointer (reloadIndex pattern) + // Cannot call reloadIndex() here — we hold operation_mutex and reloadIndex acquires + // indices_mutex_, while deleteIndex holds indices_mutex_ then acquires operation_mutex. + // Calling reloadIndex here would deadlock with a concurrent delete on the same index. auto fresh_alg = std::make_unique>(index_path, 0); - - fresh_alg->setVectorFetcher([vs = entry->vector_storage](ndd::idInt label, uint8_t* buffer) { - return vs->get_vector(label, buffer); - }); - - fresh_alg->setVectorFetcherBatch([vs = entry->vector_storage](const ndd::idInt* labels, - uint8_t* buffers, - bool* success, - size_t count) -> size_t { - return vs->get_vectors_batch_into(labels, buffers, success, count); - }); - + wireVectorFetchers(fresh_alg.get(), entry->vector_storage); entry->alg = std::move(fresh_alg); - // Delete temp checkpoint and timestamped file + // Both files are deleted here on success. If the server crashes before reaching this + // point, the timestamped file (default.idx.) may be left on disk — it is safe + // to delete manually on next startup as it does not affect index correctness. if (std::filesystem::exists(temp_path)) { std::filesystem::remove(temp_path); } diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index 98d10192f4..fdd2591843 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -14,6 +14,13 @@ #include "settings.hpp" #include "log.hpp" +#include "json/nlohmann_json.hpp" + +struct RebuildResult { + bool success; + int http_code; + std::string message; +}; struct ActiveRebuild { std::string index_id; @@ -57,8 +64,8 @@ class Rebuild { std::filesystem::remove(entry.path()); } } - } catch (const std::exception&) { - // Silently ignore cleanup errors on startup + } catch (const std::exception& e) { + LOG_WARN(2053, "rebuild", "Failed to cleanup temp files on startup: " << e.what()); } } @@ -133,6 +140,38 @@ class Rebuild { } } + void attachRebuildThread(const std::string& username, std::jthread&& thread) { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + it->second->thread = std::move(thread); + } + } + + nlohmann::json getProgress(const std::string& username, const std::string& index_id) const { + auto state = getActiveRebuild(username); + if (state && state->index_id == index_id) { + size_t processed = state->vectors_processed.load(); + size_t total = state->total_vectors.load(); + double percent = total > 0 ? (100.0 * processed / total) : 0.0; + nlohmann::json result = { + {"status", state->status}, + {"vectors_processed", processed}, + {"total_vectors", total}, + {"percent_complete", percent}, + {"started_at", formatTime(state->started_at)} + }; + if (state->status == "completed" || state->status == "failed") { + result["completed_at"] = formatTime(state->completed_at); + } + if (state->status == "failed" && !state->error_message.empty()) { + result["error"] = state->error_message; + } + return result; + } + return {{"status", "idle"}}; + } + std::shared_ptr getActiveRebuild(const std::string& username) const { std::lock_guard lock(rebuild_state_mutex_); auto it = active_rebuilds_.find(username); diff --git a/src/main.cpp b/src/main.cpp index 7821451ae8..69dfb908f8 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -720,7 +720,6 @@ int main(int argc, char** argv) { if (!meta) { return json_error(404, "Index not found"); } - // Parse parameters with current values as defaults size_t new_M = body.has("M") ? (size_t)body["M"].i() : meta->M; size_t new_ef_con = body.has("ef_con") ? (size_t)body["ef_con"].i() : meta->ef_con; @@ -739,19 +738,17 @@ int main(int argc, char** argv) { + " and " + std::to_string(settings::MAX_EF_CONSTRUCT)); } - // Get actual vector count for response - size_t actual_element_count = 0; - try { - actual_element_count = index_manager.getElementCount(index_id); - } catch (...) {} + // Use live count — meta->total_elements can be stale if not yet flushed to disk + size_t actual_element_count = index_manager.getElementCount(index_id); + if (actual_element_count == 0) { + return json_error(400, "Cannot rebuild an empty index"); + } try { - auto [success, message] = index_manager.rebuildIndexAsync( - index_id, new_M, new_ef_con); + auto result = index_manager.rebuildIndexAsync(index_id, new_M, new_ef_con); - if (!success) { - int code = (message.find("already in progress") != std::string::npos) ? 409 : 400; - return json_error(code, message); + if (!result.success) { + return json_error(result.http_code, result.message); } crow::json::wvalue response; From fd2c034f5639ff80e7552fe70c4226efa2b31b66 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Thu, 23 Apr 2026 14:17:10 +0530 Subject: [PATCH 05/16] rebuild also handles execute_rebuild_job --- src/core/ndd.hpp | 281 +++++++++++++------------------------------ src/core/rebuild.hpp | 132 ++++++++++++++++++++ 2 files changed, 214 insertions(+), 199 deletions(-) diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index b0ad08c9ac..17f34ffc00 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -223,46 +223,6 @@ class IndexManager { BackupStore backup_store_; Rebuild rebuild_; void executeBackupJob(const std::string& index_id, const std::string& backup_name, std::stop_token st); - void executeRebuildJob(const std::string& index_id, const std::string& username, - size_t new_M, size_t new_ef_con, std::stop_token st); - - // Shared parallel addPoint utility — static chunk partition (same as addVectors). - // ProcessFn signature: void(size_t index) - template - static void parallelAddPoints(size_t count, size_t max_threads, ProcessFn&& process) { - if (count == 0) return; - size_t num_threads = std::min(max_threads, count); - const size_t chunk_size = (count + num_threads - 1) / num_threads; // Ceiling division - std::vector threads; - threads.reserve(num_threads); - - for (size_t t = 0; t < num_threads; ++t) { - threads.emplace_back([&, t]() { - size_t start_idx = t * chunk_size; - size_t end_idx = std::min(start_idx + chunk_size, count); - for (size_t i = start_idx; i < end_idx; ++i) { - process(i); - } - }); - } - - for (auto& th : threads) { - th.join(); - } - } - - // Wires vector fetchers on an HNSW graph. Must be called before addPoint — searchBaseLayer - // during graph construction needs fetchers to compute distances for base-layer-only nodes. - static void wireVectorFetchers(hnswlib::HierarchicalNSW* alg, - std::shared_ptr vs) { - alg->setVectorFetcher([vs](ndd::idInt label, uint8_t* buffer) { - return vs->get_vector(label, buffer); - }); - alg->setVectorFetcherBatch([vs](const ndd::idInt* labels, uint8_t* buffers, - bool* success, size_t count) -> size_t { - return vs->get_vectors_batch_into(labels, buffers, success, count); - }); - } std::unique_ptr createWAL(const std::string& index_id) { const std::string wal_dir = data_dir_ + "/" + index_id; @@ -1936,11 +1896,10 @@ class IndexManager { return backup_store_.validateBackupName(backup_name); } -<<<<<<< HEAD std::pair uploadBackup(const std::string& backup_name, const std::string& username, const std::string& file_content); -======= + // Metadata access std::optional getMetadata(const std::string& index_id) { return metadata_manager_->getMetadata(index_id); @@ -1968,7 +1927,42 @@ class IndexManager { const std::string& index_id) const { return rebuild_.getProgress(username, index_id); } ->>>>>>> e66b946 (Rebuild index with new config (#136)) + + // Shared parallel addPoint utility — static chunk partition (same as addVectors). + // ProcessFn signature: void(size_t index) + template + static void parallelAddPoints(size_t count, size_t max_threads, ProcessFn&& process) { + if (count == 0) return; + size_t num_threads = std::min(max_threads, count); + const size_t chunk_size = (count + num_threads - 1) / num_threads; + std::vector threads; + threads.reserve(num_threads); + for (size_t t = 0; t < num_threads; ++t) { + threads.emplace_back([&, t]() { + size_t start_idx = t * chunk_size; + size_t end_idx = std::min(start_idx + chunk_size, count); + for (size_t i = start_idx; i < end_idx; ++i) { + process(i); + } + }); + } + for (auto& th : threads) { + th.join(); + } + } + + // Wires vector fetchers on an HNSW graph. Must be called before addPoint — searchBaseLayer + // during graph construction needs fetchers to compute distances for base-layer-only nodes. + static void wireVectorFetchers(hnswlib::HierarchicalNSW* alg, + std::shared_ptr vs) { + alg->setVectorFetcher([vs](ndd::idInt label, uint8_t* buffer) { + return vs->get_vector(label, buffer); + }); + alg->setVectorFetcherBatch([vs](const ndd::idInt* labels, uint8_t* buffers, + bool* success, size_t count) -> size_t { + return vs->get_vectors_batch_into(labels, buffers, success, count); + }); + } }; // ========== IndexManager backup implementations ========== @@ -2327,15 +2321,13 @@ inline std::pair IndexManager::uploadBackup(const std::string // ========== IndexManager rebuild implementations ========== inline RebuildResult IndexManager::rebuildIndexAsync(const std::string& index_id, - size_t new_M, - size_t new_ef_con) { - // Validate index exists + size_t new_M, + size_t new_ef_con) { auto meta = metadata_manager_->getMetadata(index_id); if (!meta) { return {false, 404, "Index not found"}; } - // Extract username for backup check std::string username; size_t pos = index_id.find('/'); if (pos != std::string::npos) { @@ -2344,7 +2336,6 @@ inline RebuildResult IndexManager::rebuildIndexAsync(const std::string& index_id return {false, 400, "Invalid index ID format"}; } - // Check for active backup or rebuild if (backup_store_.hasActiveBackup(username)) { return {false, 409, "Backup already in progress for user: " + username}; } @@ -2352,170 +2343,62 @@ inline RebuildResult IndexManager::rebuildIndexAsync(const std::string& index_id return {false, 409, "Rebuild already in progress for user: " + username}; } - // Load entry to get current element count + // Pre-fetch entry now — captured by lambdas so the thread never calls getIndexEntry auto entry = getIndexEntry(index_id); size_t current_count = entry->alg->getElementsCount(); - // Ensure at least one parameter differs if (new_M == meta->M && new_ef_con == meta->ef_con) { return {false, 400, "No configuration changes specified"}; } - // Register state FIRST with empty thread — hasActiveRebuild() now returns true immediately, - // blocking any concurrent rebuild requests before the thread is even spawned. + std::string base_path = data_dir_ + "/" + index_id; + std::string vector_storage_dir = base_path + "/vectors"; + + RebuildJobParams params{ + .index_id = index_id, + .username = username, + .new_M = new_M, + .new_ef_con = new_ef_con, + .space_type = entry->alg->getSpaceType(), + .dim = entry->alg->getDimension(), + .quant_level = entry->alg->getQuantLevel(), + .checksum = entry->alg->getChecksum(), + .max_elements = entry->alg->getMaxElements(), + .vector_storage = entry->vector_storage, + .temp_path = Rebuild::getTempPath(base_path), + .timestamped_path = Rebuild::getTimestampedPath(base_path), + .index_path = vector_storage_dir + "/" + settings::DEFAULT_SUBINDEX + ".idx", + .num_parallel_inserts = settings::NUM_PARALLEL_INSERTS, + .operation_mutex = &entry->operation_mutex, + .save_current_index = [this, entry]() { saveIndexInternal(*entry); }, + .swap_alg = [entry](auto fresh) { entry->alg = std::move(fresh); }, + .update_metadata = [this, index_id, entry](size_t nm, size_t nef) { + auto m = metadata_manager_->getMetadata(index_id); + if (m) { + m->M = nm; + m->ef_con = nef; + m->total_elements = entry->alg->getElementsCount(); + metadata_manager_->storeMetadata(index_id, *m); + } + }, + .clear_dirty = [entry]() { entry->is_dirty = false; }, + .wire_fetchers = [](auto* alg, auto vs) { IndexManager::wireVectorFetchers(alg, vs); }, + .parallel_add = [](size_t n, size_t t, std::function fn) { + IndexManager::parallelAddPoints(n, t, std::move(fn)); + }, + }; + + // Register state FIRST with empty thread — hasActiveRebuild() returns true immediately rebuild_.setActiveRebuild(username, index_id, current_count, std::jthread{}); - // THEN spawn thread - std::jthread t([this, index_id, username, new_M, new_ef_con](std::stop_token st) { - executeRebuildJob(index_id, username, new_M, new_ef_con, st); + // Spawn thread — lambda calls rebuild_.executeJob directly (execution lives in Rebuild) + std::jthread t([this, params = std::move(params)](std::stop_token st) mutable { + rebuild_.executeJob(params, st); }); // Move real thread into the already-registered state rebuild_.attachRebuildThread(username, std::move(t)); - LOG_INFO(2050, index_id, "Rebuild started: M=" << new_M - << " ef_con=" << new_ef_con); - + LOG_INFO(2050, index_id, "Rebuild started: M=" << new_M << " ef_con=" << new_ef_con); return {true, 202, "Rebuild started"}; } - -// executeRebuildJob lives in IndexManager (not Rebuild) because it needs direct access to -// CacheEntry, parallelAddPoints, and saveIndexInternal. Rebuild is a state-tracker only — -// moving execution here would create a circular dependency with IndexManager. -inline void IndexManager::executeRebuildJob(const std::string& index_id, - const std::string& username, - size_t new_M, size_t new_ef_con, - std::stop_token st) { - std::string base_path = data_dir_ + "/" + index_id; - std::string temp_path = rebuild_.getTempPath(base_path); - std::string timestamped_path = rebuild_.getTimestampedPath(base_path); - std::string vector_storage_dir = base_path + "/vectors"; - std::string index_path = vector_storage_dir + "/" + settings::DEFAULT_SUBINDEX + ".idx"; - - try { - auto entry = getIndexEntry(index_id); - - // Hold operation_mutex for entire rebuild — writes timeout, searches continue - std::unique_lock operation_lock(entry->operation_mutex); - - // Phase 1 — Save current state - saveIndexInternal(*entry); - - // Read current config from the existing HNSW graph - auto space_type = entry->alg->getSpaceType(); - size_t dim = entry->alg->getDimension(); - auto quant_level = entry->alg->getQuantLevel(); - int32_t checksum = entry->alg->getChecksum(); - size_t max_elements = entry->alg->getMaxElements(); - - // Phase 2 — Build new HNSW (same max_elements as current index) - auto new_alg = std::make_unique>( - max_elements, space_type, dim, new_M, new_ef_con, - settings::RANDOM_SEED, quant_level, checksum); - - // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes - wireVectorFetchers(new_alg.get(), entry->vector_storage); - - // Iterate VectorStore and re-insert all vectors - auto cursor = entry->vector_storage->getCursor(); - const size_t batch_size = settings::RECOVERY_BATCH_SIZE; - size_t total_processed = 0; - size_t batches_since_checkpoint = 0; - constexpr size_t CHECKPOINT_INTERVAL = 5; // Save temp every 5 batches - - while (cursor.hasNext()) { - if (st.stop_requested()) { - if (std::filesystem::exists(temp_path)) { - std::filesystem::remove(temp_path); - } - rebuild_.failActiveRebuild(username, "Rebuild interrupted by server shutdown"); - return; - } - - // Collect batch - std::vector>> batch; - batch.reserve(batch_size); - while (cursor.hasNext() && batch.size() < batch_size) { - auto [label, vec_bytes] = cursor.next(); - if (!vec_bytes.empty()) { - batch.emplace_back(label, std::move(vec_bytes)); - } - } - - if (batch.empty()) { - break; - } - - // Multi-threaded insert (shared utility with addVectors) - parallelAddPoints(batch.size(), settings::NUM_PARALLEL_INSERTS, - [&](size_t i) { - const auto& [label, vec_bytes] = batch[i]; - new_alg->addPoint(vec_bytes.data(), label); - }); - - total_processed += batch.size(); - - // Update progress - auto state = rebuild_.getActiveRebuild(username); - if (state) { - state->vectors_processed.store(total_processed); - } - - // Periodic checkpoint save - batches_since_checkpoint++; - if (batches_since_checkpoint >= CHECKPOINT_INTERVAL) { - new_alg->saveIndex(temp_path); - batches_since_checkpoint = 0; - } - } - - // Phase 3 — Save final + Copy + Swap - - // Save new graph to timestamped file - new_alg->saveIndex(timestamped_path); - - // Copy to canonical name (overwrites old default.idx on disk) - std::filesystem::copy_file(timestamped_path, index_path, - std::filesystem::copy_options::overwrite_existing); - - // Cannot call reloadIndex() here — we hold operation_mutex and reloadIndex acquires - // indices_mutex_, while deleteIndex holds indices_mutex_ then acquires operation_mutex. - // Calling reloadIndex here would deadlock with a concurrent delete on the same index. - auto fresh_alg = std::make_unique>(index_path, 0); - wireVectorFetchers(fresh_alg.get(), entry->vector_storage); - entry->alg = std::move(fresh_alg); - - // Both files are deleted here on success. If the server crashes before reaching this - // point, the timestamped file (default.idx.) may be left on disk — it is safe - // to delete manually on next startup as it does not affect index correctness. - if (std::filesystem::exists(temp_path)) { - std::filesystem::remove(temp_path); - } - if (std::filesystem::exists(timestamped_path)) { - std::filesystem::remove(timestamped_path); - } - - // Update metadata with new config - auto meta = metadata_manager_->getMetadata(index_id); - if (meta) { - meta->M = new_M; - meta->ef_con = new_ef_con; - meta->total_elements = entry->alg->getElementsCount(); - metadata_manager_->storeMetadata(index_id, *meta); - } - - entry->is_dirty = false; // We just saved the new graph - - LOG_INFO(2051, index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); - rebuild_.completeActiveRebuild(username); - - } catch (const std::exception& e) { - LOG_ERROR(2052, index_id, "Rebuild failed: " << e.what()); - - // Cleanup temp file on error - if (std::filesystem::exists(temp_path)) { - std::filesystem::remove(temp_path); - } - rebuild_.failActiveRebuild(username, e.what()); - } -} diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index fdd2591843..f7727514a7 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -5,16 +5,22 @@ #include #include #include +#include #include #include #include +#include #include #include +#include #include #include "settings.hpp" #include "log.hpp" #include "json/nlohmann_json.hpp" +#include "hnsw/hnswlib.h" +#include "vector_storage.hpp" +#include "../quant/common.hpp" struct RebuildResult { bool success; @@ -33,6 +39,45 @@ struct ActiveRebuild { std::jthread thread; // jthread: built-in stop_token + auto-join on destruction }; +// Parameters passed to Rebuild::executeJob. IndexManager-specific operations are +// provided as callbacks so rebuild.hpp does not need to include ndd.hpp. +struct RebuildJobParams { + // Identity + std::string index_id; + std::string username; + size_t new_M; + size_t new_ef_con; + + // Current graph config (read from entry->alg by IndexManager before thread spawn) + hnswlib::SpaceType space_type; + size_t dim; + ndd::quant::QuantizationLevel quant_level; + int32_t checksum; + size_t max_elements; + + // Storage for vector iteration + std::shared_ptr vector_storage; + + // File paths + std::string temp_path; + std::string timestamped_path; + std::string index_path; + + // Threading + size_t num_parallel_inserts; + + // Mutex pointer — executeJob acquires this for the whole job duration + std::shared_mutex* operation_mutex; + + // Callbacks for IndexManager-specific actions (avoids circular ndd.hpp include) + std::function save_current_index; + std::function>)> swap_alg; + std::function update_metadata; + std::function clear_dirty; + std::function*, std::shared_ptr)> wire_fetchers; + std::function)> parallel_add; +}; + class Rebuild { private: // Keyed by username — one rebuild per user at a time @@ -200,4 +245,91 @@ class Rebuild { ); return index_dir + "/vectors/" + settings::DEFAULT_SUBINDEX + ".idx." + ts; } + + // Owns all rebuild execution. Called directly from the jthread lambda spawned in + // rebuildIndexAsync. IndexManager-specific operations come in via p callbacks. + void executeJob(const RebuildJobParams& p, std::stop_token st) { + try { + std::unique_lock op_lock(*p.operation_mutex); + + // Phase 1 — save current state before rebuilding + p.save_current_index(); + + // Phase 2 — build new HNSW with updated M/ef_con + auto new_alg = std::make_unique>( + p.max_elements, p.space_type, p.dim, p.new_M, p.new_ef_con, + settings::RANDOM_SEED, p.quant_level, p.checksum); + + // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes + p.wire_fetchers(new_alg.get(), p.vector_storage); + + auto cursor = p.vector_storage->getCursor(); + const size_t batch_size = settings::RECOVERY_BATCH_SIZE; + size_t total_processed = 0; + size_t batches_since_checkpoint = 0; + constexpr size_t CHECKPOINT_INTERVAL = 5; + + while (cursor.hasNext()) { + if (st.stop_requested()) { + if (std::filesystem::exists(p.temp_path)) + std::filesystem::remove(p.temp_path); + failActiveRebuild(p.username, "Rebuild interrupted by server shutdown"); + return; + } + + std::vector>> batch; + batch.reserve(batch_size); + while (cursor.hasNext() && batch.size() < batch_size) { + auto [label, vec_bytes] = cursor.next(); + if (!vec_bytes.empty()) + batch.emplace_back(label, std::move(vec_bytes)); + } + if (batch.empty()) break; + + p.parallel_add(batch.size(), p.num_parallel_inserts, + [&](size_t i) { + const auto& [label, vec_bytes] = batch[i]; + new_alg->addPoint(vec_bytes.data(), label); + }); + + total_processed += batch.size(); + auto state = getActiveRebuild(p.username); + if (state) state->vectors_processed.store(total_processed); + + if (++batches_since_checkpoint >= CHECKPOINT_INTERVAL) { + new_alg->saveIndex(p.temp_path); + batches_since_checkpoint = 0; + } + } + + // Phase 3 — save final, copy to canonical path, load fresh from disk + new_alg->saveIndex(p.timestamped_path); + std::filesystem::copy_file(p.timestamped_path, p.index_path, + std::filesystem::copy_options::overwrite_existing); + + // Cannot call reloadIndex() here — we hold operation_mutex and reloadIndex acquires + // indices_mutex_, while deleteIndex holds indices_mutex_ then acquires operation_mutex. + // Calling reloadIndex here would deadlock with a concurrent delete on the same index. + auto fresh_alg = std::make_unique>(p.index_path, 0); + p.wire_fetchers(fresh_alg.get(), p.vector_storage); + + // Both files are deleted here on success. If the server crashes before reaching this + // point, the timestamped file (default.idx.) may be left on disk — it is safe + // to delete manually on next startup as it does not affect index correctness. + if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); + if (std::filesystem::exists(p.timestamped_path)) std::filesystem::remove(p.timestamped_path); + + p.swap_alg(std::move(fresh_alg)); + p.update_metadata(p.new_M, p.new_ef_con); + p.clear_dirty(); + + LOG_INFO(2051, p.index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); + completeActiveRebuild(p.username); + + } catch (const std::exception& e) { + LOG_ERROR(2052, p.index_id, "Rebuild failed: " << e.what()); + if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); + failActiveRebuild(p.username, e.what()); + } + } }; From 3b32a2e902b87602d3ea8a91a33396869a76f2ac Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Thu, 23 Apr 2026 15:27:29 +0530 Subject: [PATCH 06/16] correction on cleantempfiles and error code --- src/core/ndd.hpp | 30 ++++++++++-------- src/core/rebuild.hpp | 72 ++++++++++++++++++++++---------------------- src/main.cpp | 6 ++-- src/utils/types.hpp | 11 +++++++ 4 files changed, 67 insertions(+), 52 deletions(-) create mode 100644 src/utils/types.hpp diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index 17f34ffc00..e3d062dd03 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -1914,10 +1914,14 @@ class IndexManager { // ========== Rebuild operations ========== - // Orchestration method (defined below after class) - RebuildResult rebuildIndexAsync(const std::string& index_id, - size_t new_M, - size_t new_ef_con); + // Return codes: + // 0: rebuild started successfully + // 1: index not found + // 2: rebuild or backup already in progress for this user + // 3: no configuration changes specified / invalid parameters + OperationResult rebuildIndexAsync(const std::string& index_id, + size_t new_M, + size_t new_ef_con); bool hasActiveRebuild(const std::string& username) const { return rebuild_.hasActiveRebuild(username); @@ -2320,12 +2324,12 @@ inline std::pair IndexManager::uploadBackup(const std::string // ========== IndexManager rebuild implementations ========== -inline RebuildResult IndexManager::rebuildIndexAsync(const std::string& index_id, - size_t new_M, - size_t new_ef_con) { +inline OperationResult IndexManager::rebuildIndexAsync(const std::string& index_id, + size_t new_M, + size_t new_ef_con) { auto meta = metadata_manager_->getMetadata(index_id); if (!meta) { - return {false, 404, "Index not found"}; + return {1, "Index not found"}; } std::string username; @@ -2333,14 +2337,14 @@ inline RebuildResult IndexManager::rebuildIndexAsync(const std::string& index_id if (pos != std::string::npos) { username = index_id.substr(0, pos); } else { - return {false, 400, "Invalid index ID format"}; + return {3, "Invalid index ID format"}; } if (backup_store_.hasActiveBackup(username)) { - return {false, 409, "Backup already in progress for user: " + username}; + return {2, "Backup already in progress for user: " + username}; } if (rebuild_.hasActiveRebuild(username)) { - return {false, 409, "Rebuild already in progress for user: " + username}; + return {2, "Rebuild already in progress for user: " + username}; } // Pre-fetch entry now — captured by lambdas so the thread never calls getIndexEntry @@ -2348,7 +2352,7 @@ inline RebuildResult IndexManager::rebuildIndexAsync(const std::string& index_id size_t current_count = entry->alg->getElementsCount(); if (new_M == meta->M && new_ef_con == meta->ef_con) { - return {false, 400, "No configuration changes specified"}; + return {3, "No configuration changes specified"}; } std::string base_path = data_dir_ + "/" + index_id; @@ -2400,5 +2404,5 @@ inline RebuildResult IndexManager::rebuildIndexAsync(const std::string& index_id rebuild_.attachRebuildThread(username, std::move(t)); LOG_INFO(2050, index_id, "Rebuild started: M=" << new_M << " ef_con=" << new_ef_con); - return {true, 202, "Rebuild started"}; + return {0, "Rebuild started"}; } diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index f7727514a7..25ae997941 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include #include @@ -21,19 +20,14 @@ #include "hnsw/hnswlib.h" #include "vector_storage.hpp" #include "../quant/common.hpp" - -struct RebuildResult { - bool success; - int http_code; - std::string message; -}; +#include "utils/types.hpp" struct ActiveRebuild { std::string index_id; std::string status{"in_progress"}; // "in_progress", "completed", "failed" std::string error_message; - std::atomic vectors_processed{0}; - std::atomic total_vectors{0}; + size_t vectors_processed{0}; + size_t total_vectors{0}; std::chrono::system_clock::time_point started_at; std::chrono::system_clock::time_point completed_at; std::jthread thread; // jthread: built-in stop_token + auto-join on destruction @@ -103,9 +97,15 @@ class Rebuild { } try { std::string temp_filename = std::string(settings::DEFAULT_SUBINDEX) + ".idx.temp"; + std::string ts_prefix = std::string(settings::DEFAULT_SUBINDEX) + ".idx."; for (const auto& entry : std::filesystem::recursive_directory_iterator(data_dir)) { - if (entry.is_regular_file() && - entry.path().filename().string() == temp_filename) { + if (!entry.is_regular_file()) continue; + const std::string fname = entry.path().filename().string(); + bool is_temp = (fname == temp_filename); + bool is_ts = fname.size() > ts_prefix.size() + && fname.substr(0, ts_prefix.size()) == ts_prefix + && std::all_of(fname.begin() + ts_prefix.size(), fname.end(), ::isdigit); + if (is_temp || is_ts) { std::filesystem::remove(entry.path()); } } @@ -122,8 +122,8 @@ class Rebuild { auto state = std::make_shared(); state->index_id = index_id; state->status = "in_progress"; - state->total_vectors.store(total_vectors); - state->vectors_processed.store(0); + state->total_vectors = total_vectors; + state->vectors_processed = 0; state->started_at = std::chrono::system_clock::now(); state->thread = std::move(thread); active_rebuilds_[username] = state; @@ -193,39 +193,40 @@ class Rebuild { } } + void updateProgress(const std::string& username, size_t processed) { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + it->second->vectors_processed = processed; + } + } + nlohmann::json getProgress(const std::string& username, const std::string& index_id) const { - auto state = getActiveRebuild(username); - if (state && state->index_id == index_id) { - size_t processed = state->vectors_processed.load(); - size_t total = state->total_vectors.load(); + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end() && it->second->index_id == index_id) { + const auto& state = *it->second; + size_t processed = state.vectors_processed; + size_t total = state.total_vectors; double percent = total > 0 ? (100.0 * processed / total) : 0.0; nlohmann::json result = { - {"status", state->status}, + {"status", state.status}, {"vectors_processed", processed}, {"total_vectors", total}, {"percent_complete", percent}, - {"started_at", formatTime(state->started_at)} + {"started_at", formatTime(state.started_at)} }; - if (state->status == "completed" || state->status == "failed") { - result["completed_at"] = formatTime(state->completed_at); + if (state.status == "completed" || state.status == "failed") { + result["completed_at"] = formatTime(state.completed_at); } - if (state->status == "failed" && !state->error_message.empty()) { - result["error"] = state->error_message; + if (state.status == "failed" && !state.error_message.empty()) { + result["error"] = state.error_message; } return result; } return {{"status", "idle"}}; } - std::shared_ptr getActiveRebuild(const std::string& username) const { - std::lock_guard lock(rebuild_state_mutex_); - auto it = active_rebuilds_.find(username); - if (it != active_rebuilds_.end()) { - return it->second; - } - return nullptr; - } - // Format state as JSON fields static std::string formatTime(std::chrono::system_clock::time_point tp) { return timeToISO8601(tp); @@ -293,8 +294,7 @@ class Rebuild { }); total_processed += batch.size(); - auto state = getActiveRebuild(p.username); - if (state) state->vectors_processed.store(total_processed); + updateProgress(p.username, total_processed); if (++batches_since_checkpoint >= CHECKPOINT_INTERVAL) { new_alg->saveIndex(p.temp_path); @@ -314,8 +314,8 @@ class Rebuild { p.wire_fetchers(fresh_alg.get(), p.vector_storage); // Both files are deleted here on success. If the server crashes before reaching this - // point, the timestamped file (default.idx.) may be left on disk — it is safe - // to delete manually on next startup as it does not affect index correctness. + // point, the timestamped file (default.idx.) will be removed on next startup + // by cleanupTempFiles — it does not affect index correctness. if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); if (std::filesystem::exists(p.timestamped_path)) std::filesystem::remove(p.timestamped_path); diff --git a/src/main.cpp b/src/main.cpp index 69dfb908f8..05cbf21ac8 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -747,9 +747,9 @@ int main(int argc, char** argv) { try { auto result = index_manager.rebuildIndexAsync(index_id, new_M, new_ef_con); - if (!result.success) { - return json_error(result.http_code, result.message); - } + if (result.code == 1) return json_error(404, result.message); + if (result.code == 2) return json_error(409, result.message); + if (result.code == 3) return json_error(400, result.message); crow::json::wvalue response; response["status"] = "rebuilding"; diff --git a/src/utils/types.hpp b/src/utils/types.hpp new file mode 100644 index 0000000000..e45d13678f --- /dev/null +++ b/src/utils/types.hpp @@ -0,0 +1,11 @@ +#pragma once +#include + +// Generic operation result returned by async and sync operations. +// Each function documents its return codes in comments above its declaration. +// Code 0 always means success. Non-zero codes are operation-specific. +// Codes can be conglomerated into ENUMs per operation as the codebase matures. +struct OperationResult { + unsigned char code; // 0 = success, non-zero = error (operation-specific) + std::string message; +}; From 13fe6ce31d371141904e5cfe5700aeae448ad5af Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Thu, 23 Apr 2026 16:15:14 +0530 Subject: [PATCH 07/16] rebuild status enum and logs code change --- docs/logs.md | 1 + src/core/ndd.hpp | 4 ++-- src/core/rebuild.hpp | 42 ++++++++++++++++++++++++++++-------------- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/docs/logs.md b/docs/logs.md index 5ceb9d78b2..b8829df3c7 100644 --- a/docs/logs.md +++ b/docs/logs.md @@ -88,6 +88,7 @@ The same overload shapes apply to `LOG_WARN` and `LOG_ERROR`. - `1500s` metadata logs - `1600s` vector storage logs - `1700s` system sanity checks (CPU compatibility, disk, memory, ulimits) + - `1800s` rebuild subsystem logs - `2000s` index manager logs - `2100s` HNSW load/cache logs diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index e3d062dd03..087e31b215 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -2393,7 +2393,7 @@ inline OperationResult IndexManager::rebuildIndexAsync(const std::string& index_ }; // Register state FIRST with empty thread — hasActiveRebuild() returns true immediately - rebuild_.setActiveRebuild(username, index_id, current_count, std::jthread{}); + rebuild_.setActiveRebuild(username, index_id, current_count); // Spawn thread — lambda calls rebuild_.executeJob directly (execution lives in Rebuild) std::jthread t([this, params = std::move(params)](std::stop_token st) mutable { @@ -2403,6 +2403,6 @@ inline OperationResult IndexManager::rebuildIndexAsync(const std::string& index_ // Move real thread into the already-registered state rebuild_.attachRebuildThread(username, std::move(t)); - LOG_INFO(2050, index_id, "Rebuild started: M=" << new_M << " ef_con=" << new_ef_con); + LOG_INFO(1800, index_id, "Rebuild started: M=" << new_M << " ef_con=" << new_ef_con); return {0, "Rebuild started"}; } diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index 25ae997941..97fdd369ae 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -22,9 +22,15 @@ #include "../quant/common.hpp" #include "utils/types.hpp" +enum class RebuildStatus : unsigned char { + IN_PROGRESS = 0, + COMPLETED = 1, + FAILED = 2 +}; + struct ActiveRebuild { std::string index_id; - std::string status{"in_progress"}; // "in_progress", "completed", "failed" + RebuildStatus status{RebuildStatus::IN_PROGRESS}; std::string error_message; size_t vectors_processed{0}; size_t total_vectors{0}; @@ -78,6 +84,15 @@ class Rebuild { std::unordered_map> active_rebuilds_; mutable std::mutex rebuild_state_mutex_; + static std::string statusToString(RebuildStatus s) { + switch (s) { + case RebuildStatus::IN_PROGRESS: return "in_progress"; + case RebuildStatus::COMPLETED: return "completed"; + case RebuildStatus::FAILED: return "failed"; + default: return "unknown"; + } + } + static std::string timeToISO8601(std::chrono::system_clock::time_point tp) { auto time_t_val = std::chrono::system_clock::to_time_t(tp); std::tm tm_val{}; @@ -110,22 +125,21 @@ class Rebuild { } } } catch (const std::exception& e) { - LOG_WARN(2053, "rebuild", "Failed to cleanup temp files on startup: " << e.what()); + LOG_WARN(1803, "rebuild", "Failed to cleanup temp files on startup: " << e.what()); } } // State tracking — per user void setActiveRebuild(const std::string& username, const std::string& index_id, - size_t total_vectors, std::jthread&& thread) { + size_t total_vectors) { std::lock_guard lock(rebuild_state_mutex_); auto state = std::make_shared(); state->index_id = index_id; - state->status = "in_progress"; + state->status = RebuildStatus::IN_PROGRESS; state->total_vectors = total_vectors; state->vectors_processed = 0; state->started_at = std::chrono::system_clock::now(); - state->thread = std::move(thread); active_rebuilds_[username] = state; } @@ -137,7 +151,7 @@ class Rebuild { if (it->second->thread.joinable()) { it->second->thread.detach(); } - it->second->status = "completed"; + it->second->status = RebuildStatus::COMPLETED; it->second->completed_at = std::chrono::system_clock::now(); } } @@ -150,7 +164,7 @@ class Rebuild { if (it->second->thread.joinable()) { it->second->thread.detach(); } - it->second->status = "failed"; + it->second->status = RebuildStatus::FAILED; it->second->error_message = error; it->second->completed_at = std::chrono::system_clock::now(); } @@ -159,8 +173,8 @@ class Rebuild { bool hasActiveRebuild(const std::string& username) const { std::lock_guard lock(rebuild_state_mutex_); auto it = active_rebuilds_.find(username); - // Only "in_progress" blocks a new rebuild - return it != active_rebuilds_.end() && it->second->status == "in_progress"; + // Only IN_PROGRESS blocks a new rebuild + return it != active_rebuilds_.end() && it->second->status == RebuildStatus::IN_PROGRESS; } // Join all in-progress rebuild threads on shutdown. Mirrors BackupStore::joinAllThreads: @@ -210,16 +224,16 @@ class Rebuild { size_t total = state.total_vectors; double percent = total > 0 ? (100.0 * processed / total) : 0.0; nlohmann::json result = { - {"status", state.status}, + {"status", statusToString(state.status)}, {"vectors_processed", processed}, {"total_vectors", total}, {"percent_complete", percent}, {"started_at", formatTime(state.started_at)} }; - if (state.status == "completed" || state.status == "failed") { + if (state.status == RebuildStatus::COMPLETED || state.status == RebuildStatus::FAILED) { result["completed_at"] = formatTime(state.completed_at); } - if (state.status == "failed" && !state.error_message.empty()) { + if (state.status == RebuildStatus::FAILED && !state.error_message.empty()) { result["error"] = state.error_message; } return result; @@ -323,11 +337,11 @@ class Rebuild { p.update_metadata(p.new_M, p.new_ef_con); p.clear_dirty(); - LOG_INFO(2051, p.index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); + LOG_INFO(1801, p.index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); completeActiveRebuild(p.username); } catch (const std::exception& e) { - LOG_ERROR(2052, p.index_id, "Rebuild failed: " << e.what()); + LOG_ERROR(1802, p.index_id, "Rebuild failed: " << e.what()); if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); failActiveRebuild(p.username, e.what()); } From 6f4083e0515d8de577c872afa4b824a3bab0e3e0 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Thu, 23 Apr 2026 16:34:44 +0530 Subject: [PATCH 08/16] rebuild.cpp --- CMakeLists.txt | 1 + src/core/ndd.hpp | 1 + src/core/rebuild.cpp | 258 +++++++++++++++++++++++++++++++++++++++ src/core/rebuild.hpp | 284 +++---------------------------------------- 4 files changed, 279 insertions(+), 265 deletions(-) create mode 100644 src/core/rebuild.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 5fa15b0d93..ca5f37e564 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -254,6 +254,7 @@ message(STATUS "Binary name: ${NDD_BINARY_NAME}") set(NDD_CORE_SOURCES src/sparse/inverted_index.cpp src/utils/system_sanity/system_sanity.cpp + src/core/rebuild.cpp ) # Build non-main project sources separately so they can be compiled in parallel diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index 087e31b215..ccee4d756e 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -198,6 +198,7 @@ struct PersistenceConfig { #include "../storage/backup_store.hpp" #include "rebuild.hpp" +#include "utils/types.hpp" class IndexManager { private: diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp new file mode 100644 index 0000000000..96cd9410ea --- /dev/null +++ b/src/core/rebuild.cpp @@ -0,0 +1,258 @@ +#include "rebuild.hpp" + +#include +#include +#include + +#include "settings.hpp" +#include "log.hpp" +#include "utils/types.hpp" + +std::string Rebuild::statusToString(RebuildStatus s) { + switch (s) { + case RebuildStatus::IN_PROGRESS: return "in_progress"; + case RebuildStatus::COMPLETED: return "completed"; + case RebuildStatus::FAILED: return "failed"; + default: return "unknown"; + } +} + +std::string Rebuild::timeToISO8601(std::chrono::system_clock::time_point tp) { + auto time_t_val = std::chrono::system_clock::to_time_t(tp); + std::tm tm_val{}; + gmtime_r(&time_t_val, &tm_val); + std::ostringstream oss; + oss << std::put_time(&tm_val, "%Y-%m-%dT%H:%M:%SZ"); + return oss.str(); +} + +void Rebuild::cleanupTempFiles(const std::string& data_dir) { + if (!std::filesystem::exists(data_dir)) { + return; + } + try { + std::string temp_filename = std::string(settings::DEFAULT_SUBINDEX) + ".idx.temp"; + std::string ts_prefix = std::string(settings::DEFAULT_SUBINDEX) + ".idx."; + for (const auto& entry : std::filesystem::recursive_directory_iterator(data_dir)) { + if (!entry.is_regular_file()) continue; + const std::string fname = entry.path().filename().string(); + bool is_temp = (fname == temp_filename); + bool is_ts = fname.size() > ts_prefix.size() + && fname.substr(0, ts_prefix.size()) == ts_prefix + && std::all_of(fname.begin() + ts_prefix.size(), fname.end(), ::isdigit); + if (is_temp || is_ts) { + std::filesystem::remove(entry.path()); + } + } + } catch (const std::exception& e) { + LOG_WARN(1803, "rebuild", "Failed to cleanup temp files on startup: " << e.what()); + } +} + +void Rebuild::setActiveRebuild(const std::string& username, const std::string& index_id, + size_t total_vectors) { + std::lock_guard lock(rebuild_state_mutex_); + auto state = std::make_shared(); + state->index_id = index_id; + state->status = RebuildStatus::IN_PROGRESS; + state->total_vectors = total_vectors; + state->vectors_processed = 0; + state->started_at = std::chrono::system_clock::now(); + active_rebuilds_[username] = state; +} + +void Rebuild::completeActiveRebuild(const std::string& username) { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + // Called from within the thread — detach so the jthread dtor doesn't join us + if (it->second->thread.joinable()) { + it->second->thread.detach(); + } + it->second->status = RebuildStatus::COMPLETED; + it->second->completed_at = std::chrono::system_clock::now(); + } +} + +void Rebuild::failActiveRebuild(const std::string& username, const std::string& error) { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + // Called from within the thread — detach so the jthread dtor doesn't join us + if (it->second->thread.joinable()) { + it->second->thread.detach(); + } + it->second->status = RebuildStatus::FAILED; + it->second->error_message = error; + it->second->completed_at = std::chrono::system_clock::now(); + } +} + +bool Rebuild::hasActiveRebuild(const std::string& username) const { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + // Only IN_PROGRESS blocks a new rebuild + return it != active_rebuilds_.end() && it->second->status == RebuildStatus::IN_PROGRESS; +} + +void Rebuild::joinAllThreads() { + std::vector threads_to_join; + { + std::lock_guard lock(rebuild_state_mutex_); + for (auto& [username, state] : active_rebuilds_) { + if (state->thread.joinable()) { + threads_to_join.push_back(std::move(state->thread)); + } + } + active_rebuilds_.clear(); + } + for (auto& t : threads_to_join) { + t.request_stop(); + if (t.joinable()) { + t.join(); + } + } +} + +void Rebuild::attachRebuildThread(const std::string& username, std::jthread&& thread) { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + it->second->thread = std::move(thread); + } +} + +void Rebuild::updateProgress(const std::string& username, size_t processed) { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end()) { + it->second->vectors_processed = processed; + } +} + +nlohmann::json Rebuild::getProgress(const std::string& username, const std::string& index_id) const { + std::lock_guard lock(rebuild_state_mutex_); + auto it = active_rebuilds_.find(username); + if (it != active_rebuilds_.end() && it->second->index_id == index_id) { + const auto& state = *it->second; + size_t processed = state.vectors_processed; + size_t total = state.total_vectors; + double percent = total > 0 ? (100.0 * processed / total) : 0.0; + nlohmann::json result = { + {"status", statusToString(state.status)}, + {"vectors_processed", processed}, + {"total_vectors", total}, + {"percent_complete", percent}, + {"started_at", formatTime(state.started_at)} + }; + if (state.status == RebuildStatus::COMPLETED || state.status == RebuildStatus::FAILED) { + result["completed_at"] = formatTime(state.completed_at); + } + if (state.status == RebuildStatus::FAILED && !state.error_message.empty()) { + result["error"] = state.error_message; + } + return result; + } + return {{"status", "idle"}}; +} + +std::string Rebuild::formatTime(std::chrono::system_clock::time_point tp) { + return timeToISO8601(tp); +} + +std::string Rebuild::getTempPath(const std::string& index_dir) { + return index_dir + "/vectors/" + settings::DEFAULT_SUBINDEX + ".idx.temp"; +} + +std::string Rebuild::getTimestampedPath(const std::string& index_dir) { + auto ts = std::to_string( + std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch() + ).count() + ); + return index_dir + "/vectors/" + settings::DEFAULT_SUBINDEX + ".idx." + ts; +} + +void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { + try { + std::unique_lock op_lock(*p.operation_mutex); + + // Phase 1 — save current state before rebuilding + p.save_current_index(); + + // Phase 2 — build new HNSW with updated M/ef_con + auto new_alg = std::make_unique>( + p.max_elements, p.space_type, p.dim, p.new_M, p.new_ef_con, + settings::RANDOM_SEED, p.quant_level, p.checksum); + + // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes + p.wire_fetchers(new_alg.get(), p.vector_storage); + + auto cursor = p.vector_storage->getCursor(); + const size_t batch_size = settings::RECOVERY_BATCH_SIZE; + size_t total_processed = 0; + size_t batches_since_checkpoint = 0; + constexpr size_t CHECKPOINT_INTERVAL = 5; + + while (cursor.hasNext()) { + if (st.stop_requested()) { + if (std::filesystem::exists(p.temp_path)) + std::filesystem::remove(p.temp_path); + failActiveRebuild(p.username, "Rebuild interrupted by server shutdown"); + return; + } + + std::vector>> batch; + batch.reserve(batch_size); + while (cursor.hasNext() && batch.size() < batch_size) { + auto [label, vec_bytes] = cursor.next(); + if (!vec_bytes.empty()) + batch.emplace_back(label, std::move(vec_bytes)); + } + if (batch.empty()) break; + + p.parallel_add(batch.size(), p.num_parallel_inserts, + [&](size_t i) { + const auto& [label, vec_bytes] = batch[i]; + new_alg->addPoint(vec_bytes.data(), label); + }); + + total_processed += batch.size(); + updateProgress(p.username, total_processed); + + if (++batches_since_checkpoint >= CHECKPOINT_INTERVAL) { + new_alg->saveIndex(p.temp_path); + batches_since_checkpoint = 0; + } + } + + // Phase 3 — save final, copy to canonical path, load fresh from disk + new_alg->saveIndex(p.timestamped_path); + std::filesystem::copy_file(p.timestamped_path, p.index_path, + std::filesystem::copy_options::overwrite_existing); + + // Cannot call reloadIndex() here — we hold operation_mutex and reloadIndex acquires + // indices_mutex_, while deleteIndex holds indices_mutex_ then acquires operation_mutex. + // Calling reloadIndex here would deadlock with a concurrent delete on the same index. + auto fresh_alg = std::make_unique>(p.index_path, 0); + p.wire_fetchers(fresh_alg.get(), p.vector_storage); + + // Both files are deleted here on success. If the server crashes before reaching this + // point, the timestamped file (default.idx.) will be removed on next startup + // by cleanupTempFiles — it does not affect index correctness. + if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); + if (std::filesystem::exists(p.timestamped_path)) std::filesystem::remove(p.timestamped_path); + + p.swap_alg(std::move(fresh_alg)); + p.update_metadata(p.new_M, p.new_ef_con); + p.clear_dirty(); + + LOG_INFO(1801, p.index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); + completeActiveRebuild(p.username); + + } catch (const std::exception& e) { + LOG_ERROR(1802, p.index_id, "Rebuild failed: " << e.what()); + if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); + failActiveRebuild(p.username, e.what()); + } +} diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index 97fdd369ae..9b474a1475 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -7,20 +7,14 @@ #include #include #include -#include -#include -#include -#include -#include #include +#include +#include -#include "settings.hpp" -#include "log.hpp" #include "json/nlohmann_json.hpp" #include "hnsw/hnswlib.h" #include "vector_storage.hpp" #include "../quant/common.hpp" -#include "utils/types.hpp" enum class RebuildStatus : unsigned char { IN_PROGRESS = 0, @@ -80,270 +74,30 @@ struct RebuildJobParams { class Rebuild { private: - // Keyed by username — one rebuild per user at a time std::unordered_map> active_rebuilds_; mutable std::mutex rebuild_state_mutex_; - static std::string statusToString(RebuildStatus s) { - switch (s) { - case RebuildStatus::IN_PROGRESS: return "in_progress"; - case RebuildStatus::COMPLETED: return "completed"; - case RebuildStatus::FAILED: return "failed"; - default: return "unknown"; - } - } - - static std::string timeToISO8601(std::chrono::system_clock::time_point tp) { - auto time_t_val = std::chrono::system_clock::to_time_t(tp); - std::tm tm_val{}; - gmtime_r(&time_t_val, &tm_val); - std::ostringstream oss; - oss << std::put_time(&tm_val, "%Y-%m-%dT%H:%M:%SZ"); - return oss.str(); - } + static std::string statusToString(RebuildStatus s); + static std::string timeToISO8601(std::chrono::system_clock::time_point tp); public: Rebuild() = default; - // Lifecycle — cleanup temp files from interrupted rebuilds on startup - void cleanupTempFiles(const std::string& data_dir) { - if (!std::filesystem::exists(data_dir)) { - return; - } - try { - std::string temp_filename = std::string(settings::DEFAULT_SUBINDEX) + ".idx.temp"; - std::string ts_prefix = std::string(settings::DEFAULT_SUBINDEX) + ".idx."; - for (const auto& entry : std::filesystem::recursive_directory_iterator(data_dir)) { - if (!entry.is_regular_file()) continue; - const std::string fname = entry.path().filename().string(); - bool is_temp = (fname == temp_filename); - bool is_ts = fname.size() > ts_prefix.size() - && fname.substr(0, ts_prefix.size()) == ts_prefix - && std::all_of(fname.begin() + ts_prefix.size(), fname.end(), ::isdigit); - if (is_temp || is_ts) { - std::filesystem::remove(entry.path()); - } - } - } catch (const std::exception& e) { - LOG_WARN(1803, "rebuild", "Failed to cleanup temp files on startup: " << e.what()); - } - } - - // State tracking — per user + void cleanupTempFiles(const std::string& data_dir); void setActiveRebuild(const std::string& username, const std::string& index_id, - size_t total_vectors) { - std::lock_guard lock(rebuild_state_mutex_); - auto state = std::make_shared(); - state->index_id = index_id; - state->status = RebuildStatus::IN_PROGRESS; - state->total_vectors = total_vectors; - state->vectors_processed = 0; - state->started_at = std::chrono::system_clock::now(); - active_rebuilds_[username] = state; - } - - void completeActiveRebuild(const std::string& username) { - std::lock_guard lock(rebuild_state_mutex_); - auto it = active_rebuilds_.find(username); - if (it != active_rebuilds_.end()) { - // Called from within the thread — detach so the jthread dtor doesn't join us - if (it->second->thread.joinable()) { - it->second->thread.detach(); - } - it->second->status = RebuildStatus::COMPLETED; - it->second->completed_at = std::chrono::system_clock::now(); - } - } - - void failActiveRebuild(const std::string& username, const std::string& error) { - std::lock_guard lock(rebuild_state_mutex_); - auto it = active_rebuilds_.find(username); - if (it != active_rebuilds_.end()) { - // Called from within the thread — detach so the jthread dtor doesn't join us - if (it->second->thread.joinable()) { - it->second->thread.detach(); - } - it->second->status = RebuildStatus::FAILED; - it->second->error_message = error; - it->second->completed_at = std::chrono::system_clock::now(); - } - } - - bool hasActiveRebuild(const std::string& username) const { - std::lock_guard lock(rebuild_state_mutex_); - auto it = active_rebuilds_.find(username); - // Only IN_PROGRESS blocks a new rebuild - return it != active_rebuilds_.end() && it->second->status == RebuildStatus::IN_PROGRESS; - } - - // Join all in-progress rebuild threads on shutdown. Mirrors BackupStore::joinAllThreads: - // move threads out under lock, request_stop + join outside lock to avoid deadlock - // (finishing threads call completeActiveRebuild which also locks rebuild_state_mutex_). - void joinAllThreads() { - std::vector threads_to_join; - { - std::lock_guard lock(rebuild_state_mutex_); - for (auto& [username, state] : active_rebuilds_) { - if (state->thread.joinable()) { - threads_to_join.push_back(std::move(state->thread)); - } - } - active_rebuilds_.clear(); - } - for (auto& t : threads_to_join) { - t.request_stop(); - if (t.joinable()) { - t.join(); - } - } - } - - void attachRebuildThread(const std::string& username, std::jthread&& thread) { - std::lock_guard lock(rebuild_state_mutex_); - auto it = active_rebuilds_.find(username); - if (it != active_rebuilds_.end()) { - it->second->thread = std::move(thread); - } - } - - void updateProgress(const std::string& username, size_t processed) { - std::lock_guard lock(rebuild_state_mutex_); - auto it = active_rebuilds_.find(username); - if (it != active_rebuilds_.end()) { - it->second->vectors_processed = processed; - } - } - - nlohmann::json getProgress(const std::string& username, const std::string& index_id) const { - std::lock_guard lock(rebuild_state_mutex_); - auto it = active_rebuilds_.find(username); - if (it != active_rebuilds_.end() && it->second->index_id == index_id) { - const auto& state = *it->second; - size_t processed = state.vectors_processed; - size_t total = state.total_vectors; - double percent = total > 0 ? (100.0 * processed / total) : 0.0; - nlohmann::json result = { - {"status", statusToString(state.status)}, - {"vectors_processed", processed}, - {"total_vectors", total}, - {"percent_complete", percent}, - {"started_at", formatTime(state.started_at)} - }; - if (state.status == RebuildStatus::COMPLETED || state.status == RebuildStatus::FAILED) { - result["completed_at"] = formatTime(state.completed_at); - } - if (state.status == RebuildStatus::FAILED && !state.error_message.empty()) { - result["error"] = state.error_message; - } - return result; - } - return {{"status", "idle"}}; - } - - // Format state as JSON fields - static std::string formatTime(std::chrono::system_clock::time_point tp) { - return timeToISO8601(tp); - } - - // Path helpers - - static std::string getTempPath(const std::string& index_dir) { - return index_dir + "/vectors/" + settings::DEFAULT_SUBINDEX + ".idx.temp"; - } - - static std::string getTimestampedPath(const std::string& index_dir) { - auto ts = std::to_string( - std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch() - ).count() - ); - return index_dir + "/vectors/" + settings::DEFAULT_SUBINDEX + ".idx." + ts; - } - - // Owns all rebuild execution. Called directly from the jthread lambda spawned in - // rebuildIndexAsync. IndexManager-specific operations come in via p callbacks. - void executeJob(const RebuildJobParams& p, std::stop_token st) { - try { - std::unique_lock op_lock(*p.operation_mutex); - - // Phase 1 — save current state before rebuilding - p.save_current_index(); - - // Phase 2 — build new HNSW with updated M/ef_con - auto new_alg = std::make_unique>( - p.max_elements, p.space_type, p.dim, p.new_M, p.new_ef_con, - settings::RANDOM_SEED, p.quant_level, p.checksum); - - // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes - p.wire_fetchers(new_alg.get(), p.vector_storage); - - auto cursor = p.vector_storage->getCursor(); - const size_t batch_size = settings::RECOVERY_BATCH_SIZE; - size_t total_processed = 0; - size_t batches_since_checkpoint = 0; - constexpr size_t CHECKPOINT_INTERVAL = 5; - - while (cursor.hasNext()) { - if (st.stop_requested()) { - if (std::filesystem::exists(p.temp_path)) - std::filesystem::remove(p.temp_path); - failActiveRebuild(p.username, "Rebuild interrupted by server shutdown"); - return; - } - - std::vector>> batch; - batch.reserve(batch_size); - while (cursor.hasNext() && batch.size() < batch_size) { - auto [label, vec_bytes] = cursor.next(); - if (!vec_bytes.empty()) - batch.emplace_back(label, std::move(vec_bytes)); - } - if (batch.empty()) break; - - p.parallel_add(batch.size(), p.num_parallel_inserts, - [&](size_t i) { - const auto& [label, vec_bytes] = batch[i]; - new_alg->addPoint(vec_bytes.data(), label); - }); - - total_processed += batch.size(); - updateProgress(p.username, total_processed); - - if (++batches_since_checkpoint >= CHECKPOINT_INTERVAL) { - new_alg->saveIndex(p.temp_path); - batches_since_checkpoint = 0; - } - } - - // Phase 3 — save final, copy to canonical path, load fresh from disk - new_alg->saveIndex(p.timestamped_path); - std::filesystem::copy_file(p.timestamped_path, p.index_path, - std::filesystem::copy_options::overwrite_existing); - - // Cannot call reloadIndex() here — we hold operation_mutex and reloadIndex acquires - // indices_mutex_, while deleteIndex holds indices_mutex_ then acquires operation_mutex. - // Calling reloadIndex here would deadlock with a concurrent delete on the same index. - auto fresh_alg = std::make_unique>(p.index_path, 0); - p.wire_fetchers(fresh_alg.get(), p.vector_storage); - - // Both files are deleted here on success. If the server crashes before reaching this - // point, the timestamped file (default.idx.) will be removed on next startup - // by cleanupTempFiles — it does not affect index correctness. - if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); - if (std::filesystem::exists(p.timestamped_path)) std::filesystem::remove(p.timestamped_path); - - p.swap_alg(std::move(fresh_alg)); - p.update_metadata(p.new_M, p.new_ef_con); - p.clear_dirty(); - - LOG_INFO(1801, p.index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); - completeActiveRebuild(p.username); - - } catch (const std::exception& e) { - LOG_ERROR(1802, p.index_id, "Rebuild failed: " << e.what()); - if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); - failActiveRebuild(p.username, e.what()); - } - } + size_t total_vectors); + void completeActiveRebuild(const std::string& username); + void failActiveRebuild(const std::string& username, const std::string& error); + bool hasActiveRebuild(const std::string& username) const; + void joinAllThreads(); + void attachRebuildThread(const std::string& username, std::jthread&& thread); + void updateProgress(const std::string& username, size_t processed); + nlohmann::json getProgress(const std::string& username, const std::string& index_id) const; + + static std::string formatTime(std::chrono::system_clock::time_point tp); + static std::string getTempPath(const std::string& index_dir); + static std::string getTimestampedPath(const std::string& index_dir); + + void executeJob(const RebuildJobParams& p, std::stop_token st); }; From 2202923697fa5e1fab6315b4dc64ddabfd5748c8 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Sat, 25 Apr 2026 15:10:56 +0530 Subject: [PATCH 09/16] Using Rebuild as friend class on Indexmanager helps to pass CacheEntry + IndexManager to executeJob instead of unpacking fields --- CMakeLists.txt | 1 + src/core/ndd.hpp | 39 +++++++++------------------------------ src/core/rebuild.cpp | 40 +++++++++++++++++++++++++++------------- src/core/rebuild.hpp | 42 +++++++++--------------------------------- 4 files changed, 46 insertions(+), 76 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ca5f37e564..1f8e27dbe0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -289,6 +289,7 @@ target_include_directories(ndd_core PRIVATE ${ASIO_INCLUDE_DIR} ${OPENSSL_INCLUDE_DIR} ${CURL_INCLUDE_DIRS} + ${LIBARCHIVE_INCLUDE_DIR} ) target_include_directories(${NDD_BINARY_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index ccee4d756e..3fc95e6fa6 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -201,6 +201,7 @@ struct PersistenceConfig { #include "utils/types.hpp" class IndexManager { + friend class Rebuild; // executeJob accesses saveIndexInternal + metadata_manager_ private: std::deque indices_list_; std::unordered_map> indices_; @@ -2360,37 +2361,15 @@ inline OperationResult IndexManager::rebuildIndexAsync(const std::string& index_ std::string vector_storage_dir = base_path + "/vectors"; RebuildJobParams params{ - .index_id = index_id, - .username = username, - .new_M = new_M, - .new_ef_con = new_ef_con, - .space_type = entry->alg->getSpaceType(), - .dim = entry->alg->getDimension(), - .quant_level = entry->alg->getQuantLevel(), - .checksum = entry->alg->getChecksum(), - .max_elements = entry->alg->getMaxElements(), - .vector_storage = entry->vector_storage, - .temp_path = Rebuild::getTempPath(base_path), - .timestamped_path = Rebuild::getTimestampedPath(base_path), - .index_path = vector_storage_dir + "/" + settings::DEFAULT_SUBINDEX + ".idx", + .username = username, + .new_M = new_M, + .new_ef_con = new_ef_con, + .entry = entry, + .manager = this, + .temp_path = Rebuild::getTempPath(base_path), + .timestamped_path = Rebuild::getTimestampedPath(base_path), + .index_path = vector_storage_dir + "/" + settings::DEFAULT_SUBINDEX + ".idx", .num_parallel_inserts = settings::NUM_PARALLEL_INSERTS, - .operation_mutex = &entry->operation_mutex, - .save_current_index = [this, entry]() { saveIndexInternal(*entry); }, - .swap_alg = [entry](auto fresh) { entry->alg = std::move(fresh); }, - .update_metadata = [this, index_id, entry](size_t nm, size_t nef) { - auto m = metadata_manager_->getMetadata(index_id); - if (m) { - m->M = nm; - m->ef_con = nef; - m->total_elements = entry->alg->getElementsCount(); - metadata_manager_->storeMetadata(index_id, *m); - } - }, - .clear_dirty = [entry]() { entry->is_dirty = false; }, - .wire_fetchers = [](auto* alg, auto vs) { IndexManager::wireVectorFetchers(alg, vs); }, - .parallel_add = [](size_t n, size_t t, std::function fn) { - IndexManager::parallelAddPoints(n, t, std::move(fn)); - }, }; // Register state FIRST with empty thread — hasActiveRebuild() returns true immediately diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp index 96cd9410ea..d5e04a2af2 100644 --- a/src/core/rebuild.cpp +++ b/src/core/rebuild.cpp @@ -7,6 +7,7 @@ #include "settings.hpp" #include "log.hpp" #include "utils/types.hpp" +#include "ndd.hpp" // CacheEntry, IndexManager (friend access) std::string Rebuild::statusToString(RebuildStatus s) { switch (s) { @@ -174,21 +175,25 @@ std::string Rebuild::getTimestampedPath(const std::string& index_dir) { } void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { + auto& entry = p.entry; // shared_ptr + auto* manager = p.manager; try { - std::unique_lock op_lock(*p.operation_mutex); + std::unique_lock op_lock(entry->operation_mutex); // Phase 1 — save current state before rebuilding - p.save_current_index(); + manager->saveIndexInternal(*entry); // Phase 2 — build new HNSW with updated M/ef_con + auto* old_alg = entry->alg.get(); auto new_alg = std::make_unique>( - p.max_elements, p.space_type, p.dim, p.new_M, p.new_ef_con, - settings::RANDOM_SEED, p.quant_level, p.checksum); + old_alg->getMaxElements(), old_alg->getSpaceType(), old_alg->getDimension(), + p.new_M, p.new_ef_con, + settings::RANDOM_SEED, old_alg->getQuantLevel(), old_alg->getChecksum()); // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes - p.wire_fetchers(new_alg.get(), p.vector_storage); + IndexManager::wireVectorFetchers(new_alg.get(), entry->vector_storage); - auto cursor = p.vector_storage->getCursor(); + auto cursor = entry->vector_storage->getCursor(); const size_t batch_size = settings::RECOVERY_BATCH_SIZE; size_t total_processed = 0; size_t batches_since_checkpoint = 0; @@ -211,7 +216,7 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { } if (batch.empty()) break; - p.parallel_add(batch.size(), p.num_parallel_inserts, + IndexManager::parallelAddPoints(batch.size(), p.num_parallel_inserts, [&](size_t i) { const auto& [label, vec_bytes] = batch[i]; new_alg->addPoint(vec_bytes.data(), label); @@ -235,7 +240,7 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { // indices_mutex_, while deleteIndex holds indices_mutex_ then acquires operation_mutex. // Calling reloadIndex here would deadlock with a concurrent delete on the same index. auto fresh_alg = std::make_unique>(p.index_path, 0); - p.wire_fetchers(fresh_alg.get(), p.vector_storage); + IndexManager::wireVectorFetchers(fresh_alg.get(), entry->vector_storage); // Both files are deleted here on success. If the server crashes before reaching this // point, the timestamped file (default.idx.) will be removed on next startup @@ -243,15 +248,24 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); if (std::filesystem::exists(p.timestamped_path)) std::filesystem::remove(p.timestamped_path); - p.swap_alg(std::move(fresh_alg)); - p.update_metadata(p.new_M, p.new_ef_con); - p.clear_dirty(); + entry->alg = std::move(fresh_alg); - LOG_INFO(1801, p.index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); + // Update metadata (uses friend access to manager->metadata_manager_) + auto m = manager->metadata_manager_->getMetadata(entry->index_id); + if (m) { + m->M = p.new_M; + m->ef_con = p.new_ef_con; + m->total_elements = entry->alg->getElementsCount(); + manager->metadata_manager_->storeMetadata(entry->index_id, *m); + } + + entry->is_dirty = false; + + LOG_INFO(1801, entry->index_id, "Rebuild completed: " << total_processed << " vectors rebuilt"); completeActiveRebuild(p.username); } catch (const std::exception& e) { - LOG_ERROR(1802, p.index_id, "Rebuild failed: " << e.what()); + LOG_ERROR(1802, entry->index_id, "Rebuild failed: " << e.what()); if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); failActiveRebuild(p.username, e.what()); } diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index 9b474a1475..e87a8ce041 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -4,17 +4,16 @@ #include #include #include -#include #include #include #include #include -#include #include "json/nlohmann_json.hpp" -#include "hnsw/hnswlib.h" -#include "vector_storage.hpp" -#include "../quant/common.hpp" + +// Forward declarations — full definitions live in ndd.hpp, included by rebuild.cpp. +struct CacheEntry; +class IndexManager; enum class RebuildStatus : unsigned char { IN_PROGRESS = 0, @@ -33,43 +32,20 @@ struct ActiveRebuild { std::jthread thread; // jthread: built-in stop_token + auto-join on destruction }; -// Parameters passed to Rebuild::executeJob. IndexManager-specific operations are -// provided as callbacks so rebuild.hpp does not need to include ndd.hpp. +// Parameters passed to Rebuild::executeJob. `entry` and `manager` give executeJob +// direct access to graph config, vector storage, mutexes, save/metadata operations. struct RebuildJobParams { - // Identity - std::string index_id; std::string username; size_t new_M; size_t new_ef_con; - // Current graph config (read from entry->alg by IndexManager before thread spawn) - hnswlib::SpaceType space_type; - size_t dim; - ndd::quant::QuantizationLevel quant_level; - int32_t checksum; - size_t max_elements; - - // Storage for vector iteration - std::shared_ptr vector_storage; + std::shared_ptr entry; // shared_ptr keeps CacheEntry alive for the rebuild duration + IndexManager* manager; // saveIndexInternal, metadata_manager_ (via friend) - // File paths std::string temp_path; std::string timestamped_path; std::string index_path; - - // Threading - size_t num_parallel_inserts; - - // Mutex pointer — executeJob acquires this for the whole job duration - std::shared_mutex* operation_mutex; - - // Callbacks for IndexManager-specific actions (avoids circular ndd.hpp include) - std::function save_current_index; - std::function>)> swap_alg; - std::function update_metadata; - std::function clear_dirty; - std::function*, std::shared_ptr)> wire_fetchers; - std::function)> parallel_add; + size_t num_parallel_inserts; }; class Rebuild { From c2adbac09d716706d48f3fd22e0e42d3cfe20b65 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Mon, 27 Apr 2026 09:43:50 +0530 Subject: [PATCH 10/16] fix(rebuild): using new_alg directly, stop_request before phase3, and doc review comments --- docs/rebuild.md | 12 +++++++++++- src/core/ndd.hpp | 2 +- src/core/rebuild.cpp | 39 ++++++++++++++++----------------------- src/core/rebuild.hpp | 1 - 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/docs/rebuild.md b/docs/rebuild.md index be6b35640f..a78342c344 100644 --- a/docs/rebuild.md +++ b/docs/rebuild.md @@ -121,4 +121,14 @@ The following parameters **cannot** be changed via rebuild (returns 400): - **One rebuild at a time per user** — cannot start a rebuild on any index while another rebuild is in progress for the same user. Also cannot run concurrently with a backup. - **Periodic checkpoints** — the in-progress graph is saved to a temp file at regular intervals. - **On completion**, the new graph replaces `default.idx`. All temporary and intermediate files are cleaned up. -- **On server restart** during an incomplete rebuild, the old index loads normally. Temp files are cleaned up automatically. The rebuild must be restarted manually. +- **On server restart** during an incomplete rebuild, the old index loads normally. Orphaned temp files are removed automatically on startup. The rebuild must be restarted manually. To confirm a rebuild was incomplete, check that M/ef_con in the index info still show the original values. + +--- + +## Capacity and Timing + +**Disk space:** Plan for roughly **2× the index file size** free. A temporary copy of the completed graph is written before being renamed into place. + +**Memory:** Both the old and new graphs are in RAM simultaneously during rebuild. Peak usage is approximately **2× the index graph size** in addition to normal vector storage. + +**Duration:** Roughly 8-10 minutes per million vectors on commodity hardware at default settings. Higher M or ef_con increases build time. The final disk save adds additional time proportional to index size. diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index 3fc95e6fa6..8d1907efdf 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -2376,7 +2376,7 @@ inline OperationResult IndexManager::rebuildIndexAsync(const std::string& index_ rebuild_.setActiveRebuild(username, index_id, current_count); // Spawn thread — lambda calls rebuild_.executeJob directly (execution lives in Rebuild) - std::jthread t([this, params = std::move(params)](std::stop_token st) mutable { + std::jthread t([this, params = std::move(params)](std::stop_token st) { rebuild_.executeJob(params, st); }); diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp index d5e04a2af2..ebdf14d7ac 100644 --- a/src/core/rebuild.cpp +++ b/src/core/rebuild.cpp @@ -14,8 +14,8 @@ std::string Rebuild::statusToString(RebuildStatus s) { case RebuildStatus::IN_PROGRESS: return "in_progress"; case RebuildStatus::COMPLETED: return "completed"; case RebuildStatus::FAILED: return "failed"; - default: return "unknown"; } + __builtin_unreachable(); } std::string Rebuild::timeToISO8601(std::chrono::system_clock::time_point tp) { @@ -45,7 +45,7 @@ void Rebuild::cleanupTempFiles(const std::string& data_dir) { std::filesystem::remove(entry.path()); } } - } catch (const std::exception& e) { + } catch (const std::filesystem::filesystem_error& e) { LOG_WARN(1803, "rebuild", "Failed to cleanup temp files on startup: " << e.what()); } } @@ -144,10 +144,10 @@ nlohmann::json Rebuild::getProgress(const std::string& username, const std::stri {"vectors_processed", processed}, {"total_vectors", total}, {"percent_complete", percent}, - {"started_at", formatTime(state.started_at)} + {"started_at", timeToISO8601(state.started_at)} }; if (state.status == RebuildStatus::COMPLETED || state.status == RebuildStatus::FAILED) { - result["completed_at"] = formatTime(state.completed_at); + result["completed_at"] = timeToISO8601(state.completed_at); } if (state.status == RebuildStatus::FAILED && !state.error_message.empty()) { result["error"] = state.error_message; @@ -157,10 +157,6 @@ nlohmann::json Rebuild::getProgress(const std::string& username, const std::stri return {{"status", "idle"}}; } -std::string Rebuild::formatTime(std::chrono::system_clock::time_point tp) { - return timeToISO8601(tp); -} - std::string Rebuild::getTempPath(const std::string& index_dir) { return index_dir + "/vectors/" + settings::DEFAULT_SUBINDEX + ".idx.temp"; } @@ -231,24 +227,21 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { } } - // Phase 3 — save final, copy to canonical path, load fresh from disk + if (st.stop_requested()) { + if (std::filesystem::exists(p.temp_path)) + std::filesystem::remove(p.temp_path); + failActiveRebuild(p.username, "Rebuild interrupted by server shutdown"); + return; + } + + // Phase 3 — persist to timestamped path, atomically rename to canonical path new_alg->saveIndex(p.timestamped_path); - std::filesystem::copy_file(p.timestamped_path, p.index_path, - std::filesystem::copy_options::overwrite_existing); - - // Cannot call reloadIndex() here — we hold operation_mutex and reloadIndex acquires - // indices_mutex_, while deleteIndex holds indices_mutex_ then acquires operation_mutex. - // Calling reloadIndex here would deadlock with a concurrent delete on the same index. - auto fresh_alg = std::make_unique>(p.index_path, 0); - IndexManager::wireVectorFetchers(fresh_alg.get(), entry->vector_storage); - - // Both files are deleted here on success. If the server crashes before reaching this - // point, the timestamped file (default.idx.) will be removed on next startup - // by cleanupTempFiles — it does not affect index correctness. + std::filesystem::rename(p.timestamped_path, p.index_path); + if (std::filesystem::exists(p.temp_path)) std::filesystem::remove(p.temp_path); - if (std::filesystem::exists(p.timestamped_path)) std::filesystem::remove(p.timestamped_path); - entry->alg = std::move(fresh_alg); + // new_alg is fully built and fetchers are already wired (line 194) — use directly + entry->alg = std::move(new_alg); // Update metadata (uses friend access to manager->metadata_manager_) auto m = manager->metadata_manager_->getMetadata(entry->index_id); diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index e87a8ce041..4bd14a44ae 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -71,7 +71,6 @@ class Rebuild { void updateProgress(const std::string& username, size_t processed); nlohmann::json getProgress(const std::string& username, const std::string& index_id) const; - static std::string formatTime(std::chrono::system_clock::time_point tp); static std::string getTempPath(const std::string& index_dir); static std::string getTimestampedPath(const std::string& index_dir); From af444b62a572c80188f2ecca4eb058a07ae642d5 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Mon, 27 Apr 2026 14:07:17 +0530 Subject: [PATCH 11/16] test cases --- src/core/rebuild.cpp | 5 +- src/utils/settings.hpp | 1 + tests/CMakeLists.txt | 34 +++++ tests/README.md | 84 +++++++++-- tests/filter_test.cpp | 2 +- tests/rebuild_test.cpp | 328 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 442 insertions(+), 12 deletions(-) create mode 100644 tests/rebuild_test.cpp diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp index ebdf14d7ac..9526d7aaf1 100644 --- a/src/core/rebuild.cpp +++ b/src/core/rebuild.cpp @@ -46,7 +46,10 @@ void Rebuild::cleanupTempFiles(const std::string& data_dir) { } } } catch (const std::filesystem::filesystem_error& e) { - LOG_WARN(1803, "rebuild", "Failed to cleanup temp files on startup: " << e.what()); + if (e.code() != std::errc::no_such_file_or_directory) + LOG_WARN(1803, "rebuild", "Error during temp cleanup: " << e.what()); + } catch (const std::exception& e) { + LOG_WARN(1803, "rebuild", "Error during temp cleanup: " << e.what()); } } diff --git a/src/utils/settings.hpp b/src/utils/settings.hpp index 9949e9109e..07210e7bc9 100644 --- a/src/utils/settings.hpp +++ b/src/utils/settings.hpp @@ -5,6 +5,7 @@ #include #include #include +#include constexpr uint64_t KB = (1024ULL); constexpr uint64_t MB = (1024ULL * KB); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0793a2e2f3..ccd5019366 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -3,6 +3,7 @@ include(FetchContent) FetchContent_Declare( googletest URL https://github.com/google/googletest/archive/refs/tags/v1.14.0.zip + DOWNLOAD_EXTRACT_TIMESTAMP TRUE ) # For Windows: Prevent overriding the parent project's compiler/linker settings set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) @@ -38,3 +39,36 @@ target_compile_definitions(ndd_filter_test PRIVATE MDB_MAXKEYSIZE=512) include(GoogleTest) gtest_discover_tests(ndd_filter_test) + +# --- ndd_rebuild_test --- +add_executable(ndd_rebuild_test rebuild_test.cpp ${LMDB_SOURCES} ${ROARING_SOURCE}) + +set_source_files_properties(${LMDB_SOURCES} PROPERTIES + COMPILE_FLAGS "-DMDBX_BUILD_SHARED_LIBRARY=0 -DMDBX_BUILD_FLAGS=\\\"NDD_EMBEDDED\\\"" +) + +target_include_directories(ndd_rebuild_test PRIVATE + ${CMAKE_SOURCE_DIR}/src + ${CMAKE_SOURCE_DIR}/src/core + ${CMAKE_SOURCE_DIR}/src/utils + ${CMAKE_SOURCE_DIR}/src/server + ${CMAKE_SOURCE_DIR}/src/storage + ${CMAKE_SOURCE_DIR}/src/filter + ${CMAKE_SOURCE_DIR}/src/sparse + ${CMAKE_SOURCE_DIR}/src/hnsw + ${CMAKE_SOURCE_DIR}/src/quant + ${CMAKE_SOURCE_DIR}/third_party + ${CMAKE_SOURCE_DIR}/third_party/json + ${CMAKE_SOURCE_DIR}/third_party/msgpack/include + ${LIBARCHIVE_INCLUDE_DIR} +) + +target_link_libraries(ndd_rebuild_test + PRIVATE + ndd_core + GTest::gtest_main +) + +target_compile_definitions(ndd_rebuild_test PRIVATE MDB_MAXKEYSIZE=512) + +gtest_discover_tests(ndd_rebuild_test) diff --git a/tests/README.md b/tests/README.md index a62ef40998..a5a04a2c0a 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,19 +1,83 @@ # Tests -This folder contains unit tests for Endee. +Unit tests for Endee. Currently two test suites: filter and rebuild. -## Build & Run +## Build & Run All Tests -From the repository root: + cmake -S . -B build -DENABLE_TESTING=ON -DUSE_NEON=ON # Apple Silicon + cmake -S . -B build -DENABLE_TESTING=ON -DUSE_AVX2=ON # Intel/AMD + cmake --build build + ctest --test-dir build --output-on-failure -1. Configure with tests enabled: - - `cmake -S . -B build -DENABLE_TESTING=ON` -2. Build the test target: - - `cmake --build build --target ndd_filter_test` -3. Run: - - `./build/tests/ndd_filter_test` +## ndd_filter_test + +Tests for the filter subsystem (categorical, numeric, boolean filtering). + +Build and run individually: + + cmake --build build --target ndd_filter_test + ./build/tests/ndd_filter_test + +Test cases: +- BucketTest: bucket serialization and deserialization +- FilterTest/CategoryFilterBasics: string category filter add and query +- FilterTest/BooleanFilterBasics: boolean filter via JSON input +- FilterTest/NumericFilterBasics: integer range queries +- FilterTest/FloatNumericFilter: float range queries +- FilterTest/MixedAndLogic: AND logic across multiple fields +- FilterTest/InOperator: $in operator with multiple values +- FilterTest/DeleteFilter: removal of categorical filters +- FilterTest/NumericDelete: removal of numeric filters + +## ndd_rebuild_test + +Unit and integration tests for the rebuild subsystem. + +Build and run individually: + + cmake --build build --target ndd_rebuild_test + ./build/tests/ndd_rebuild_test + +Test cases: + +State management (Rebuild class in isolation): +- RebuildStateTest/NoRebuild_HasActiveIsFalse +- RebuildStateTest/NoRebuild_GetProgressIsIdle +- RebuildStateTest/SetActive_HasActiveIsTrue +- RebuildStateTest/SetActive_GetProgressShowsInProgress +- RebuildStateTest/UpdateProgress_ReflectedInGetProgress +- RebuildStateTest/PercentComplete_CalculatedCorrectly +- RebuildStateTest/PercentComplete_ZeroTotal_IsZero +- RebuildStateTest/Complete_StatusIsCompleted +- RebuildStateTest/Complete_HasActiveIsFalse +- RebuildStateTest/Complete_CompletedAtPresent +- RebuildStateTest/Fail_StatusIsFailed +- RebuildStateTest/Fail_HasActiveIsFalse +- RebuildStateTest/Fail_ErrorMessagePresent +- RebuildStateTest/Fail_CompletedAtPresent +- RebuildStateTest/TwoUsers_IndependentState +- RebuildStateTest/GetProgress_WrongIndex_ReturnsIdle +- RebuildStateTest/SetActive_OverwritesPreviousCompleted + +Temp file cleanup and path helpers: +- RebuildCleanupTest/CleanupTempFiles_NonExistentDir_NoOp +- RebuildCleanupTest/CleanupTempFiles_RemovesTempFile +- RebuildCleanupTest/CleanupTempFiles_RemovesTimestampedFile +- RebuildCleanupTest/CleanupTempFiles_LeavesCanonicalIndex +- RebuildCleanupTest/CleanupTempFiles_EmptyDir_NoOp +- RebuildPathTest/GetTempPath_Format +- RebuildPathTest/GetTimestampedPath_HasTimestamp + +End-to-end rebuild via IndexManager: +- RebuildIntegrationTest/RebuildAsync_ReturnSuccessCode +- RebuildIntegrationTest/RebuildCompletes_ConfigUpdated +- RebuildIntegrationTest/RebuildCompletes_VectorCountPreserved +- RebuildIntegrationTest/RebuildWhileInProgress_Returns409Code +- RebuildIntegrationTest/RebuildNonExistentIndex_Returns404Code +- RebuildIntegrationTest/RebuildNoChange_Returns400Code ## Notes -- Tests can also be built in a dedicated tests build directory (e.g., `tests/build/`). +- Tests use real file I/O and real MDBX databases — no mocking. +- Each test creates its own temp directory and removes it on teardown. - The `tests/build/` directory is ignored by git. diff --git a/tests/filter_test.cpp b/tests/filter_test.cpp index 101be3403e..f75d51ed15 100644 --- a/tests/filter_test.cpp +++ b/tests/filter_test.cpp @@ -37,7 +37,7 @@ class FilterTest : public ::testing::Test { } // Initialize Filter - filter = std::make_unique(db_path); + filter = std::make_unique(db_path, "testuser/testidx"); } void TearDown() override { diff --git a/tests/rebuild_test.cpp b/tests/rebuild_test.cpp new file mode 100644 index 0000000000..e3a8a38f6a --- /dev/null +++ b/tests/rebuild_test.cpp @@ -0,0 +1,328 @@ +#include +#include +#include +#include +#include + +#include "rebuild.hpp" +#include "ndd.hpp" +#include "utils/msgpack_ndd.hpp" +#include "server/auth.hpp" + +namespace fs = std::filesystem; + +// ============================================================ +// Layer 1 — Rebuild state management (no IndexManager needed) +// ============================================================ + +class RebuildStateTest : public ::testing::Test { +protected: + Rebuild rebuild; +}; + +TEST_F(RebuildStateTest, NoRebuild_HasActiveIsFalse) { + EXPECT_FALSE(rebuild.hasActiveRebuild("alice")); +} + +TEST_F(RebuildStateTest, NoRebuild_GetProgressIsIdle) { + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_EQ(p["status"], "idle"); +} + +TEST_F(RebuildStateTest, SetActive_HasActiveIsTrue) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + EXPECT_TRUE(rebuild.hasActiveRebuild("alice")); +} + +TEST_F(RebuildStateTest, SetActive_GetProgressShowsInProgress) { + rebuild.setActiveRebuild("alice", "alice/idx", 200); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_EQ(p["status"], "in_progress"); + EXPECT_EQ(p["total_vectors"], 200); + EXPECT_EQ(p["vectors_processed"], 0); +} + +TEST_F(RebuildStateTest, UpdateProgress_ReflectedInGetProgress) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.updateProgress("alice", 50); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_EQ(p["vectors_processed"], 50); +} + +TEST_F(RebuildStateTest, PercentComplete_CalculatedCorrectly) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.updateProgress("alice", 50); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_DOUBLE_EQ(p["percent_complete"].get(), 50.0); +} + +TEST_F(RebuildStateTest, PercentComplete_ZeroTotal_IsZero) { + rebuild.setActiveRebuild("alice", "alice/idx", 0); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_DOUBLE_EQ(p["percent_complete"].get(), 0.0); +} + +TEST_F(RebuildStateTest, Complete_StatusIsCompleted) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.completeActiveRebuild("alice"); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_EQ(p["status"], "completed"); +} + +TEST_F(RebuildStateTest, Complete_HasActiveIsFalse) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.completeActiveRebuild("alice"); + EXPECT_FALSE(rebuild.hasActiveRebuild("alice")); +} + +TEST_F(RebuildStateTest, Complete_CompletedAtPresent) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.completeActiveRebuild("alice"); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_TRUE(p.contains("completed_at")); +} + +TEST_F(RebuildStateTest, Fail_StatusIsFailed) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.failActiveRebuild("alice", "disk full"); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_EQ(p["status"], "failed"); +} + +TEST_F(RebuildStateTest, Fail_HasActiveIsFalse) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.failActiveRebuild("alice", "disk full"); + EXPECT_FALSE(rebuild.hasActiveRebuild("alice")); +} + +TEST_F(RebuildStateTest, Fail_ErrorMessagePresent) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.failActiveRebuild("alice", "disk full"); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_EQ(p["error"], "disk full"); +} + +TEST_F(RebuildStateTest, Fail_CompletedAtPresent) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.failActiveRebuild("alice", "oom"); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_TRUE(p.contains("completed_at")); +} + +TEST_F(RebuildStateTest, TwoUsers_IndependentState) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + EXPECT_TRUE(rebuild.hasActiveRebuild("alice")); + EXPECT_FALSE(rebuild.hasActiveRebuild("bob")); + rebuild.setActiveRebuild("bob", "bob/idx", 50); + EXPECT_TRUE(rebuild.hasActiveRebuild("bob")); + rebuild.completeActiveRebuild("alice"); + EXPECT_FALSE(rebuild.hasActiveRebuild("alice")); + EXPECT_TRUE(rebuild.hasActiveRebuild("bob")); +} + +TEST_F(RebuildStateTest, GetProgress_WrongIndex_ReturnsIdle) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + auto p = rebuild.getProgress("alice", "alice/other"); + EXPECT_EQ(p["status"], "idle"); +} + +TEST_F(RebuildStateTest, SetActive_OverwritesPreviousCompleted) { + rebuild.setActiveRebuild("alice", "alice/idx", 100); + rebuild.completeActiveRebuild("alice"); + rebuild.setActiveRebuild("alice", "alice/idx", 200); + auto p = rebuild.getProgress("alice", "alice/idx"); + EXPECT_EQ(p["status"], "in_progress"); + EXPECT_EQ(p["total_vectors"], 200); +} + +// ============================================================ +// Layer 2 — Temp file cleanup and path helpers +// ============================================================ + +class RebuildCleanupTest : public ::testing::Test { +protected: + std::string dir_; + Rebuild rebuild_; + + void SetUp() override { + dir_ = "./test_rebuild_cleanup_" + std::to_string(rand()); + fs::create_directories(dir_ + "/user/idx/vectors"); + } + + void TearDown() override { + if (fs::exists(dir_)) fs::remove_all(dir_); + } + + void touch(const std::string& rel_path) { + std::ofstream f(dir_ + "/" + rel_path); + f << "x"; + } + + bool exists(const std::string& rel_path) { + return fs::exists(dir_ + "/" + rel_path); + } +}; + +TEST_F(RebuildCleanupTest, CleanupTempFiles_NonExistentDir_NoOp) { + EXPECT_NO_THROW(rebuild_.cleanupTempFiles("/nonexistent/path/xyz")); +} + +TEST_F(RebuildCleanupTest, CleanupTempFiles_RemovesTempFile) { + touch("user/idx/vectors/default.idx.temp"); + rebuild_.cleanupTempFiles(dir_); + EXPECT_FALSE(exists("user/idx/vectors/default.idx.temp")); +} + +TEST_F(RebuildCleanupTest, CleanupTempFiles_RemovesTimestampedFile) { + touch("user/idx/vectors/default.idx.1714900000"); + rebuild_.cleanupTempFiles(dir_); + EXPECT_FALSE(exists("user/idx/vectors/default.idx.1714900000")); +} + +TEST_F(RebuildCleanupTest, CleanupTempFiles_LeavesCanonicalIndex) { + touch("user/idx/vectors/default.idx"); + rebuild_.cleanupTempFiles(dir_); + EXPECT_TRUE(exists("user/idx/vectors/default.idx")); +} + +TEST_F(RebuildCleanupTest, CleanupTempFiles_EmptyDir_NoOp) { + EXPECT_NO_THROW(rebuild_.cleanupTempFiles(dir_)); +} + +TEST(RebuildPathTest, GetTempPath_Format) { + auto path = Rebuild::getTempPath("/data/user/idx"); + EXPECT_EQ(path, "/data/user/idx/vectors/default.idx.temp"); +} + +TEST(RebuildPathTest, GetTimestampedPath_HasTimestamp) { + auto path = Rebuild::getTimestampedPath("/data/user/idx"); + // Should match /data/user/idx/vectors/default.idx. + std::string prefix = "/data/user/idx/vectors/default.idx."; + ASSERT_GT(path.size(), prefix.size()); + EXPECT_EQ(path.substr(0, prefix.size()), prefix); + std::string suffix = path.substr(prefix.size()); + EXPECT_FALSE(suffix.empty()); + EXPECT_TRUE(std::all_of(suffix.begin(), suffix.end(), ::isdigit)); +} + +// ============================================================ +// Layer 3 — Integration tests via IndexManager +// ============================================================ + +class RebuildIntegrationTest : public ::testing::Test { +protected: + static constexpr const char* USERNAME = "testuser"; + static constexpr const char* IDX_NAME = "testidx"; + static constexpr const char* INDEX_ID = "testuser/testidx"; + static constexpr size_t DIM = 32; + static constexpr size_t N_VECTORS = 100; + + std::string data_dir_; + std::unique_ptr manager_; + + void SetUp() override { + data_dir_ = "./test_rebuild_integration_" + std::to_string(rand()); + fs::create_directories(data_dir_); + PersistenceConfig pcfg; + pcfg.save_on_shutdown = false; + manager_ = std::make_unique(data_dir_, pcfg); + } + + void TearDown() override { + manager_.reset(); + if (fs::exists(data_dir_)) fs::remove_all(data_dir_); + } + + void createTestIndex(size_t M = 8, size_t ef_con = 64) { + IndexConfig config{ + .dim = DIM, + .max_elements = 1000, + .space_type_str = "cosine", + .M = M, + .ef_construction = ef_con, + .quant_level = ndd::quant::QuantizationLevel::FP32, + .checksum = 0 + }; + manager_->createIndex(INDEX_ID, config, UserType::Admin, 0); + } + + void insertVectors(size_t n = N_VECTORS) { + std::vector vecs; + vecs.reserve(n); + for (size_t i = 0; i < n; ++i) { + ndd::HybridVectorObject v; + v.id = "vec_" + std::to_string(i); + v.vector.resize(DIM); + for (size_t d = 0; d < DIM; ++d) + v.vector[d] = static_cast(rand()) / RAND_MAX; + vecs.push_back(std::move(v)); + } + manager_->addVectors(INDEX_ID, vecs); + } + + // Returns true if rebuild completed successfully within timeout_sec. + bool waitForRebuild(int timeout_sec = 10) { + auto deadline = std::chrono::steady_clock::now() + + std::chrono::seconds(timeout_sec); + while (std::chrono::steady_clock::now() < deadline) { + auto progress = manager_->getRebuildProgress(USERNAME, INDEX_ID); + std::string status = progress.value("status", ""); + if (status == "completed") return true; + if (status == "failed") return false; + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + return false; + } +}; + +TEST_F(RebuildIntegrationTest, RebuildAsync_ReturnSuccessCode) { + createTestIndex(); + insertVectors(); + auto result = manager_->rebuildIndexAsync(INDEX_ID, 16, 128); + EXPECT_EQ(result.code, 0); + waitForRebuild(); +} + +TEST_F(RebuildIntegrationTest, RebuildCompletes_ConfigUpdated) { + createTestIndex(8, 64); + insertVectors(); + manager_->rebuildIndexAsync(INDEX_ID, 16, 128); + ASSERT_TRUE(waitForRebuild()); + auto meta = manager_->getMetadata(INDEX_ID); + ASSERT_TRUE(meta.has_value()); + EXPECT_EQ(meta->M, 16u); + EXPECT_EQ(meta->ef_con, 128u); +} + +TEST_F(RebuildIntegrationTest, RebuildCompletes_VectorCountPreserved) { + createTestIndex(); + insertVectors(N_VECTORS); + size_t before = manager_->getElementCount(INDEX_ID); + manager_->rebuildIndexAsync(INDEX_ID, 16, 128); + ASSERT_TRUE(waitForRebuild()); + size_t after = manager_->getElementCount(INDEX_ID); + EXPECT_EQ(before, after); +} + +TEST_F(RebuildIntegrationTest, RebuildWhileInProgress_Returns409Code) { + createTestIndex(); + insertVectors(); + // setActiveRebuild is synchronous — second call sees IN_PROGRESS before thread starts + auto r1 = manager_->rebuildIndexAsync(INDEX_ID, 16, 128); + ASSERT_EQ(r1.code, 0); + auto r2 = manager_->rebuildIndexAsync(INDEX_ID, 32, 256); + EXPECT_EQ(r2.code, 2); + waitForRebuild(); +} + +TEST_F(RebuildIntegrationTest, RebuildNonExistentIndex_Returns404Code) { + auto result = manager_->rebuildIndexAsync("testuser/doesnotexist", 16, 128); + EXPECT_EQ(result.code, 1); +} + +TEST_F(RebuildIntegrationTest, RebuildNoChange_Returns400Code) { + createTestIndex(8, 64); + insertVectors(); + auto result = manager_->rebuildIndexAsync(INDEX_ID, 8, 64); + EXPECT_EQ(result.code, 3); +} From cd39eb5e3343d4f8f5a8cc9ca359d92ab4fa43fa Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Wed, 29 Apr 2026 16:37:28 +0530 Subject: [PATCH 12/16] check for deleted vectors using idmapper getDeletedIds --- src/core/rebuild.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp index 9526d7aaf1..aca0c0201a 100644 --- a/src/core/rebuild.cpp +++ b/src/core/rebuild.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include "settings.hpp" #include "log.hpp" @@ -192,6 +193,8 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes IndexManager::wireVectorFetchers(new_alg.get(), entry->vector_storage); + auto deleted_ids_vec = entry->id_mapper->getDeletedIds(SIZE_MAX); + std::unordered_set deleted_ids(deleted_ids_vec.begin(), deleted_ids_vec.end()); auto cursor = entry->vector_storage->getCursor(); const size_t batch_size = settings::RECOVERY_BATCH_SIZE; size_t total_processed = 0; @@ -210,7 +213,7 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { batch.reserve(batch_size); while (cursor.hasNext() && batch.size() < batch_size) { auto [label, vec_bytes] = cursor.next(); - if (!vec_bytes.empty()) + if (!vec_bytes.empty() && deleted_ids.count(label) == 0) batch.emplace_back(label, std::move(vec_bytes)); } if (batch.empty()) break; From bbddd1e6e3873246cee4f6e5fa6a0d8bc2a16dd8 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Wed, 29 Apr 2026 16:58:12 +0530 Subject: [PATCH 13/16] Revert "check for deleted vectors using idmapper getDeletedIds" This reverts commit cd39eb5. Co-Authored-By: Claude Sonnet 4.6 --- src/core/rebuild.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp index aca0c0201a..9526d7aaf1 100644 --- a/src/core/rebuild.cpp +++ b/src/core/rebuild.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include "settings.hpp" #include "log.hpp" @@ -193,8 +192,6 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes IndexManager::wireVectorFetchers(new_alg.get(), entry->vector_storage); - auto deleted_ids_vec = entry->id_mapper->getDeletedIds(SIZE_MAX); - std::unordered_set deleted_ids(deleted_ids_vec.begin(), deleted_ids_vec.end()); auto cursor = entry->vector_storage->getCursor(); const size_t batch_size = settings::RECOVERY_BATCH_SIZE; size_t total_processed = 0; @@ -213,7 +210,7 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { batch.reserve(batch_size); while (cursor.hasNext() && batch.size() < batch_size) { auto [label, vec_bytes] = cursor.next(); - if (!vec_bytes.empty() && deleted_ids.count(label) == 0) + if (!vec_bytes.empty()) batch.emplace_back(label, std::move(vec_bytes)); } if (batch.empty()) break; From a1b3b69e1e01a3ba750f6c2a9616cbcff8121983 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Wed, 29 Apr 2026 20:01:34 +0530 Subject: [PATCH 14/16] fix: rebuild using delete vector ids --- src/core/rebuild.cpp | 4 +++- src/storage/id_mapper.hpp | 18 ++++++++++++++++++ tests/README.md | 1 + tests/rebuild_test.cpp | 15 +++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp index 9526d7aaf1..daf78c6268 100644 --- a/src/core/rebuild.cpp +++ b/src/core/rebuild.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include "settings.hpp" #include "log.hpp" @@ -192,6 +193,7 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { // MUST wire fetchers before addPoint — searchBaseLayer needs this for base-layer-only nodes IndexManager::wireVectorFetchers(new_alg.get(), entry->vector_storage); + auto deleted_ids = entry->id_mapper->getDeletedIdsSet(); auto cursor = entry->vector_storage->getCursor(); const size_t batch_size = settings::RECOVERY_BATCH_SIZE; size_t total_processed = 0; @@ -210,7 +212,7 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { batch.reserve(batch_size); while (cursor.hasNext() && batch.size() < batch_size) { auto [label, vec_bytes] = cursor.next(); - if (!vec_bytes.empty()) + if (!vec_bytes.empty() && deleted_ids.count(label) == 0) batch.emplace_back(label, std::move(vec_bytes)); } if (batch.empty()) break; diff --git a/src/storage/id_mapper.hpp b/src/storage/id_mapper.hpp index cda3109702..1aac8527d3 100644 --- a/src/storage/id_mapper.hpp +++ b/src/storage/id_mapper.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include "../core/types.hpp" #include "../utils/settings.hpp" @@ -478,6 +479,23 @@ class IDMapper { return result; } + // Read-only snapshot of deleted numeric IDs — does not consume or modify MDBX + std::unordered_set getDeletedIdsSet() const { + std::unordered_set result; + MDBX_txn* txn; + if (mdbx_txn_begin(env_, nullptr, MDBX_TXN_RDONLY, &txn) != MDBX_SUCCESS) + return result; + std::string del_key = DELETED_IDS_KEY; + MDBX_val key{const_cast(del_key.data()), del_key.size()}, val; + if (mdbx_get(txn, dbi_, &key, &val) == MDBX_SUCCESS) { + size_t count = val.iov_len / sizeof(idInt); + idInt* raw = reinterpret_cast(val.iov_base); + result.insert(raw, raw + count); + } + mdbx_txn_abort(txn); + return result; + } + // Public method to add failed IDs back to deleted_ids for reuse void reclaim_failed_ids(const std::vector& failed_ids) { add_to_deleted_ids(failed_ids); diff --git a/tests/README.md b/tests/README.md index a5a04a2c0a..b2fb31d4b2 100644 --- a/tests/README.md +++ b/tests/README.md @@ -75,6 +75,7 @@ End-to-end rebuild via IndexManager: - RebuildIntegrationTest/RebuildWhileInProgress_Returns409Code - RebuildIntegrationTest/RebuildNonExistentIndex_Returns404Code - RebuildIntegrationTest/RebuildNoChange_Returns400Code +- RebuildIntegrationTest/RebuildExcludesDeletedVectors ## Notes diff --git a/tests/rebuild_test.cpp b/tests/rebuild_test.cpp index e3a8a38f6a..c5e22400d4 100644 --- a/tests/rebuild_test.cpp +++ b/tests/rebuild_test.cpp @@ -326,3 +326,18 @@ TEST_F(RebuildIntegrationTest, RebuildNoChange_Returns400Code) { auto result = manager_->rebuildIndexAsync(INDEX_ID, 8, 64); EXPECT_EQ(result.code, 3); } + +TEST_F(RebuildIntegrationTest, RebuildExcludesDeletedVectors) { + createTestIndex(); + insertVectors(N_VECTORS); // 100 vectors: vec_0 .. vec_99 + + for (size_t i = 0; i < 10; ++i) + manager_->deleteVector(INDEX_ID, "vec_" + std::to_string(i)); + + EXPECT_EQ(manager_->getElementCount(INDEX_ID), 90u); + + manager_->rebuildIndexAsync(INDEX_ID, 16, 128); + ASSERT_TRUE(waitForRebuild()); + + EXPECT_EQ(manager_->getElementCount(INDEX_ID), 90u); +} From bee43b49fe8185d083383dfa2561161b8a09f254 Mon Sep 17 00:00:00 2001 From: Hemant Sharma Date: Sat, 2 May 2026 15:06:53 +0530 Subject: [PATCH 15/16] Using elementscount + MAX_ELEMENTS_INCREMENT as size of new index --- src/core/rebuild.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp index daf78c6268..540fd52a3a 100644 --- a/src/core/rebuild.cpp +++ b/src/core/rebuild.cpp @@ -185,8 +185,12 @@ void Rebuild::executeJob(const RebuildJobParams& p, std::stop_token st) { // Phase 2 — build new HNSW with updated M/ef_con auto* old_alg = entry->alg.get(); + // Size new graph based on live vector count + buffer, not allocated capacity. + // This ensures default.idx shrinks proportionally after deletions. + size_t live_count = old_alg->getElementsCount(); + size_t new_max = live_count + settings::MAX_ELEMENTS_INCREMENT; auto new_alg = std::make_unique>( - old_alg->getMaxElements(), old_alg->getSpaceType(), old_alg->getDimension(), + new_max, old_alg->getSpaceType(), old_alg->getDimension(), p.new_M, p.new_ef_con, settings::RANDOM_SEED, old_alg->getQuantLevel(), old_alg->getChecksum()); From bbd2065d34c47864f312e5b4c33005fedf2d6baa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 29 Jun 2026 12:12:41 +0000 Subject: [PATCH 16/16] fix: resolve merge conflicts with master branch - src/utils/types.hpp: merge HEAD OperationResult with master's ndd-namespaced template version (license header + ndd namespace + template + ok() method) - src/core/ndd.hpp: use void uploadBackup (from master), keep all rebuild methods; update OperationResult -> ndd::OperationResult<>; remove stale return value from uploadBackup implementation - src/core/rebuild.hpp/.cpp: add AGPLv3 license headers consistent with relicense commit e507080 --- src/core/ndd.hpp | 26 +++++--------------------- src/core/rebuild.cpp | 16 ++++++++++++++++ src/core/rebuild.hpp | 16 ++++++++++++++++ src/utils/types.hpp | 18 ++++-------------- 4 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/core/ndd.hpp b/src/core/ndd.hpp index 70080dd050..b388563ae7 100644 --- a/src/core/ndd.hpp +++ b/src/core/ndd.hpp @@ -1915,10 +1915,9 @@ class IndexManager { backup_store_.validateBackupName(backup_name); } -<<<<<<< HEAD - std::pair uploadBackup(const std::string& backup_name, - const std::string& username, - const std::string& file_content); + void uploadBackup(const std::string& backup_name, + const std::string& username, + const std::string& file_content); // Metadata access std::optional getMetadata(const std::string& index_id) { @@ -1939,7 +1938,7 @@ class IndexManager { // 1: index not found // 2: rebuild or backup already in progress for this user // 3: no configuration changes specified / invalid parameters - OperationResult rebuildIndexAsync(const std::string& index_id, + ndd::OperationResult<> rebuildIndexAsync(const std::string& index_id, size_t new_M, size_t new_ef_con); @@ -1987,11 +1986,6 @@ class IndexManager { return vs->get_vectors_batch_into(labels, buffers, success, count); }); } -======= - void uploadBackup(const std::string& backup_name, - const std::string& username, - const std::string& file_content); ->>>>>>> origin/master }; // ========== IndexManager backup implementations ========== @@ -2278,12 +2272,8 @@ inline std::string IndexManager::createBackupAsync(const std::string& index_id, return backup_name; } -<<<<<<< HEAD -inline std::pair IndexManager::uploadBackup(const std::string& backup_name, const std::string& username, const std::string& file_content) { -======= inline void IndexManager::uploadBackup(const std::string& backup_name, const std::string& username, const std::string& file_content) { ->>>>>>> origin/master std::string user_backup_dir = backup_store_.getUserBackupDir(username); std::filesystem::create_directories(user_backup_dir); std::string backup_path = user_backup_dir + "/" + backup_name + ".tar"; @@ -2340,14 +2330,11 @@ inline void IndexManager::uploadBackup(const std::string& backup_name, const std nlohmann::json backup_db = backup_store_.readBackupJson(username); backup_db[backup_name] = backup_json; backup_store_.writeBackupJson(username, backup_db); -<<<<<<< HEAD - - return {true, "Backup uploaded successfully"}; } // ========== IndexManager rebuild implementations ========== -inline OperationResult IndexManager::rebuildIndexAsync(const std::string& index_id, +inline ndd::OperationResult<> IndexManager::rebuildIndexAsync(const std::string& index_id, size_t new_M, size_t new_ef_con) { auto meta = metadata_manager_->getMetadata(index_id); @@ -2407,6 +2394,3 @@ inline OperationResult IndexManager::rebuildIndexAsync(const std::string& index_ LOG_INFO(1800, index_id, "Rebuild started: M=" << new_M << " ef_con=" << new_ef_con); return {0, "Rebuild started"}; } -======= -} ->>>>>>> origin/master diff --git a/src/core/rebuild.cpp b/src/core/rebuild.cpp index 540fd52a3a..56320f290b 100644 --- a/src/core/rebuild.cpp +++ b/src/core/rebuild.cpp @@ -1,3 +1,19 @@ +// Endee — high-performance vector database +// Copyright (C) 2026 Endee Labs +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + #include "rebuild.hpp" #include diff --git a/src/core/rebuild.hpp b/src/core/rebuild.hpp index 4bd14a44ae..e98d1761b5 100644 --- a/src/core/rebuild.hpp +++ b/src/core/rebuild.hpp @@ -1,3 +1,19 @@ +// Endee — high-performance vector database +// Copyright (C) 2026 Endee Labs +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + #pragma once #include diff --git a/src/utils/types.hpp b/src/utils/types.hpp index c02003562c..ec3259fd21 100644 --- a/src/utils/types.hpp +++ b/src/utils/types.hpp @@ -1,16 +1,3 @@ -<<<<<<< HEAD -#pragma once -#include - -// Generic operation result returned by async and sync operations. -// Each function documents its return codes in comments above its declaration. -// Code 0 always means success. Non-zero codes are operation-specific. -// Codes can be conglomerated into ENUMs per operation as the codebase matures. -struct OperationResult { - unsigned char code; // 0 = success, non-zero = error (operation-specific) - std::string message; -}; -======= // Endee — high-performance vector database // Copyright (C) 2026 Endee Labs // @@ -33,6 +20,10 @@ struct OperationResult { #include #include +// Generic operation result returned by async and sync operations. +// Each function documents its return codes in comments above its declaration. +// Code 0 always means success. Non-zero codes are operation-specific. +// Codes can be conglomerated into ENUMs per operation as the codebase matures. namespace ndd { template @@ -45,4 +36,3 @@ struct OperationResult { }; } // namespace ndd ->>>>>>> origin/master