ソースを参照

fix(lmdb): an empty document id reads as absent, not as an error

A zero-length LMDB key is rejected by mdb_get with MDB_BAD_VALSIZE, so
get(coll, "") threw. The handler turned that into a gRPC INTERNAL and logged
an ERROR line on every occurrence - seen on the live instance immediately after
the 2.9.0 upgrade as:

  v2.3 Get LMDB query failed coll=smartbotic-automation:executions id=:
  LMDB get: MDB_BAD_VALSIZE

Pre-existing, not a 2.9.0 regression: nothing in this release touched Get. But
the log noise masks real failures, and a malformed request should not read as an
internal fault.

A key that cannot exist is absent, which is the same answer any other missing id
gets, so get() returns nullopt and del() returns false without touching LMDB.
Handled in the store rather than per-handler so every caller is covered.

ctest 21/21.
fszontagh 1 ヶ月 前
親
コミット
e48fcbdc18
2 ファイル変更、45 行追加、0 行削除
  1. 10 0
      service/src/storage/document_store_lmdb.cpp
  2. 35 0
      tests/test_subdb_identity.cpp

+ 10 - 0
service/src/storage/document_store_lmdb.cpp

@@ -783,6 +783,13 @@ bool LmdbDocumentStore::drop_index(std::string_view collection,
 
 std::optional<smartbotic::database::Document>
 LmdbDocumentStore::get(std::string_view collection, std::string_view id) {
+    // An empty id is a zero-length LMDB key, which mdb_get rejects with
+    // MDB_BAD_VALSIZE. Throwing turned a malformed request into a gRPC INTERNAL
+    // plus an ERROR log line on every occurrence - observed in production as
+    // "Get LMDB query failed coll=... id=:" once a consumer started asking for
+    // an empty id. A key that cannot exist is simply absent, which is the same
+    // answer any other missing id gets.
+    if (id.empty()) return std::nullopt;
     ReadTxn rtxn(env_);
     auto dbi_opt = try_open_for_read(rtxn, collection);
     if (!dbi_opt) return std::nullopt;
@@ -796,6 +803,9 @@ LmdbDocumentStore::get(std::string_view collection, std::string_view id) {
 }
 
 bool LmdbDocumentStore::del(std::string_view collection, std::string_view id) {
+    // Same reasoning as get(): a zero-length key cannot exist, so there is
+    // nothing to delete rather than an error to raise.
+    if (id.empty()) return false;
     // Open as a write txn unconditionally so we have MDB_CREATE available
     // if the collection doesn't exist yet — but in that case there's
     // nothing to delete; just probe with a read txn first to avoid

+ 35 - 0
tests/test_subdb_identity.cpp

@@ -1009,6 +1009,40 @@ void test_indexed_and_unindexed_plans_agree() {
           "the unselective index does exist and holds 150 of 300 rows");
 }
 
+// An empty document id is a zero-length LMDB key, which mdb_get rejects with
+// MDB_BAD_VALSIZE. That surfaced in production as a gRPC INTERNAL and an ERROR
+// log line every time a consumer asked for one - noise that masks real failures.
+// A key that cannot exist is absent, not an error.
+void test_empty_id_reads_as_absent() {
+    TmpEnv t("empty-id");
+    LmdbDocumentStore store(t.env);
+
+    Document d;
+    d.id = "real";
+    d.collection = "c";
+    d.set_data(nlohmann::json{{"x", 1}});
+    store.put("c", "real", d);
+
+    bool threw = false;
+    try {
+        check(!store.get("c", "").has_value(), "an empty id reads as absent");
+    } catch (const std::exception&) {
+        threw = true;
+    }
+    check(!threw, "and does NOT throw - MDB_BAD_VALSIZE became a gRPC INTERNAL");
+
+    threw = false;
+    try {
+        check(!store.del("c", ""), "deleting an empty id is a no-op");
+    } catch (const std::exception&) {
+        threw = true;
+    }
+    check(!threw, "and does not throw either");
+
+    check(store.get("c", "real").has_value(), "real ids still work");
+    check(store.count("c") == 1, "and nothing was disturbed");
+}
+
 }  // namespace
 
 int main() {
@@ -1029,6 +1063,7 @@ int main() {
     test_build_index_over_existing_rows();
     test_index_numeric_equality_matches_scan();
     test_indexed_and_unindexed_plans_agree();
+    test_empty_id_reads_as_absent();
 
     std::cout << "passed: " << g_pass << ", failed: " << g_fail << "\n";
     return g_fail == 0 ? 0 : 1;