Procházet zdrojové kódy

feat(unique): T11 - activate unique constraints end to end

The unique-constraint check has existed since v2.10.0 but was deliberately
unreachable: UniqueViolation was thrown from put() inside
applyDualWriteMirror, which caught every exception, swallowed it, bumped
mirror drift and flipped mirror_healthy_ - the exact v2.8.1 fault, triggered
by a constraint doing its job.

- applyDualWriteMirror catches UniqueViolation separately and rethrows
  without touching health/drift; every other exception keeps today's
  behaviour exactly.
- MemoryStore undoes its own in-memory mutation on a rejected write via a
  new mirrorDocOrUndo() helper, applied at every INSERT/UPDATE write path
  that mutates CollectionData before mirroring (insert, update, upsert,
  updateIfVersion, patchDocument, setAdd, restoreToVersion). DELETE-only
  paths need no undo: maintainIndexes only checks uniqueness on postings
  being added.
- Mapped to grpc::StatusCode::ALREADY_EXISTS in Insert, Update,
  PatchDocument, Upsert, SetAdd, RestoreVersion, RestoreToDate (the latter
  two had no try/catch at all before this).
- Exposed: CreateIndexRequest.unique, CollectionCfg.uniqueFields wired to
  set_unique_fields, ListIndexes reports unique, Client::createUniqueIndex +
  listIndexes(collection, uniqueFlags) overload, CLI `index-create --unique`.
  Declaring uniqueness over existing duplicates is refused with sample
  colliding document ids (find_duplicate_values). DropIndex strips a field
  from uniqueFields too, since the constraint has no life independent of
  its backing index.
- Armed at boot: applyIndexDeclarations() now also calls set_unique_fields,
  the same way indexedFields already does.

Tests: tests/test_dual_write_mirror.cpp gained
test_unique_violation_on_insert_rejects_and_leaves_no_trace and
test_unique_violation_on_update_reverts_to_prior_state - both confirmed to
fail for the right reason (swallowed, row left behind, health flipped,
drift bumped) before the fix.
fszontagh před 1 měsícem
rodič
revize
94d421799b

+ 35 - 5
cli/main.cpp

@@ -18,6 +18,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include <algorithm>
 #include <cerrno>
 #include <cstdio>
 #include <cstring>
@@ -605,21 +606,28 @@ bool execCommand(smartbotic::database::Client& client,
         // an operator can see that coming.
         if (cmd == "indexes") {
             if (params.empty()) { printError("usage: indexes <collection>"); return false; }
-            auto list = client.listIndexes(params[0]);
+            std::vector<bool> uniqueFlags;
+            auto list = client.listIndexes(params[0], uniqueFlags);
             if (list.empty()) {
                 std::cout << "no indexes on " << params[0] << "\n";
                 return true;
             }
-            std::cout << C_BOLD << "field                          distinct      entries"
+            std::cout << C_BOLD << "field                          distinct      entries  unique"
                       << C_RESET << "\n";
-            for (const auto& i : list) {
+            for (size_t idx = 0; idx < list.size(); ++idx) {
+                const auto& i = list[idx];
+                const bool unique = idx < uniqueFlags.size() && uniqueFlags[idx];
                 std::cout << "  " << i.field
                           << std::string(i.field.size() < 29 ? 29 - i.field.size() : 1, ' ')
                           << i.distinctValues
                           << std::string(std::to_string(i.distinctValues).size() < 13
                                              ? 13 - std::to_string(i.distinctValues).size()
                                              : 1, ' ')
-                          << i.entries << "\n";
+                          << i.entries
+                          << std::string(std::to_string(i.entries).size() < 9
+                                             ? 9 - std::to_string(i.entries).size()
+                                             : 1, ' ')
+                          << (unique ? "yes" : "no") << "\n";
             }
             return true;
         }
@@ -648,9 +656,31 @@ bool execCommand(smartbotic::database::Client& client,
 
         if (cmd == "index-create") {
             if (params.size() < 2) {
-                printError("usage: index-create <collection> <field>");
+                printError("usage: index-create <collection> <field> [--unique]");
                 return false;
             }
+            const bool unique = std::find(params.begin(), params.end(), "--unique") != params.end();
+            if (unique) {
+                // v2.11.0 T11 — a duplicate-refusal is not a generic failure:
+                // surface the examples so the operator can go fix the data
+                // rather than guess what "could not create the index" meant.
+                auto result = client.createUniqueIndex(params[0], params[1]);
+                if (!result.success) {
+                    printError(result.error);
+                    if (!result.duplicateExamples.empty()) {
+                        std::cout << "  colliding document ids: ";
+                        for (size_t i = 0; i < result.duplicateExamples.size(); ++i) {
+                            if (i) std::cout << ", ";
+                            std::cout << result.duplicateExamples[i];
+                        }
+                        std::cout << "\n";
+                    }
+                    return false;
+                }
+                std::cout << "indexed " << result.rowsIndexed << " existing row(s) on "
+                          << params[0] << "#" << params[1] << " (unique)\n";
+                return true;
+            }
             uint64_t rows = 0;
             if (!client.createIndex(params[0], params[1], rows)) {
                 printError("could not create the index (see the service log)");

+ 46 - 0
client/include/smartbotic/database/client.hpp

@@ -673,6 +673,42 @@ public:
                      uint64_t& rowsIndexed);
     bool dropIndex(const std::string& collection, const std::string& field);
 
+    /**
+     * Result of createUniqueIndex() below. A brand-new struct, not a member
+     * added to IndexDefinition or CreateIndexResponse's shape - same ABI
+     * reasoning as the note above createIndex.
+     */
+    struct CreateIndexResult {
+        bool success = false;
+        std::string error;
+        uint64_t rowsIndexed = 0;
+        bool alreadyExisted = false;
+        /// Populated only when a fresh unique declaration was refused because
+        /// the field already holds duplicate values: up to five document ids
+        /// that collide, one per colliding value.
+        std::vector<std::string> duplicateExamples;
+    };
+
+    /**
+     * Declare `field` a secondary index AND a UNIQUE constraint: two
+     * documents in `collection` may never hold the same value for it. v2.11.0.
+     *
+     * Same backfill-before-declare semantics as createIndex(); idempotent
+     * for a field that is already unique. Declaring uniqueness over a field
+     * that already holds duplicate values is REFUSED - result.success is
+     * false, result.error explains it, and result.duplicateExamples carries
+     * sample colliding document ids so the caller can go fix the data rather
+     * than guess. A refused declaration over a field that had no index
+     * before this call leaves no index behind either.
+     *
+     * A separate method rather than a `bool unique` overload of
+     * createIndex(): the richer result (duplicate examples on refusal) is
+     * worth its own name rather than an overload set that only sometimes
+     * needs it.
+     */
+    [[nodiscard]] CreateIndexResult createUniqueIndex(const std::string& collection,
+                                                       const std::string& field);
+
     /** One declared index, as reported by listIndexes(). */
     struct IndexDefinition {
         std::string field;
@@ -684,6 +720,16 @@ public:
     };
     [[nodiscard]] std::vector<IndexDefinition> listIndexes(const std::string& collection);
 
+    /**
+     * Same as listIndexes() above, but also reports which fields carry a
+     * UNIQUE constraint via a parallel out-vector (uniqueFlags[i]
+     * corresponds to the returned IndexDefinition at the same position) -
+     * an out-parameter rather than a member added to IndexDefinition, same
+     * ABI reasoning as everywhere else on this class.
+     */
+    [[nodiscard]] std::vector<IndexDefinition> listIndexes(const std::string& collection,
+                                                            std::vector<bool>& uniqueFlags);
+
     /**
      * Distinct values an indexed field holds, with how many rows hold each. v2.10.0.
      *

+ 48 - 2
client/src/client.cpp

@@ -1269,7 +1269,38 @@ public:
         return response.success();
     }
 
-    std::vector<Client::IndexDefinition> listIndexes(const std::string& collection) {
+    // v2.11.0 T11 — unique constraints.
+    Client::CreateIndexResult createUniqueIndex(const std::string& collection,
+                                                const std::string& field) {
+        smartbotic::databasepb::CreateIndexRequest request;
+        request.set_collection(qualify(collection));
+        request.set_field(field);
+        request.set_unique(true);
+        smartbotic::databasepb::CreateIndexResponse response;
+        grpc::ClientContext context;
+        setDeadline(context);
+
+        Client::CreateIndexResult out;
+        auto status = stub_->CreateIndex(&context, request, &response);
+        if (!status.ok()) {
+            out.error = status.error_message();
+            spdlog::error("Client::createUniqueIndex failed: {}", out.error);
+            return out;
+        }
+        out.success = response.success();
+        out.error = response.error();
+        out.rowsIndexed = response.rows_indexed();
+        out.alreadyExisted = response.already_existed();
+        out.duplicateExamples.assign(response.duplicate_examples().begin(),
+                                     response.duplicate_examples().end());
+        if (!out.success) {
+            spdlog::error("Client::createUniqueIndex rejected: {}", out.error);
+        }
+        return out;
+    }
+
+    std::vector<Client::IndexDefinition> listIndexes(const std::string& collection,
+                                                      std::vector<bool>* uniqueFlags) {
         smartbotic::databasepb::ListIndexesRequest request;
         request.set_collection(qualify(collection));
         smartbotic::databasepb::ListIndexesResponse response;
@@ -1283,12 +1314,17 @@ public:
             return out;
         }
         out.reserve(response.indexes_size());
+        if (uniqueFlags != nullptr) {
+            uniqueFlags->clear();
+            uniqueFlags->reserve(response.indexes_size());
+        }
         for (const auto& i : response.indexes()) {
             Client::IndexDefinition d;
             d.field = i.field();
             d.distinctValues = i.distinct_values();
             d.entries = i.entries();
             out.push_back(std::move(d));
+            if (uniqueFlags != nullptr) uniqueFlags->push_back(i.unique());
         }
         return out;
     }
@@ -2435,8 +2471,18 @@ bool Client::dropIndex(const std::string& collection, const std::string& field)
     return impl_->dropIndex(collection, field);
 }
 
+Client::CreateIndexResult Client::createUniqueIndex(const std::string& collection,
+                                                    const std::string& field) {
+    return impl_->createUniqueIndex(collection, field);
+}
+
 std::vector<Client::IndexDefinition> Client::listIndexes(const std::string& collection) {
-    return impl_->listIndexes(collection);
+    return impl_->listIndexes(collection, nullptr);
+}
+
+std::vector<Client::IndexDefinition> Client::listIndexes(const std::string& collection,
+                                                          std::vector<bool>& uniqueFlags) {
+    return impl_->listIndexes(collection, &uniqueFlags);
 }
 
 std::vector<Client::IndexValue> Client::indexValues(const std::string& collection,

+ 13 - 0
proto/database.proto

@@ -1101,6 +1101,12 @@ message CreateIndexRequest {
     // document metadata fields (_id, _created_at, _updated_at, _version) are
     // addressable too.
     string field = 2;
+    // v2.11.0 T11 — when true, the field carries a UNIQUE constraint: two
+    // documents may never hold the same value for it. Declaring this over a
+    // collection that already contains duplicate values is REFUSED (see
+    // CreateIndexResponse.duplicate_examples) rather than accepted and left
+    // silently unenforced against the data that already violates it.
+    bool unique = 3;
 }
 
 message CreateIndexResponse {
@@ -1112,6 +1118,11 @@ message CreateIndexResponse {
     uint64 rows_indexed = 3;
     // True when the index already existed - creation is idempotent.
     bool already_existed = 4;
+    // v2.11.0 T11 — set when a `unique=true` request was refused because the
+    // field already holds duplicate values. `error` explains the refusal;
+    // these are up to five example document ids that collide, one entry per
+    // colliding value, so the caller can go fix the data rather than guess.
+    repeated string duplicate_examples = 5;
 }
 
 message DropIndexRequest {
@@ -1135,6 +1146,8 @@ message IndexInfo {
     // postings are concentrated in few values will not be used by the planner.
     uint64 distinct_values = 2;
     uint64 entries = 3;
+    // v2.11.0 T11 — true when this index also carries a UNIQUE constraint.
+    bool unique = 4;
 }
 
 message ListIndexesResponse {

+ 131 - 27
service/src/database_grpc_impl.cpp

@@ -410,6 +410,11 @@ grpc::Status DatabaseGrpcImpl::Insert(
     } catch (const nlohmann::json::exception& e) {
         return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT,
                            "Invalid JSON: " + std::string(e.what()));
+    } catch (const smartbotic::db::storage::UniqueViolation& e) {
+        // v2.11.0 T11 — a duplicate is the caller's mistake, not a server
+        // fault. MemoryStore has already undone its own mutation by the
+        // time this propagates here (see mirrorDocOrUndo).
+        return grpc::Status(grpc::StatusCode::ALREADY_EXISTS, e.what());
     } catch (const std::exception& e) {
         return grpc::Status(grpc::StatusCode::INTERNAL, e.what());
     }
@@ -603,6 +608,9 @@ grpc::Status DatabaseGrpcImpl::Update(
     } catch (const nlohmann::json::exception& e) {
         return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT,
                            "Invalid JSON: " + std::string(e.what()));
+    } catch (const smartbotic::db::storage::UniqueViolation& e) {
+        // v2.11.0 T11 — see the Insert handler's note.
+        return grpc::Status(grpc::StatusCode::ALREADY_EXISTS, e.what());
     } catch (const std::exception& e) {
         return grpc::Status(grpc::StatusCode::INTERNAL, e.what());
     }
@@ -669,6 +677,9 @@ grpc::Status DatabaseGrpcImpl::PatchDocument(
     } catch (const nlohmann::json::exception& e) {
         return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT,
                            "Invalid JSON: " + std::string(e.what()));
+    } catch (const smartbotic::db::storage::UniqueViolation& e) {
+        // v2.11.0 T11 — see the Insert handler's note.
+        return grpc::Status(grpc::StatusCode::ALREADY_EXISTS, e.what());
     } catch (const std::exception& e) {
         return grpc::Status(grpc::StatusCode::INTERNAL, e.what());
     }
@@ -765,6 +776,9 @@ grpc::Status DatabaseGrpcImpl::Upsert(
     } catch (const nlohmann::json::exception& e) {
         return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT,
                            "Invalid JSON: " + std::string(e.what()));
+    } catch (const smartbotic::db::storage::UniqueViolation& e) {
+        // v2.11.0 T11 — see the Insert handler's note.
+        return grpc::Status(grpc::StatusCode::ALREADY_EXISTS, e.what());
     } catch (const std::exception& e) {
         return grpc::Status(grpc::StatusCode::INTERNAL, e.what());
     }
@@ -969,19 +983,29 @@ grpc::Status DatabaseGrpcImpl::RestoreVersion(
             std::to_string(store_.pressurePercent()) + "%); retry after backoff");
         return grpc::Status::OK;
     }
-    uint64_t newVersion = store_.restoreToVersion(
-        request->collection(), request->id(),
-        request->version(), request->actor());
+    // v2.11.0 T11 — restoreToVersion can raise UniqueViolation (a restored
+    // row can collide with a value some other row holds now). It previously
+    // ran outside any try/catch here, which would have propagated an
+    // uncaught exception straight out of the gRPC handler.
+    try {
+        uint64_t newVersion = store_.restoreToVersion(
+            request->collection(), request->id(),
+            request->version(), request->actor());
 
-    if (newVersion == 0) {
-        response->set_success(false);
-        response->set_error("Version not found");
+        if (newVersion == 0) {
+            response->set_success(false);
+            response->set_error("Version not found");
+            return grpc::Status::OK;
+        }
+
+        response->set_new_version(newVersion);
+        response->set_success(true);
         return grpc::Status::OK;
+    } catch (const smartbotic::db::storage::UniqueViolation& e) {
+        return grpc::Status(grpc::StatusCode::ALREADY_EXISTS, e.what());
+    } catch (const std::exception& e) {
+        return grpc::Status(grpc::StatusCode::INTERNAL, e.what());
     }
-
-    response->set_new_version(newVersion);
-    response->set_success(true);
-    return grpc::Status::OK;
 }
 
 grpc::Status DatabaseGrpcImpl::RestoreToDate(
@@ -1006,20 +1030,28 @@ grpc::Status DatabaseGrpcImpl::RestoreToDate(
             std::to_string(store_.pressurePercent()) + "%); retry after backoff");
         return grpc::Status::OK;
     }
-    auto [restoredVersion, newVersion] = store_.restoreToDate(
-        request->collection(), request->id(),
-        request->timestamp(), request->actor());
+    // v2.11.0 T11 — restoreToDate delegates to restoreToVersion, which can
+    // raise UniqueViolation. Same reasoning as RestoreVersion above.
+    try {
+        auto [restoredVersion, newVersion] = store_.restoreToDate(
+            request->collection(), request->id(),
+            request->timestamp(), request->actor());
 
-    if (newVersion == 0) {
-        response->set_success(false);
-        response->set_error("No version found at the given timestamp");
+        if (newVersion == 0) {
+            response->set_success(false);
+            response->set_error("No version found at the given timestamp");
+            return grpc::Status::OK;
+        }
+
+        response->set_restored_version(restoredVersion);
+        response->set_new_version(newVersion);
+        response->set_success(true);
         return grpc::Status::OK;
+    } catch (const smartbotic::db::storage::UniqueViolation& e) {
+        return grpc::Status(grpc::StatusCode::ALREADY_EXISTS, e.what());
+    } catch (const std::exception& e) {
+        return grpc::Status(grpc::StatusCode::INTERNAL, e.what());
     }
-
-    response->set_restored_version(restoredVersion);
-    response->set_new_version(newVersion);
-    response->set_success(true);
-    return grpc::Status::OK;
 }
 
 // ===== Batch Operations =====
@@ -1527,9 +1559,18 @@ grpc::Status DatabaseGrpcImpl::SetAdd(
     if (store_.pressure() == MemoryPressure::Emergency) {
         return memoryEmergencyStatus("SetAdd", store_);
     }
-    bool added = store_.setAdd(request->collection(), request->set_id(), request->member());
-    response->set_added(added);
-    return grpc::Status::OK;
+    // v2.11.0 T11 — setAdd can raise UniqueViolation if a unique field is
+    // declared over this collection; without this, an uncaught exception
+    // would propagate straight out of the gRPC handler.
+    try {
+        bool added = store_.setAdd(request->collection(), request->set_id(), request->member());
+        response->set_added(added);
+        return grpc::Status::OK;
+    } catch (const smartbotic::db::storage::UniqueViolation& e) {
+        return grpc::Status(grpc::StatusCode::ALREADY_EXISTS, e.what());
+    } catch (const std::exception& e) {
+        return grpc::Status(grpc::StatusCode::INTERNAL, e.what());
+    }
 }
 
 grpc::Status DatabaseGrpcImpl::SetRemove(
@@ -3214,15 +3255,57 @@ grpc::Status DatabaseGrpcImpl::CreateIndex(
         const bool existed =
             std::find(cfg.indexedFields.begin(), cfg.indexedFields.end(),
                       request->field()) != cfg.indexedFields.end();
+        const bool uniqueExisted =
+            std::find(cfg.uniqueFields.begin(), cfg.uniqueFields.end(),
+                      request->field()) != cfg.uniqueFields.end();
 
         // Backfill BEFORE declaring. Between the declaration and the backfill an
         // index is incomplete, and the planner would happily serve a query from
         // it and omit rows. Building first means it is only ever consulted once
-        // complete.
+        // complete. This also builds the index the duplicate check below reads.
         const uint64_t rows = lmdb->build_index(rc.collection, request->field());
 
+        // v2.11.0 T11 — a fresh unique request must be refused, with examples,
+        // if the field already holds duplicate values: accepting it would
+        // silently fail later writes for reasons the caller never caused (the
+        // FIRST write to any already-duplicated value would throw, for a
+        // constraint the caller believed had been accepted outright).
+        // request->unique() is additive-only (see CreateIndexRequest comment
+        // and DropIndex below) — a caller that omits it, or passes false, never
+        // un-declares an existing unique constraint on this field.
+        if (request->unique() && !uniqueExisted) {
+            auto dupes = lmdb->find_duplicate_values(rc.collection, request->field());
+            if (!dupes.empty()) {
+                // The index itself did not exist before this call - leave no
+                // trace of the refused attempt rather than a plain index the
+                // caller never asked for.
+                if (!existed) {
+                    lmdb->drop_index(rc.collection, request->field());
+                }
+                std::string msg = "cannot declare " + request->collection() + "#" +
+                    request->field() + " unique: " + std::to_string(dupes.size()) +
+                    " duplicate value(s) found, e.g. ";
+                for (size_t i = 0; i < dupes.size(); ++i) {
+                    if (i) msg += ", ";
+                    msg += dupes[i].sample_id + " (x" + std::to_string(dupes[i].count) + ")";
+                    response->add_duplicate_examples(dupes[i].sample_id);
+                }
+                response->set_success(false);
+                response->set_error(msg);
+                return grpc::Status::OK;
+            }
+        }
+
+        bool cfgChanged = false;
         if (!existed) {
             cfg.indexedFields.push_back(request->field());
+            cfgChanged = true;
+        }
+        if (request->unique() && !uniqueExisted) {
+            cfg.uniqueFields.push_back(request->field());
+            cfgChanged = true;
+        }
+        if (cfgChanged) {
             std::string err;
             if (!config_manager_.setConfig(request->collection(), cfg, err)) {
                 response->set_success(false);
@@ -3231,12 +3314,14 @@ grpc::Status DatabaseGrpcImpl::CreateIndex(
             }
         }
         lmdb->set_indexed_fields(rc.collection, cfg.indexedFields);
+        lmdb->set_unique_fields(rc.collection, cfg.uniqueFields);
 
         response->set_success(true);
         response->set_rows_indexed(rows);
         response->set_already_existed(existed);
-        spdlog::info("v2.9 index created coll={} field={} rows={} (already_existed={})",
-                     request->collection(), request->field(), rows, existed);
+        spdlog::info("v2.9 index created coll={} field={} rows={} (already_existed={}, unique={})",
+                     request->collection(), request->field(), rows, existed,
+                     request->unique() || uniqueExisted);
         return grpc::Status::OK;
     } catch (const std::invalid_argument& e) {
         return grpc::Status(grpc::StatusCode::INVALID_ARGUMENT, e.what());
@@ -3276,8 +3361,24 @@ grpc::Status DatabaseGrpcImpl::DropIndex(
         CollectionCfg cfg = config_manager_.configFor(request->collection());
         auto it = std::find(cfg.indexedFields.begin(), cfg.indexedFields.end(),
                             request->field());
+        // v2.11.0 T11 — a unique constraint has no life independent of its
+        // backing index (set_unique_fields lives on the same
+        // LmdbDocumentStore state that indexed_fields does, and
+        // maintainIndexes only runs at all when indexed_fields is
+        // non-empty), so dropping the index must drop the unique
+        // declaration with it, not strand a constraint nothing enforces.
+        auto uit = std::find(cfg.uniqueFields.begin(), cfg.uniqueFields.end(),
+                             request->field());
+        bool cfgChanged = false;
         if (it != cfg.indexedFields.end()) {
             cfg.indexedFields.erase(it);
+            cfgChanged = true;
+        }
+        if (uit != cfg.uniqueFields.end()) {
+            cfg.uniqueFields.erase(uit);
+            cfgChanged = true;
+        }
+        if (cfgChanged) {
             std::string err;
             if (!config_manager_.setConfig(request->collection(), cfg, err)) {
                 response->set_success(false);
@@ -3286,6 +3387,7 @@ grpc::Status DatabaseGrpcImpl::DropIndex(
             }
         }
         lmdb->set_indexed_fields(rc.collection, cfg.indexedFields);
+        lmdb->set_unique_fields(rc.collection, cfg.uniqueFields);
         lmdb->drop_index(rc.collection, request->field());
 
         // Idempotent: dropping an index that is not there is a success, the
@@ -3324,6 +3426,8 @@ grpc::Status DatabaseGrpcImpl::ListIndexes(
             }
             auto* info = response->add_indexes();
             info->set_field(f);
+            info->set_unique(std::find(cfg.uniqueFields.begin(), cfg.uniqueFields.end(), f) !=
+                              cfg.uniqueFields.end());
             if (lmdb != nullptr) {
                 if (auto st = lmdb->index_stats(rc.collection, f)) {
                     info->set_distinct_values(st->distinct_values);

+ 10 - 2
service/src/database_service.cpp

@@ -349,9 +349,17 @@ void DatabaseService::applyIndexDeclarations() {
                 dynamic_cast<smartbotic::db::storage::LmdbDocumentStore*>(ds);
             if (lmdb == nullptr) continue;
             lmdb->set_indexed_fields(rc.collection, cfg.indexedFields);
+            // v2.11.0 T11 — arm uniqueFields the same way. This is the fix
+            // for the exact failure mode the type was built to avoid:
+            // uniqueFields has been persisted since T8 with no RPC or proto
+            // surface, which was safe (nothing consulted it); now that
+            // set_unique_fields is reachable, a restart that skipped this
+            // call would silently stop enforcing a constraint every write
+            // handler still advertises as active.
+            lmdb->set_unique_fields(rc.collection, cfg.uniqueFields);
             ++applied;
-            spdlog::info("v2.9 index: {} field(s) active on {}",
-                         cfg.indexedFields.size(), qualified);
+            spdlog::info("v2.9 index: {} field(s) active on {} ({} unique)",
+                         cfg.indexedFields.size(), qualified, cfg.uniqueFields.size());
 
             // Self-heal a declared index whose sub-db is absent. That happens
             // when the KEY FORMAT VERSION in the sub-db prefix changes (v2.9.1

+ 171 - 10
service/src/memory_store.cpp

@@ -5,6 +5,7 @@
 #include "persistence/history_store.hpp"
 #include "project_addressing.hpp"
 #include "storage/document_store.hpp"
+#include "storage/document_store_lmdb.hpp"
 #include "storage/dual_write_mirror.hpp"
 #include "storage/filter_eval.hpp"
 
@@ -364,7 +365,18 @@ std::string MemoryStore::insert(const std::string& collection, Document doc) {
     uint64_t docSize = estimateDocumentSize(doc);
 
     // v2.0 dual-write under lock — see header comment on mirrorWriteToDocStore.
-    mirrorWriteToDocStore(collection, docId, doc, EventType::INSERT);
+    // v2.11.0 T11 — routed through mirrorDocOrUndo: a UniqueViolation must
+    // undo the map insert, the vector, and the expiration index entry added
+    // above, so a rejected insert leaves no trace of ever having happened.
+    mirrorDocOrUndo(collection, docId, doc, EventType::INSERT, [&]() {
+        if (doc.expiresAt > 0) {
+            removeFromExpirationIndex(*coll, docId, doc.expiresAt);
+        }
+        if (!vec.empty()) {
+            removeVector(*coll, docId);
+        }
+        coll->documents.erase(docId);
+    });
     if (!vec.empty()) {
         mirrorVectorToDocStore(collection, docId, &vec, EventType::INSERT);
     }
@@ -488,6 +500,15 @@ bool MemoryStore::update(const std::string& collection, const std::string& id, c
     // Track memory change (old size)
     uint64_t oldSize = estimateDocumentSize(it->second);
 
+    // v2.11.0 T11 — snapshot enough of the pre-write state to undo, in case
+    // the mirror rejects this as a UniqueViolation. Captured before anything
+    // is mutated: the document itself, and whether/what vector it held.
+    const Document original = it->second;
+    std::optional<std::vector<float>> originalVec;
+    if (auto vit = coll->vectors.find(id); vit != coll->vectors.end()) {
+        originalVec = vit->second;
+    }
+
     // Remove from old expiration index
     if (it->second.expiresAt > 0) {
         removeFromExpirationIndex(*coll, id, it->second.expiresAt);
@@ -546,7 +567,23 @@ bool MemoryStore::update(const std::string& collection, const std::string& id, c
     uint64_t newSize = estimateDocumentSize(updated);
 
     // v2.0 dual-write under lock
-    mirrorWriteToDocStore(collection, id, updated, EventType::UPDATE);
+    // v2.11.0 T11 — a UniqueViolation here must put the document, its
+    // vector, and the expiration index back exactly as `original` had them,
+    // so a rejected update leaves the row as if the update never happened.
+    mirrorDocOrUndo(collection, id, updated, EventType::UPDATE, [&]() {
+        if (updated.expiresAt > 0) {
+            removeFromExpirationIndex(*coll, id, updated.expiresAt);
+        }
+        it->second = original;
+        if (original.expiresAt > 0) {
+            addToExpirationIndex(*coll, id, original.expiresAt);
+        }
+        if (originalVec.has_value()) {
+            storeVector(*coll, id, *originalVec);
+        } else {
+            removeVector(*coll, id);
+        }
+    });
     if (!vec.empty()) {
         mirrorVectorToDocStore(collection, id, &vec, EventType::UPDATE);
     } else {
@@ -608,6 +645,19 @@ std::string MemoryStore::upsert(const std::string& collection, Document doc) {
     uint64_t oldSize = 0;
 
     auto it = coll->documents.find(docId);
+
+    // v2.11.0 T11 — snapshot pre-write state for undo before either branch
+    // mutates anything. Only meaningful on the update branch (isInsert=false
+    // undo just erases what it added), but cheap enough to always capture.
+    const bool hadExisting = it != coll->documents.end();
+    const Document originalDoc = hadExisting ? it->second : Document{};
+    std::optional<std::vector<float>> originalVec;
+    if (hadExisting) {
+        if (auto vit = coll->vectors.find(docId); vit != coll->vectors.end()) {
+            originalVec = vit->second;
+        }
+    }
+
     if (it == coll->documents.end()) {
         // Insert
         isInsert = true;
@@ -664,8 +714,29 @@ std::string MemoryStore::upsert(const std::string& collection, Document doc) {
     uint64_t newSize = estimateDocumentSize(doc);
 
     // v2.0 dual-write under lock (upsert path — INSERT or UPDATE depending on isInsert)
-    mirrorWriteToDocStore(collection, docId, doc,
-                          isInsert ? EventType::INSERT : EventType::UPDATE);
+    // v2.11.0 T11 — on UniqueViolation, undo whichever branch above ran: an
+    // insert erases the row it added, an update restores the row (and its
+    // vector/expiration index) to the pre-upsert snapshot.
+    mirrorDocOrUndo(collection, docId, doc,
+                    isInsert ? EventType::INSERT : EventType::UPDATE, [&]() {
+        if (doc.expiresAt > 0) {
+            removeFromExpirationIndex(*coll, docId, doc.expiresAt);
+        }
+        if (isInsert) {
+            if (!vec.empty()) removeVector(*coll, docId);
+            coll->documents.erase(docId);
+        } else {
+            coll->documents[docId] = originalDoc;
+            if (originalDoc.expiresAt > 0) {
+                addToExpirationIndex(*coll, docId, originalDoc.expiresAt);
+            }
+            if (originalVec.has_value()) {
+                storeVector(*coll, docId, *originalVec);
+            } else {
+                removeVector(*coll, docId);
+            }
+        }
+    });
     if (!vec.empty()) {
         mirrorVectorToDocStore(collection, docId, &vec,
                                isInsert ? EventType::INSERT : EventType::UPDATE);
@@ -792,6 +863,13 @@ bool MemoryStore::updateIfVersion(const std::string& collection, const std::stri
     // Track memory change (old size)
     uint64_t oldSize = estimateDocumentSize(it->second);
 
+    // v2.11.0 T11 — snapshot for undo, same reasoning as update() above.
+    const Document original = it->second;
+    std::optional<std::vector<float>> originalVec;
+    if (auto vit = coll->vectors.find(id); vit != coll->vectors.end()) {
+        originalVec = vit->second;
+    }
+
     // Remove from old expiration index
     if (it->second.expiresAt > 0) {
         removeFromExpirationIndex(*coll, id, it->second.expiresAt);
@@ -858,7 +936,22 @@ bool MemoryStore::updateIfVersion(const std::string& collection, const std::stri
     uint64_t newSize = estimateDocumentSize(updated);
 
     // v2.0 dual-write under lock — both doc body and vector.
-    mirrorWriteToDocStore(collection, id, updated, EventType::UPDATE);
+    // v2.11.0 T11 — same undo shape as update(): restore doc/vector/expiry
+    // to the pre-write snapshot on a rejected write.
+    mirrorDocOrUndo(collection, id, updated, EventType::UPDATE, [&]() {
+        if (updated.expiresAt > 0) {
+            removeFromExpirationIndex(*coll, id, updated.expiresAt);
+        }
+        it->second = original;
+        if (original.expiresAt > 0) {
+            addToExpirationIndex(*coll, id, original.expiresAt);
+        }
+        if (originalVec.has_value()) {
+            storeVector(*coll, id, *originalVec);
+        } else {
+            removeVector(*coll, id);
+        }
+    });
     if (!vec.empty()) {
         mirrorVectorToDocStore(collection, id, &vec, EventType::UPDATE);
     } else {
@@ -905,6 +998,15 @@ uint64_t MemoryStore::patchDocument(const std::string& collection, const std::st
     // Track memory change (old size)
     uint64_t oldSize = estimateDocumentSize(it->second);
 
+    // v2.11.0 T11 — snapshot for undo. patchDocument mutates it->second in
+    // place rather than building a separate "updated" object, so the undo
+    // needs the whole pre-patch document plus whatever vector it held.
+    const Document original = it->second;
+    std::optional<std::vector<float>> originalVec;
+    if (auto vit = coll->vectors.find(id); vit != coll->vectors.end()) {
+        originalVec = vit->second;
+    }
+
     // Remove from old expiration index
     if (it->second.expiresAt > 0) {
         removeFromExpirationIndex(*coll, id, it->second.expiresAt);
@@ -959,7 +1061,26 @@ uint64_t MemoryStore::patchDocument(const std::string& collection, const std::st
     // v2.0 dual-write under lock (patch path — vector only mirrored when
     // the patch actually supplied a new _vector; otherwise the existing
     // vector stays in place on both stores)
-    mirrorWriteToDocStore(collection, id, updatedDoc, EventType::UPDATE);
+    // v2.11.0 T11 — undo restores the whole pre-patch document, then
+    // re-adds the original expiration index entry (removing whatever entry
+    // the patch attempt left behind first). Vector is only touched back if
+    // the patch attempt itself touched it.
+    mirrorDocOrUndo(collection, id, updatedDoc, EventType::UPDATE, [&]() {
+        if (it->second.expiresAt > 0) {
+            removeFromExpirationIndex(*coll, id, it->second.expiresAt);
+        }
+        it->second = original;
+        if (original.expiresAt > 0) {
+            addToExpirationIndex(*coll, id, original.expiresAt);
+        }
+        if (!vec.empty()) {
+            if (originalVec.has_value()) {
+                storeVector(*coll, id, *originalVec);
+            } else {
+                removeVector(*coll, id);
+            }
+        }
+    });
     if (!vec.empty()) {
         mirrorVectorToDocStore(collection, id, &vec, EventType::UPDATE);
     }
@@ -1224,7 +1345,13 @@ bool MemoryStore::setAdd(const std::string& collection, const std::string& setId
         coll->updatedAt = doc.updatedAt;
 
         // v2.0 dual-write under lock
-        mirrorWriteToDocStore(collection, setId, doc, EventType::INSERT);
+        // v2.11.0 T11 — undo the just-created set document on rejection.
+        mirrorDocOrUndo(collection, setId, doc, EventType::INSERT, [&]() {
+            if (doc.expiresAt > 0) {
+                removeFromExpirationIndex(*coll, setId, doc.expiresAt);
+            }
+            coll->documents.erase(setId);
+        });
 
         lock.unlock();
 
@@ -1255,6 +1382,8 @@ bool MemoryStore::setAdd(const std::string& collection, const std::string& setId
         }
     }
 
+    const Document original = it->second;
+
     saveToHistory(*coll, it->second);
     members.push_back(member);
     it->second.set_data(tree);
@@ -1266,7 +1395,10 @@ bool MemoryStore::setAdd(const std::string& collection, const std::string& setId
     Document updated = it->second;
 
     // v2.0 dual-write under lock (sadd-style: append member, update doc)
-    mirrorWriteToDocStore(collection, setId, updated, EventType::UPDATE);
+    // v2.11.0 T11 — undo restores the document to its pre-add state.
+    mirrorDocOrUndo(collection, setId, updated, EventType::UPDATE, [&]() {
+        it->second = original;
+    });
 
     lock.unlock();
 
@@ -1977,6 +2109,9 @@ uint64_t MemoryStore::restoreToVersion(const std::string& collection, const std:
     uint64_t newVersion = 0;
     Document restoredDoc;
 
+    // v2.11.0 T11 — snapshot for undo on the "active document" branch.
+    const Document originalIfActive = wasDeleted ? Document{} : docIt->second;
+
     if (!wasDeleted) {
         // Active document: save current state to history, then overwrite
         saveToHistory(*coll, docIt->second);
@@ -2027,8 +2162,23 @@ uint64_t MemoryStore::restoreToVersion(const std::string& collection, const std:
 
     // v2.0 dual-write under lock (restore path — INSERT if doc was previously
     // deleted/missing, UPDATE if we replaced an existing doc).
-    mirrorWriteToDocStore(collection, id, restoredDoc,
-                          wasDeleted ? EventType::INSERT : EventType::UPDATE);
+    // v2.11.0 T11 — undo depends on which branch ran: a restore-of-deleted
+    // erases the row it recreated, a restore-over-active puts the document
+    // back exactly as it was before the restore.
+    mirrorDocOrUndo(collection, id, restoredDoc,
+                    wasDeleted ? EventType::INSERT : EventType::UPDATE, [&]() {
+        if (restoredDoc.expiresAt > 0) {
+            removeFromExpirationIndex(*coll, id, restoredDoc.expiresAt);
+        }
+        if (wasDeleted) {
+            coll->documents.erase(id);
+        } else {
+            docIt->second = originalIfActive;
+            if (originalIfActive.expiresAt > 0) {
+                addToExpirationIndex(*coll, id, originalIfActive.expiresAt);
+            }
+        }
+    });
 
     lock.unlock();
 
@@ -2436,6 +2586,17 @@ void MemoryStore::mirrorWriteToDocStore(const std::string& collection, const std
         pc.collection, id, doc, eventType);
 }
 
+void MemoryStore::mirrorDocOrUndo(const std::string& collection, const std::string& id,
+                                   const Document& doc, EventType eventType,
+                                   const std::function<void()>& undo) {
+    try {
+        mirrorWriteToDocStore(collection, id, doc, eventType);
+    } catch (const smartbotic::db::storage::UniqueViolation&) {
+        undo();
+        throw;
+    }
+}
+
 void MemoryStore::mirrorVectorToDocStore(const std::string& collection, const std::string& id,
                                           const std::vector<float>* vec, EventType eventType) {
     if (!docStoreResolver_ || !mirrorHealthy_ || !mirrorDriftCount_) return;

+ 21 - 0
service/src/memory_store.hpp

@@ -764,6 +764,27 @@ private:
     void mirrorWriteToDocStore(const std::string& collection, const std::string& id,
                                 const std::optional<Document>& doc, EventType eventType);
 
+    // v2.11.0 T11 — mirrors an INSERT/UPDATE doc write, and on a rejected
+    // UniqueViolation runs `undo` (which must restore CollectionData to its
+    // exact pre-mutation state — the document map entry, the expiration
+    // index, and coll->vectors, whichever this call site touched before
+    // mirroring) and then rethrows. `doc` is always present for the
+    // INSERT/UPDATE case this exists for, unlike the DELETE-capable
+    // mirrorWriteToDocStore above.
+    //
+    // Every write path that mutates CollectionData BEFORE calling the mirror
+    // MUST route through this rather than mirrorWriteToDocStore directly:
+    // applyDualWriteMirror now rethrows UniqueViolation instead of
+    // swallowing it (storage/dual_write_mirror.hpp), and MemoryStore mutates
+    // its map before the mirror runs — so without an undo, a rejected write
+    // still leaves the row in memory even though the caller was told it
+    // failed. DELETE call sites don't need this: maintainIndexes only checks
+    // uniqueness on postings being ADDED, never on ones being removed, so a
+    // delete cannot raise UniqueViolation.
+    void mirrorDocOrUndo(const std::string& collection, const std::string& id,
+                         const Document& doc, EventType eventType,
+                         const std::function<void()>& undo);
+
     // v2.0 Stage 5 — mirror a vector write/delete into the LMDB sub-db
     // `_vectors_<collection>`. Called from within the per-collection
     // write lock, after the doc mirror. For INSERT/UPDATE: pass the

+ 15 - 0
service/src/storage/dual_write_mirror.hpp

@@ -27,6 +27,7 @@
 #include "document.hpp"
 #include "memory_store.hpp"
 #include "storage/document_store.hpp"
+#include "storage/document_store_lmdb.hpp"
 
 namespace smartbotic::db::storage {
 
@@ -54,6 +55,20 @@ inline void applyDualWriteMirror(
             default:
                 break;
         }
+    } catch (const UniqueViolation&) {
+        // v2.11.0 T11 — a rejected write is the CALLER's business, not a
+        // storage fault. Genuine LMDB faults still fall into the catch
+        // below and degrade the mirror; a UniqueViolation must not, on
+        // either count:
+        //   - bumping mirror_drift_count would report a data-integrity
+        //     problem that doesn't exist - the write correctly did not land.
+        //   - flipping mirror_healthy_ would send every read in the
+        //     process to MemoryStore for the rest of its life over a
+        //     single duplicate-key rejection. That's the v2.8.1 fault.
+        // Rethrown as-is so the caller (MemoryStore::insert/update/etc.)
+        // can undo its own in-memory mutation, and so it eventually reaches
+        // the gRPC handler as ALREADY_EXISTS rather than INTERNAL.
+        throw;
     } catch (const std::exception& e) {
         spdlog::error("v2.0 mirror failed coll={} id={} op={}: {}",
                       collection, id, static_cast<int>(eventType), e.what());

+ 101 - 0
tests/test_dual_write_mirror.cpp

@@ -190,6 +190,105 @@ void test_unit_insert_without_doc_is_noop() {
     check(healthy.load() && drift.load() == 0, "INSERT with nullopt doc: clean");
 }
 
+// ---------- v2.11.0 T11 — unique constraint activation ----------
+//
+// The check itself (LmdbDocumentStore::set_unique_fields +
+// maintainIndexes's UniqueViolation throw) is tested in isolation elsewhere.
+// What's under test here is the wiring: a UniqueViolation raised inside
+// put() during the mirror must (1) actually reject the write instead of
+// being swallowed like every other mirror exception, (2) leave no trace of
+// the rejected write in MemoryStore, and (3) NOT be treated as a mirror
+// fault — mirror_healthy_ and mirror_drift_count_ must be untouched. #3 is
+// the one that is easy to get wrong: it is the exact v2.8.1 failure mode
+// (a single bad write taking every read in the process off LMDB) triggered
+// by a constraint doing its job correctly.
+
+void test_unique_violation_on_insert_rejects_and_leaves_no_trace() {
+    TmpEnv tmp("unique-insert");
+    LmdbDocumentStore doc_store(tmp.env);
+    doc_store.set_indexed_fields("users", {"email"});
+    doc_store.set_unique_fields("users", {"email"});
+
+    std::atomic<bool> healthy{true};
+    std::atomic<uint64_t> drift{0};
+
+    MemoryStore::Config cfg;
+    cfg.nodeId = "test-node";
+    cfg.maxMemoryBytes = 64ULL * 1024 * 1024;
+    MemoryStore mem(cfg);
+    mem.setDocumentStoreMirror(
+        [&](std::string_view) -> DocumentStore* { return &doc_store; },
+        &healthy, &drift);
+
+    Document d1 = make_doc("u1", "users", {{"email", "alice@example.com"}});
+    std::string id1 = mem.insert("users", d1);
+    check(id1 == "u1", "unique/insert: first row with the value succeeds");
+
+    Document d2 = make_doc("u2", "users", {{"email", "alice@example.com"}});
+    bool threw = false;
+    try {
+        mem.insert("users", d2);
+    } catch (const smartbotic::db::storage::UniqueViolation&) {
+        threw = true;
+    }
+    check(threw, "unique/insert: duplicate value is rejected");
+    check(!mem.get("users", "u2").has_value(),
+          "unique/insert: no row left behind in MemoryStore");
+    check(healthy.load(),
+          "unique/insert: mirror health untouched by a rejected write");
+    check(drift.load() == 0,
+          "unique/insert: mirror drift untouched by a rejected write");
+
+    // The first row must still be intact and still readable from LMDB —
+    // a rejected second write must not have disturbed it.
+    check(mem.get("users", "u1").has_value(), "unique/insert: first row intact");
+    check(doc_store.get("users", "u1").has_value(),
+          "unique/insert: first row still in LMDB");
+}
+
+void test_unique_violation_on_update_reverts_to_prior_state() {
+    TmpEnv tmp("unique-update");
+    LmdbDocumentStore doc_store(tmp.env);
+    doc_store.set_indexed_fields("users", {"email"});
+    doc_store.set_unique_fields("users", {"email"});
+
+    std::atomic<bool> healthy{true};
+    std::atomic<uint64_t> drift{0};
+
+    MemoryStore::Config cfg;
+    cfg.nodeId = "test-node";
+    cfg.maxMemoryBytes = 64ULL * 1024 * 1024;
+    MemoryStore mem(cfg);
+    mem.setDocumentStoreMirror(
+        [&](std::string_view) -> DocumentStore* { return &doc_store; },
+        &healthy, &drift);
+
+    Document d1 = make_doc("u1", "users", {{"email", "taken@example.com"}});
+    mem.insert("users", d1);
+    Document d2 = make_doc("u2", "users", {{"email", "mine@example.com"}});
+    mem.insert("users", d2);
+
+    Document update2 = make_doc("u2", "users", {{"email", "taken@example.com"}});
+    bool threw = false;
+    try {
+        mem.update("users", "u2", update2);
+    } catch (const smartbotic::db::storage::UniqueViolation&) {
+        threw = true;
+    }
+    check(threw, "unique/update: colliding update is rejected");
+
+    auto got = mem.get("users", "u2");
+    check(got.has_value(), "unique/update: row u2 still exists");
+    check(got.has_value() && got->data().value("email", "") == "mine@example.com",
+          "unique/update: u2's email reverted to its pre-update value");
+    check(got.has_value() && got->version == 1,
+          "unique/update: version did not advance on a rejected write");
+    check(healthy.load(),
+          "unique/update: mirror health untouched by a rejected write");
+    check(drift.load() == 0,
+          "unique/update: mirror drift untouched by a rejected write");
+}
+
 // ---------- Integration via MemoryStore callback ----------
 //
 // Mirrors the DatabaseService wiring: install a callback on MemoryStore
@@ -441,6 +540,8 @@ int main() {
     test_unit_insert_update_delete_round_trip();
     test_unit_failure_marks_degraded();
     test_unit_insert_without_doc_is_noop();
+    test_unique_violation_on_insert_rejects_and_leaves_no_trace();
+    test_unique_violation_on_update_reverts_to_prior_state();
     test_integration_via_memory_store_callback();
     test_vector_mirror_round_trip();
     test_vector_mirror_empty_is_noop_on_insert();