Переглянути джерело

perf(scan): stop decoding the whole collection to return one page

Found while analysing a browser paging timeout on a 112-document, 414 MB
collection. The client-side diagnosis (page 3 is 54 MB of base64) was correct
but incomplete: the server was also doing O(collection-bytes) work per page.

LmdbDocumentStore::scan() decoded EVERY document in the collection, collected
them all into a vector, and only then sliced out `offset..offset+limit`. Cost
therefore tracked total collection bytes, not page size. Measured on the live
instance:

  collection      docs    MB     find(limit=1)
  image_hashes    1005    0.8      21 ms
  anime_images     112  413.6     382 ms
  executions      9571  505.3    2602 ms

9x more documents but 500x less data was 18x faster - it was paying for bytes.
Returning a single 510-byte document from anime_images took ~380ms, while
count() on the same collection took 1ms because it never decodes anything.

Fast path for no filters and no sort, which is exactly what a paging UI sends:
walk keys with the cursor and decode only the rows inside the window.
total_matched comes from mdb_stat, so it stays exact without touching a value.
Filtered and sorted queries keep the general path - they genuinely need every
match.

A/B on identical data, before (installed) vs after (patched):

  anime_images limit=1  off=0        319 ms  ->    1 ms
  anime_images limit=10 off=0        298 ms  ->    1 ms
  anime_images limit=10 off=100      932 ms  ->  591 ms
  executions   limit=10 off=0       2655 ms  ->    9 ms
  executions   limit=10 off=9000    2516 ms  ->    6 ms
  sessions     limit=10 off=0        125 ms  ->    1 ms

The off=100 case stays slow because those documents really are 2.5 MB each -
25 MB has to be decoded and serialised. That part is the caller's to avoid,
which brings us to the second half.

Client::QueryOptions gains `projection`. FindRequest.projection has existed on
the wire since v2.0 and fromProtoQuery has always honoured it, but the client
never exposed it - so a caller had no way to page a collection of blobs without
receiving every byte. Measured against production on the heavy page:

  full documents            1544 ms   22,791,981 bytes
  projection=[_id]           505 ms        1,530 bytes
  projection=[_id,filename]  587 ms        1,790 bytes

Combined, a browser list view goes from 22.8 MB and ~1.5s to 1.5 KB and ~1ms.

Tests: test_subdb_identity gains test_scan_fast_path_matches_general_path (38
assertions total) pinning that the fast path agrees with the general one on
total_matched, has_more, the final partial page, offset past the end, full
coverage when paging, limit=0 semantics that Count depends on, and that a
filter or a sort still forces the general path. ctest 17/17, namespacing 31/31,
views and policy enforcement green.
fszontagh 1 місяць тому
батько
коміт
bbbf56f94f

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

@@ -335,6 +335,20 @@ public:
         bool sortDescending = false;
         uint32_t limit = 100;
         uint32_t offset = 0;
+
+        // v2.7.1 — fields to return; empty means the whole document.
+        //
+        // The wire and the server have supported this since v2.0
+        // (`FindRequest.projection`), but it was never exposed here, so callers
+        // had no way to avoid transferring large fields. That matters on
+        // collections holding blobs: a browser paging through documents that
+        // each carry megabytes of base64 had to receive all of it just to render
+        // a list of ids.
+        //
+        // Projection is applied server-side AFTER pagination, so it cuts
+        // response size and client parse time; it does not change which
+        // documents match.
+        std::vector<std::string> projection;
     };
 
     /**

+ 18 - 0
client/src/client.cpp

@@ -562,6 +562,15 @@ public:
         request.set_limit(options.limit);
         request.set_offset(options.offset);
 
+        // v2.7.1 — field projection. Applied server-side after pagination, so it
+        // cuts response size and client parse time without changing which
+        // documents match. Exposed because collections holding large fields
+        // (base64 blobs) were otherwise impossible to page through: the caller
+        // had to receive every byte just to list ids.
+        for (const auto& f : options.projection) {
+            request.add_projection(f);
+        }
+
         smartbotic::databasepb::FindResponse response;
         grpc::ClientContext context;
         setDeadline(context);
@@ -628,6 +637,15 @@ public:
         request.set_limit(options.limit);
         request.set_offset(options.offset);
 
+        // v2.7.1 — field projection. Applied server-side after pagination, so it
+        // cuts response size and client parse time without changing which
+        // documents match. Exposed because collections holding large fields
+        // (base64 blobs) were otherwise impossible to page through: the caller
+        // had to receive every byte just to list ids.
+        for (const auto& f : options.projection) {
+            request.add_projection(f);
+        }
+
         smartbotic::databasepb::FindResponse response;
         grpc::ClientContext context;
         setDeadline(context);

+ 69 - 1
service/src/storage/document_store_lmdb.cpp

@@ -348,9 +348,77 @@ ScanResult LmdbDocumentStore::scan(std::string_view collection,
         ~CursorGuard() { if (c) mdb_cursor_close(c); }
     } guard{cursor};
 
-    std::vector<smartbotic::database::Document> matches;
     MDB_val k{0, nullptr};
     MDB_val v{0, nullptr};
+
+    // -------------------------------------------------------------------
+    // v2.7.1 fast path: no filters and no sort.
+    //
+    // That is exactly what a paging UI sends, and it is the case where the
+    // general path below is pathological: it decoded EVERY document in the
+    // collection to return `limit` of them, so cost tracked total collection
+    // BYTES rather than page size. Measured on a live instance: returning one
+    // 510-byte document from a 414 MB collection took ~380ms, and 2.6s from a
+    // 505 MB one, while count() on the same collections took 1ms.
+    //
+    // Without a predicate there is nothing to evaluate and without a sort the
+    // cursor order IS the result order, so we can walk keys and decode only the
+    // rows inside the window. mdb_cursor_get with MDB_NEXT still hands back the
+    // value pointer, but pointing into the mmap costs nothing - the expense was
+    // decode_document (JSON parse + Document construction), which is now paid
+    // `limit` times instead of `count` times.
+    //
+    // total_matched comes from mdb_stat rather than from counting, so it stays
+    // exact without touching a single value.
+    if (query.filters.empty() && !(query.sort && !query.sort->field.empty())) {
+        MDB_stat st{};
+        mdb_check(mdb_stat(rtxn.raw(), *dbi_opt, &st), "stat (scan fast path)");
+        uint64_t total = st.ms_entries;
+
+        const uint64_t start = query.offset;
+        const uint64_t end = (query.limit == 0)
+                                 ? start
+                                 : start + static_cast<uint64_t>(query.limit);
+
+        uint64_t index = 0;   // position among real documents
+        int frc = mdb_cursor_get(cursor, &k, &v, MDB_FIRST);
+        while (frc == MDB_SUCCESS) {
+            if (is_identity_key(to_sv(k))) {
+                // Counted by mdb_stat but not a document. It has a leading NUL
+                // so it always sorts first, which is why one subtraction here is
+                // enough and `total` is exact from this point on.
+                if (total > 0) --total;
+            } else {
+                if (index >= start && index < end) {
+                    result.documents.push_back(decode_document(to_sv(v)));
+                }
+                ++index;
+                // Everything needed is known: `total` came from mdb_stat and the
+                // sentinel is already accounted for, so there is no reason to
+                // walk the remaining rows.
+                if (index >= end) break;
+            }
+            frc = mdb_cursor_get(cursor, &k, &v, MDB_NEXT);
+        }
+        if (frc != MDB_NOTFOUND && frc != MDB_SUCCESS) {
+            throw_mdb(frc, "cursor_get (scan fast path)");
+        }
+
+        result.total_matched = total;
+        result.has_more = end < total;
+
+        if (!query.projection.empty()) {
+            for (auto& d : result.documents) {
+                nlohmann::json data = d.data();
+                apply_projection_inplace(data, query.projection);
+                d.set_data(data);
+            }
+        }
+        return result;
+    }
+    // -------------------------------------------------------------------
+
+    std::vector<smartbotic::database::Document> matches;
     int rc = mdb_cursor_get(cursor, &k, &v, MDB_FIRST);
     while (rc == MDB_SUCCESS) {
         // Skip the identity sentinel — it is not JSON and would throw in

+ 81 - 0
tests/test_subdb_identity.cpp

@@ -14,6 +14,8 @@
 #include <atomic>
 #include <filesystem>
 #include <iostream>
+#include <cstdio>
+#include <set>
 #include <string>
 #include <unistd.h>
 
@@ -273,6 +275,84 @@ void test_scan_limit_zero_reports_total() {
           "filtered limit=0 reports the matching total, not the collection size");
 }
 
+// v2.7.1 — the unfiltered/unsorted fast path in scan() must agree with the
+// general path exactly. It exists because the general path decoded every
+// document in the collection to return `limit` of them, so cost tracked total
+// bytes rather than page size (382ms to return one 510-byte document from a
+// 414 MB collection on a live instance). Any divergence here is a paging bug.
+void test_scan_fast_path_matches_general_path() {
+    TmpEnv t("fastpath");
+    LmdbDocumentStore store(t.env);
+    for (int i = 0; i < 25; ++i) {
+        Document d;
+        char buf[16];
+        std::snprintf(buf, sizeof(buf), "id%02d", i);
+        d.id = buf;
+        d.collection = "things";
+        d.set_data(nlohmann::json{{"n", i}, {"kind", i % 2 ? "odd" : "even"}});
+        store.put("things", d.id, d);
+    }
+
+    // total_matched and has_more must match what a full count says.
+    smartbotic::database::Query page;
+    page.limit = 10;
+    page.offset = 0;
+    auto p0 = store.scan("things", page);
+    check(p0.documents.size() == 10, "fast path returns exactly `limit` docs");
+    check(p0.total_matched == 25, "fast path total_matched excludes the sentinel");
+    check(p0.has_more, "has_more true when more remain");
+
+    page.offset = 20;
+    auto p2 = store.scan("things", page);
+    check(p2.documents.size() == 5, "final page returns the remainder");
+    check(p2.total_matched == 25, "total_matched stable across pages");
+    check(!p2.has_more, "has_more false on the last page");
+
+    page.offset = 25;
+    auto p3 = store.scan("things", page);
+    check(p3.documents.empty(), "offset past the end returns nothing");
+    check(p3.total_matched == 25, "and still reports the true total");
+
+    // Paging must cover every document exactly once, in a stable order.
+    std::set<std::string> seen;
+    for (uint32_t off = 0; off < 25; off += 7) {
+        smartbotic::database::Query q;
+        q.limit = 7;
+        q.offset = off;
+        for (const auto& d : store.scan("things", q).documents) seen.insert(d.id);
+    }
+    check(seen.size() == 25, "paging the whole collection yields every document once");
+
+    // A filter forces the general path; it must still be correct.
+    smartbotic::database::Query fq;
+    fq.limit = 100;
+    smartbotic::database::Filter f;
+    f.field = "kind";
+    f.op = smartbotic::database::FilterOp::EQ;
+    f.value = "odd";
+    fq.filters.push_back(f);
+    auto filtered = store.scan("things", fq);
+    check(filtered.total_matched == 12, "filtered path still counts matches, not rows");
+
+    // A sort also forces the general path.
+    smartbotic::database::Query sq;
+    sq.limit = 3;
+    sq.sort = smartbotic::database::Sort{"n", true};
+    auto sorted = store.scan("things", sq);
+    check(sorted.documents.size() == 3, "sorted path paginates");
+    check(sorted.total_matched == 25, "sorted path totals all rows");
+    check(sorted.documents[0].data().value("n", -1) == 24,
+          "descending sort really sorted (fast path must not swallow sorts)");
+
+    // limit=0 keeps meaning "no documents, but a true total" - the contract
+    // Count depends on (see test_scan_limit_zero_reports_total).
+    smartbotic::database::Query zq;
+    zq.limit = 0;
+    auto z = store.scan("things", zq);
+    check(z.documents.empty(), "limit=0 returns no documents on the fast path");
+    check(z.total_matched == 25, "limit=0 still reports the true total");
+}
+
 }  // namespace
 
 int main() {
@@ -284,6 +364,7 @@ int main() {
     test_sentinel_invisible_through_store();
     test_vector_subdb_sentinel();
     test_scan_limit_zero_reports_total();
+    test_scan_fast_path_matches_general_path();
 
     std::cout << "passed: " << g_pass << ", failed: " << g_fail << "\n";
     return g_fail == 0 ? 0 : 1;