Procházet zdrojové kódy

release(v2.4.5): namespace collection management; per-collection versioning switch

BREAKING (ABI): Client::CollectionConfig gained a member, so consumers must
rebuild. SOVERSION stays 2, matching the v2.1 precedent.

Fix 1 - createCollection / dropCollection / getCollectionInfo ignored the
project. They sent set_name(name) bare while ~30 other call sites send
set_name(qualify(...)). qualify() prepends the project unconditionally,
including "default", so no project had the two paths agree.

One createCollection("things") plus one insert("things") from the same client
produced two collections: bare "things" got the options (encrypted,
maxVersions, vectorDimension, TTL) and stayed empty, while <project>:things
was created implicitly by the insert with defaults. No error anywhere. There
is no server-side normalization - MemoryStore::createCollection stores the
name verbatim - so qualification is entirely the client's job.

Reproduced consequences: vector_dimension is immutable after creation, so a
declared vector collection never got its dimension and similaritySearch
returned 0 hits; encrypted:true silently did not apply to the collection
holding the data; and dropCollection returned TRUE while every document
stayed readable, because it dropped the phantom. Without a prior
createCollection it returned false, so the failure mode depended on call
history.

Audited every name-carrying field: exactly these three were wrong.
createProject/dropProject take project names and uploadFile/listFiles take
filenames - correctly bare. listCollections() now unqualifies results so bare
names round-trip; ListCollectionsRequest still has no project filter.

Migration: leftover empty phantom collections are harmless litter, but
swallowed options do NOT retroactively apply. A collection that needed
vector_dimension must be recreated.

Fix 2 - versioning can now be enabled/disabled on an EXISTING collection via
configureCollection. CollectionCfg::versioningEnabled defaults to true.

It lives in CollectionCfg (the _collection_meta collection) rather than
CollectionOptions because _collection_meta is an ordinary collection and so is
WAL'd and snapshotted for free; CollectionOptions would have needed a new
ALTER_COLLECTION WAL op to survive a restart between snapshots. Enforced in
one place, MemoryStore::saveToHistory, so no write path can forget it.
Disabling stops new versions and leaves existing history readable, so it is
reversible. Not the same as maxVersions==0, which means unlimited.

ConfigureCollection is now a partial update. optional bool
versioning_enabled (proto3 field presence) and empty timestamp_precision both
mean "leave unchanged". The presence is load-bearing: a plain proto3 bool
defaults to false and would have silently disabled versioning on any
precision-only call.

Test: tests/load_test/test_client_namespacing.{cpp,sh}, 17 assertions over
gRPC with a non-default project. This is the boundary both namespacing bugs
slipped through - test_views.cpp only unit-tests applyProjection() in-process
and nothing drove a real Client with Config::project set.

ctest 15/15, views e2e and TLS/auth e2e 4/4 green.
fszontagh před 1 měsícem
rodič
revize
d1653249ca

Rozdílová data souboru nebyla zobrazena, protože soubor je příliš velký
+ 0 - 0
CLAUDE.md


+ 1 - 1
VERSION

@@ -1 +1 @@
-2.4.4
+2.4.5

+ 16 - 2
client/include/smartbotic/database/client.hpp

@@ -474,11 +474,25 @@ public:
         // "ms" (default) — document _created_at/_updated_at in milliseconds since epoch
         // "ns"            — nanoseconds since epoch (use for collections with rapid writes)
         std::string timestampPrecision = "ms";
+
+        // v2.4.5 — version history on/off, settable on an EXISTING collection.
+        //
+        // Unset means "leave unchanged", so you can configure precision without
+        // touching versioning and vice versa. On getCollectionConfig() this is
+        // always populated with the collection's effective value.
+        //
+        // Disabling stops NEW versions being recorded; history already written
+        // stays readable, so the switch is reversible. This is NOT the same as
+        // maxVersions == 0, which means "keep unlimited history".
+        std::optional<bool> versioningEnabled;
     };
 
     /**
-     * Set or replace the configuration for a collection. Idempotent.
-     * Default (no explicit config) is ms precision.
+     * Update the configuration for a collection. Idempotent.
+     *
+     * v2.4.5: this is a PARTIAL update — fields left unset are preserved, not
+     * reset to defaults. An empty timestampPrecision leaves precision alone,
+     * and an unset versioningEnabled leaves versioning alone.
      *
      * Note: this does NOT migrate existing timestamps. Newly-inserted docs use
      * the new precision; old docs keep whatever they had. Run

+ 40 - 4
client/src/client.cpp

@@ -770,7 +770,14 @@ public:
     bool createCollection(const std::string& name, uint32_t defaultTtlSeconds, bool encrypted,
                           uint32_t maxVersions, uint32_t vectorDimension) {
         smartbotic::databasepb::CreateCollectionRequest request;
-        request.set_name(name);
+        // v2.4.5 — MUST qualify. Sending a bare name here while insert/get/
+        // find all qualify created TWO collections from one call site: the
+        // bare one received the options (encrypted, maxVersions,
+        // vectorDimension, TTL) and stayed empty, while the qualified one
+        // was created implicitly by the first insert with DEFAULTS. Silent,
+        // and unrecoverable for vectorDimension, which is immutable after
+        // creation. Same class of bug as the v2.4.2 createView break.
+        request.set_name(qualify(name));
         auto* options = request.mutable_options();
         if (defaultTtlSeconds > 0) {
             options->set_default_ttl_seconds(defaultTtlSeconds);
@@ -798,7 +805,11 @@ public:
 
     bool dropCollection(const std::string& name) {
         smartbotic::databasepb::DropCollectionRequest request;
-        request.set_name(name);
+        // v2.4.5 — MUST qualify. Unqualified, this dropped the empty phantom
+        // created by createCollection and returned TRUE while every document
+        // in <project>:<name> remained readable — a deletion request that
+        // reports success and deletes nothing.
+        request.set_name(qualify(name));
 
         smartbotic::databasepb::DropCollectionResponse response;
         grpc::ClientContext context;
@@ -826,12 +837,25 @@ public:
             return {};
         }
 
-        return {response.names().begin(), response.names().end()};
+        // v2.4.5 — round-trip the naming: callers pass bare names in, so they
+        // get bare names back for their OWN project. Names belonging to other
+        // projects stay visibly qualified rather than being flattened into
+        // ours (same rule as listViews). Note the server still returns every
+        // project's collections; ListCollectionsRequest has no project filter
+        // yet, unlike ListViewsRequest.
+        std::vector<std::string> out;
+        out.reserve(static_cast<size_t>(response.names_size()));
+        for (const auto& n : response.names()) {
+            out.push_back(unqualify(n));
+        }
+        return out;
     }
 
     std::optional<Client::CollectionInfo> getCollectionInfo(const std::string& name) {
         smartbotic::databasepb::GetCollectionInfoRequest request;
-        request.set_name(name);
+        // v2.4.5 — MUST qualify, else this reports on the phantom (0 docs)
+        // rather than the collection the caller reads and writes.
+        request.set_name(qualify(name));
 
         smartbotic::databasepb::GetCollectionInfoResponse response;
         grpc::ClientContext context;
@@ -927,6 +951,11 @@ public:
         smartbotic::databasepb::ConfigureCollectionRequest request;
         request.set_collection(qualify(collection));
         request.mutable_config()->set_timestamp_precision(cfg.timestampPrecision);
+        // v2.4.5 — only send versioning when the caller actually set it, so
+        // the server's partial update leaves it alone otherwise.
+        if (cfg.versioningEnabled.has_value()) {
+            request.mutable_config()->set_versioning_enabled(*cfg.versioningEnabled);
+        }
 
         smartbotic::databasepb::ConfigureCollectionResponse response;
         grpc::ClientContext context;
@@ -960,6 +989,13 @@ public:
         }
         out.timestampPrecision = response.config().timestamp_precision();
         if (out.timestampPrecision.empty()) out.timestampPrecision = "ms";
+        // v2.4.5 — always populated on read, so callers get the effective
+        // value rather than the "leave unchanged" sentinel. A server older
+        // than 2.4.5 omits the field; treat that as versioning on, which is
+        // what those builds always did.
+        out.versioningEnabled = response.config().has_versioning_enabled()
+                                    ? response.config().versioning_enabled()
+                                    : true;
         return out;
     }
 

+ 15 - 0
proto/database.proto

@@ -823,7 +823,22 @@ message GetViewInfoResponse {
 message CollectionConfig {
     // "ms" (default) — _created_at/_updated_at stamped in milliseconds since epoch
     // "ns"            — _created_at/_updated_at stamped in nanoseconds since epoch
+    // Empty on a ConfigureCollection request means "leave unchanged".
     string timestamp_precision = 1;
+
+    // v2.4.5 — version history on/off for an EXISTING collection.
+    //
+    // `optional` (proto3 field presence) is load-bearing, not decoration: a
+    // plain bool defaults to false, so any ConfigureCollection call that meant
+    // to set only timestamp_precision would silently switch versioning OFF.
+    // With presence, an absent field means "leave unchanged" and the server
+    // overlays only what the caller actually set.
+    //
+    // Disabling stops NEW versions being recorded; history already on disk
+    // stays readable, so this is reversible. Distinct from max_versions=0,
+    // which means "unlimited", not "off".
+    optional bool versioning_enabled = 2;
+
     // Room for future per-collection knobs
 }
 

+ 5 - 1
service/src/config/collection_config_manager.cpp

@@ -11,7 +11,8 @@ namespace {
 
 nlohmann::json toJson(const CollectionCfg& cfg) {
     return {
-        {"timestamp_precision", cfg.timestampPrecision}
+        {"timestamp_precision", cfg.timestampPrecision},
+        {"versioning_enabled", cfg.versioningEnabled}
     };
 }
 
@@ -19,6 +20,9 @@ CollectionCfg fromJson(const nlohmann::json& j) {
     CollectionCfg c;
     // v2.2 — default flipped to "ns" (see collection_config_manager.hpp).
     c.timestampPrecision = j.value("timestamp_precision", std::string("ns"));
+    // v2.4.5 — absent on records written before 2.4.5, which all had history
+    // on, so the default must be true.
+    c.versioningEnabled = j.value("versioning_enabled", true);
     return c;
 }
 

+ 13 - 0
service/src/config/collection_config_manager.hpp

@@ -24,6 +24,19 @@ struct CollectionCfg {
     // collections that already have a stored config keep their value;
     // only collections without an explicit config inherit the new default.
     std::string timestampPrecision = "ns";
+
+    // v2.4.5 — whether writes record version history for this collection.
+    //
+    // Defaults to true so every pre-2.4.5 collection keeps its current
+    // behaviour on upgrade. Flipping it via configureCollection() takes effect
+    // on the next write and is durable, because configs live in the
+    // `_collection_meta` collection — an ordinary collection, so it is WAL'd
+    // and snapshotted with everything else. That is why this knob lives here
+    // rather than in CollectionOptions, which would have needed a new WAL op
+    // type to survive a restart between snapshots.
+    //
+    // NOT the same as maxVersions == 0, which means "unlimited history".
+    bool versioningEnabled = true;
 };
 
 /**

+ 15 - 2
service/src/database_grpc_impl.cpp

@@ -2011,11 +2011,23 @@ grpc::Status DatabaseGrpcImpl::ConfigureCollection(
         return grpc::Status::OK;
     }
     try {
-        CollectionCfg cfg;
-        cfg.timestampPrecision = request->config().timestamp_precision();
+        // v2.4.5 — partial update. Start from the collection's CURRENT config
+        // and overlay only the fields the caller actually set, so configuring
+        // one knob never resets another. Before this, the handler built a
+        // fresh CollectionCfg, which was harmless while precision was the only
+        // field but would silently clobber versioning the moment a second
+        // field existed.
+        CollectionCfg cfg = config_manager_.configFor(request->collection());
+
+        if (!request->config().timestamp_precision().empty()) {
+            cfg.timestampPrecision = request->config().timestamp_precision();
+        }
         if (cfg.timestampPrecision.empty()) {
             cfg.timestampPrecision = "ns";  // v2.2 — default flipped from "ms"
         }
+        if (request->config().has_versioning_enabled()) {
+            cfg.versioningEnabled = request->config().versioning_enabled();
+        }
 
         // Idempotent: setConfig handles create-or-update
         std::string err;
@@ -2037,6 +2049,7 @@ grpc::Status DatabaseGrpcImpl::GetCollectionConfig(
 ) {
     auto cfg = config_manager_.configFor(request->collection());
     response->mutable_config()->set_timestamp_precision(cfg.timestampPrecision);
+    response->mutable_config()->set_versioning_enabled(cfg.versioningEnabled);
     response->set_found(config_manager_.hasExplicitConfig(request->collection()));
     return grpc::Status::OK;
 }

+ 11 - 0
service/src/memory_store.cpp

@@ -1749,6 +1749,17 @@ void MemoryStore::saveToHistory(CollectionData& coll, const Document& currentDoc
     // the dominant source of the Zoe RSS gap; see BUG-memory-leak-zoe.md.
     if (!historyStore_) return;
 
+    // v2.4.5 — per-collection versioning switch. Checked here rather than at
+    // each of the ~10 write sites that call saveToHistory, so there is exactly
+    // one place versioning can be turned off and no write path can forget.
+    // configFor() is the same O(1) cached lookup the timestamp-precision hot
+    // path uses. Disabling stops new versions; existing history stays on disk
+    // and remains readable, which is what makes the switch reversible.
+    if (configManager_ &&
+        !configManager_->configFor(currentDoc.collection).versioningEnabled) {
+        return;
+    }
+
     DocumentVersion ver;
     ver.version = currentDoc.version;
     ver.set_data(currentDoc.data());

+ 89 - 0
tests/load_test/test_client_namespacing.cpp

@@ -0,0 +1,89 @@
+// v2.4.5 client/server boundary test.
+//
+// Covers the gap that let TWO namespacing bugs ship. v2.4.2's createView break
+// and v2.4.5's createCollection/dropCollection/getCollectionInfo break were
+// both invisible to the unit suite, because tests/test_views.cpp only exercises
+// applyProjection() in-process and nothing drove a real Client against a real
+// server with a non-default project. Everything here runs over gRPC with
+// Config::project set, which is the only place these bugs are observable.
+//
+// Part 1 — collection management must be project-namespaced like insert/get/
+// find. Unqualified, createCollection created a phantom that swallowed the
+// options (vector_dimension is immutable, so that was unrecoverable) and
+// dropCollection returned true while leaving every document readable.
+//
+// Part 2 — versioning can be switched on an existing collection, is durable,
+// is reversible, and a precision-only configure must not clobber it.
+
+#include <smartbotic/database/client.hpp>
+#include <iostream>
+using namespace smartbotic::database;
+static int pass=0, fail=0;
+static void ck(bool c, const char* m){ if(c){++pass;} else {++fail; std::cerr<<"FAIL: "<<m<<"\n";} }
+
+int main() {
+    Client c({.address="127.0.0.1:9011", .project="proj1"});
+    c.connect();
+
+    // ---- Part 1: createCollection / dropCollection / getCollectionInfo namespacing
+    ck(c.createCollection("things", 0, false, 0, 4), "createCollection returns created");
+    auto cols = c.listCollections();
+    int bare=0, qual=0;
+    for (auto& n : cols) { if(n=="things") ++bare; if(n=="proj1:things") ++qual; }
+    ck(bare==1, "listCollections shows the collection under its BARE name (round-trip)");
+    ck(qual==0, "listCollections does NOT show a qualified duplicate");
+
+    c.insert("things", nlohmann::json{{"a",1}});
+    auto info = c.getCollectionInfo("things");
+    ck(info.has_value(), "getCollectionInfo finds the collection");
+    ck(info && info->documentCount==1, "getCollectionInfo counts the doc the caller inserted");
+
+    // vector_dimension must have landed on the collection that holds the data
+    c.insert("things", nlohmann::json{{"_vector", {1.0,0.0,0.0,0.0}},{"tag","x"}});
+    auto hits = c.similaritySearch("things", {1.0f,0.0f,0.0f,0.0f}, 5);
+    ck(!hits.empty(), "similaritySearch works => vector_dimension reached the real collection");
+
+    // drop must actually delete
+    c.insert("gone", nlohmann::json{{"pii","alice"}});
+    ck(c.dropCollection("gone"), "dropCollection returns true");
+    auto after = c.getCollectionInfo("gone");
+    ck(!after || after->documentCount==0, "dropCollection actually removed the data");
+
+    // ---- Part 2: versioning toggle on an EXISTING collection
+    c.createCollection("v", 0, false, 0, 0);
+    auto id = c.insert("v", nlohmann::json{{"n",1}});
+    c.update("v", id, nlohmann::json{{"n",2}});
+    c.update("v", id, nlohmann::json{{"n",3}});
+    auto h1 = c.getVersionHistory("v", id);
+    ck(h1.totalCount >= 2, "versioning ON by default records history");
+
+    Client::CollectionConfig off; off.timestampPrecision=""; off.versioningEnabled=false;
+    ck(c.configureCollection("v", off), "configureCollection(versioningEnabled=false) accepted");
+    auto cfg = c.getCollectionConfig("v");
+    ck(cfg.versioningEnabled.has_value() && *cfg.versioningEnabled==false, "config reads back disabled");
+
+    auto before = c.getVersionHistory("v", id).totalCount;
+    c.update("v", id, nlohmann::json{{"n",4}});
+    c.update("v", id, nlohmann::json{{"n",5}});
+    auto afterN = c.getVersionHistory("v", id).totalCount;
+    ck(afterN == before, "disabled: no NEW versions recorded");
+    ck(afterN >= 2, "disabled: existing history still readable (reversible)");
+
+    // partial update must not clobber versioning
+    Client::CollectionConfig justPrec; justPrec.timestampPrecision="ms";
+    ck(c.configureCollection("v", justPrec), "precision-only configure accepted");
+    auto cfg2 = c.getCollectionConfig("v");
+    ck(cfg2.versioningEnabled.has_value() && *cfg2.versioningEnabled==false,
+       "precision-only configure did NOT re-enable versioning (partial update)");
+    ck(cfg2.timestampPrecision=="ms", "precision-only configure applied precision");
+
+    // re-enable
+    Client::CollectionConfig on; on.timestampPrecision=""; on.versioningEnabled=true;
+    c.configureCollection("v", on);
+    auto b2 = c.getVersionHistory("v", id).totalCount;
+    c.update("v", id, nlohmann::json{{"n",6}});
+    ck(c.getVersionHistory("v", id).totalCount > b2, "re-enabled: versions recorded again");
+
+    std::cout << "\npassed=" << pass << " failed=" << fail << "\n";
+    return fail==0?0:1;
+}

+ 67 - 0
tests/load_test/test_client_namespacing.sh

@@ -0,0 +1,67 @@
+#!/usr/bin/env bash
+# v2.4.5 client/server boundary e2e — collection-management namespacing and the
+# per-collection versioning switch. See test_client_namespacing.cpp for what
+# each assertion covers and why the unit suite could not catch it.
+
+set -euo pipefail
+
+cd "$(dirname "$0")"
+
+ROOT=/data/smartbotic-database
+DIR=/tmp/sbdb-client-namespacing-e2e
+PORT=9011
+
+rm -rf "$DIR"
+mkdir -p "$DIR/data"
+
+cat > "$DIR/config.json" <<EOF
+{
+  "storage": {
+    "data_directory": "$DIR/data",
+    "bind_address": "127.0.0.1",
+    "rpc_port": $PORT,
+    "encryption": { "enabled": false },
+    "migrations": { "enabled": false },
+    "replication": { "enabled": false }
+  }
+}
+EOF
+
+echo "=== building test binary ==="
+g++ -std=c++20 -O1 -o "$DIR/test_client_namespacing" test_client_namespacing.cpp \
+    -I"$ROOT/client/include" -I"$ROOT/build/client" \
+    -L"$ROOT/build/client" -lsmartbotic-db-client -lspdlog -lfmt
+
+echo "=== booting server on $PORT ==="
+"$ROOT/build/service/smartbotic-database" --config "$DIR/config.json" \
+    > "$DIR/server.log" 2>&1 &
+SERVER_PID=$!
+cleanup() { kill "$SERVER_PID" 2>/dev/null || true; wait "$SERVER_PID" 2>/dev/null || true; }
+trap cleanup EXIT
+
+# Wait for readiness rather than sleeping a fixed amount.
+for _ in $(seq 1 60); do
+    grep -q "Notified systemd: READY" "$DIR/server.log" && break
+    sleep 0.5
+done
+if ! grep -q "Notified systemd: READY" "$DIR/server.log"; then
+    echo "server failed to become ready; log:"
+    tail -30 "$DIR/server.log"
+    exit 1
+fi
+
+echo "=== running assertions ==="
+set +e
+LD_LIBRARY_PATH="$ROOT/build/client" "$DIR/test_client_namespacing"
+RC=$?
+set -e
+
+if [[ $RC -ne 0 ]]; then
+    echo ""
+    echo "FAILED — server log tail:"
+    tail -30 "$DIR/server.log"
+    exit $RC
+fi
+
+echo ""
+echo "OK — client namespacing + versioning switch e2e passed"

Některé soubory nejsou zobrazeny, neboť je v těchto rozdílových datech změněno mnoho souborů