SERVER-66314 improve columnar steady-state write impl

8 files changed, 209 insertions, 88 deletions

diff --git a/src/mongo/db/index/column_key_generator.cpp b/src/mongo/db/index/column_key_generator.cpp
index aae01066cec..5002236eec3 100644
--- a/src/mongo/db/index/column_key_generator.cpp
+++ b/src/mongo/db/index/column_key_generator.cpp
@@ -102,6 +102,45 @@ public:
         }
     }
 
+    static void multiVisit(
+        const std::vector<BsonRecord>& recs,
+        function_ref<void(PathView, const BsonRecord& record, const UnencodedCellView&)> cb) {
+        const size_t count = recs.size();
+        if (count == 0)
+            return;
+
+        std::vector<ColumnShredder> shredders;
+        shredders.reserve(count);
+        for (auto&& rec : recs) {
+            shredders.emplace_back(*rec.docPtr);
+        }
+
+        if (count == 1) {
+            // Don't need to merge if just 1 record.
+            shredders[0].visitCells([&](PathView path, const UnencodedCellView& cell) {  //
+                cb(path, recs[0], cell);
+            });
+            return;
+        }
+
+        // Mapping: column name -> [raw cells and indexes to recs and shredders]
+        StringDataMap<std::vector<std::pair<RawCellValue*, size_t>>> columns;
+        for (size_t i = 0; i < count; i++) {
+            for (auto&& [path, rcv] : shredders[i]._paths) {
+                auto&& vec = columns[path];
+                if (vec.empty())
+                    vec.reserve(count);  // Optimize for columns existing in all records.
+                vec.emplace_back(&rcv, i);
+            }
+        }
+
+        for (auto&& [path, rows] : columns) {
+            for (auto&& [rcv, i] : rows) {
+                cb(path, recs[i], shredders[i].makeCellView(path, *rcv));
+            }
+        }
+    }
+
     static void visitDiff(const BSONObj& oldObj,
                           const BSONObj& newObj,
                           function_ref<void(DiffAction, PathView, const UnencodedCellView*)> cb) {
@@ -544,6 +583,12 @@ void visitCellsForInsert(const BSONObj& obj,
     ColumnShredder(obj).visitCells(cb);
 }
 
+void visitCellsForInsert(
+    const std::vector<BsonRecord>& recs,
+    function_ref<void(PathView, const BsonRecord& record, const UnencodedCellView&)> cb) {
+    ColumnShredder::multiVisit(recs, cb);
+}
+
 void visitPathsForDelete(const BSONObj& obj, function_ref<void(PathView)> cb) {
     ColumnShredder(obj, ColumnShredder::kOnlyPaths).visitPaths(cb);
 }
diff --git a/src/mongo/db/index/column_key_generator.h b/src/mongo/db/index/column_key_generator.h
index e9714071672..5fa06031bd4 100644
--- a/src/mongo/db/index/column_key_generator.h
+++ b/src/mongo/db/index/column_key_generator.h
@@ -31,6 +31,7 @@
 
 #include <iosfwd>
 
+#include "mongo/db/catalog/index_catalog.h"
 #include "mongo/db/storage/column_store.h"
 #include "mongo/util/functional.h"
 
@@ -86,6 +87,21 @@ void visitCellsForInsert(const BSONObj& obj,
                          function_ref<void(PathView, const UnencodedCellView&)> cb);
 
 /**
+ * Visits all paths within obj and provides their cell values.
+ * Visit order is completely unspecified, so callers should not assume anything, but this function
+ * will attempt to perform the visits in an order optimized for inserting into a tree.
+ *
+ * Current implementation will visit all cells for a given path before moving on to the next path.
+ * Additionally, within each path, the cells will be visited in an order matching the order of their
+ * corresponding entries in the input vector. This will typically be ordered by RecordId since
+ * callers will typically pass records in that order, but this function neither relies on nor
+ * ensures that.
+ */
+void visitCellsForInsert(
+    const std::vector<BsonRecord>& recs,
+    function_ref<void(PathView, const BsonRecord& record, const UnencodedCellView&)> cb);
+
+/**
  * Visits all paths within obj. When deleting, you do not need to know about values.
  * Path visit order is unspecified.
  */
diff --git a/src/mongo/db/index/column_key_generator_test.cpp b/src/mongo/db/index/column_key_generator_test.cpp
index c852d1da9cd..3f49f8eb8bf 100644
--- a/src/mongo/db/index/column_key_generator_test.cpp
+++ b/src/mongo/db/index/column_key_generator_test.cpp
@@ -130,7 +130,7 @@ void insertTest(int line,
             << "test:" << line << " path:" << path;
     });
 
-    for (auto [path, _] : expected) {
+    for (auto&& [path, _] : expected) {
         if (seenPaths.contains(path))
             continue;
 
@@ -138,6 +138,60 @@ void insertTest(int line,
     }
 }
 
+void insertMultiTest(int line,
+                     const BSONObj& doc,
+                     const StringMap<UnencodedCellValue_ForTest>& expected) {
+    // Test with both 1 and 2 records because they use different code paths.
+    for (size_t size = 1; size <= 2; size++) {
+        BSONObj owner;
+        std::vector<BSONElement> elems;
+
+        auto recs = std::vector<BsonRecord>(size);
+        for (size_t i = 0; i < size; i++) {
+            recs[i].docPtr = &doc;
+            recs[i].id = RecordId(i);
+            recs[i].ts = Timestamp(i);
+        }
+
+        size_t counter = 0;
+        auto seenPaths = std::vector<StringSet>(size);
+        visitCellsForInsert(
+            recs, [&](PathView path, const BsonRecord& rec, const UnencodedCellView& cell) {
+                size_t i = counter++ % size;
+                ASSERT_EQ(rec.id.getLong(), int64_t(i));
+                ASSERT_EQ(rec.ts.asInt64(), int64_t(i));
+                ASSERT_EQ(&rec, &recs[i]);
+
+                seenPaths[i].insert(path.toString());
+
+                auto it = expected.find(path);
+                if (it == expected.end()) {
+                    FAIL("Unexpected path in insert") << "test:" << line << " path:" << path;
+                }
+                auto expectedCell = it->second.toView(owner, elems);
+                ASSERT_EQ(cell, expectedCell) << "test:" << line << " path:" << path;
+
+                // Round-trip the cell through encoder/parser and make sure we still agree.
+                BufBuilder encoder;
+                writeEncodedCell(cell, &encoder);
+                auto decoded = SplitCellView::parse({encoder.buf(), size_t(encoder.len())});
+                CellConverter converter;
+                ASSERT_EQ(converter.toUnencodedCellView(decoded), expectedCell)
+                    << "test:" << line << " path:" << path;
+            });
+
+        for (auto&& set : seenPaths) {
+            for (auto&& [path, _] : expected) {
+                if (set.contains(path))
+                    continue;
+
+                FAIL("Expected to see path in insert, but didn't")
+                    << "test:" << line << " path:" << path;
+            }
+        }
+    };
+}
+
 void deleteTest(int line,
                 const BSONObj& doc,
                 const StringMap<UnencodedCellValue_ForTest>& expected) {
@@ -151,7 +205,7 @@ void deleteTest(int line,
         }
     });
 
-    for (auto [path, _] : expected) {
+    for (auto&& [path, _] : expected) {
         if (seenPaths.contains(path))
             continue;
 
@@ -179,7 +233,7 @@ void updateToEmptyTest(int line,
             ASSERT(!cell) << "test:" << line << " path:" << path;
         });
 
-    for (auto [path, _] : expected) {
+    for (auto&& [path, _] : expected) {
         if (seenPaths.contains(path))
             continue;
 
@@ -214,7 +268,7 @@ void updateFromEmptyTest(int line,
             ASSERT_EQ(*cell, expectedCell) << "test:" << line << " path:" << path;
         });
 
-    for (auto [path, _] : expected) {
+    for (auto&& [path, _] : expected) {
         if (seenPaths.contains(path))
             continue;
 
@@ -244,6 +298,7 @@ void basicTests(int line, std::string json, StringMap<UnencodedCellValue_ForTest
     expected.insert({ColumnStore::kRowIdPath.toString(), {"", "", kHasSubPath}});
 
     insertTest(line, doc, expected);
+    insertMultiTest(line, doc, expected);
     deleteTest(line, doc, expected);
 
     expected.erase(ColumnStore::kRowIdPath);
@@ -724,7 +779,7 @@ void updateTest(int line,
                            }
                        });
 
-    for (auto [path, _] : expected) {
+    for (auto&& [path, _] : expected) {
         if (seenPaths.contains(path))
             continue;
 
diff --git a/src/mongo/db/index/columns_access_method.cpp b/src/mongo/db/index/columns_access_method.cpp
index 015886f4e18..87501f61cd5 100644
--- a/src/mongo/db/index/columns_access_method.cpp
+++ b/src/mongo/db/index/columns_access_method.cpp
@@ -45,6 +45,13 @@
 
 
 namespace mongo {
+namespace {
+inline void inc(int64_t* counter) {
+    if (counter)
+        ++*counter;
+};
+}  // namespace
+
 ColumnStoreAccessMethod::ColumnStoreAccessMethod(IndexCatalogEntry* ice,
                                                  std::unique_ptr<ColumnStore> store)
     : _store(std::move(store)), _indexCatalogEntry(ice), _descriptor(ice->descriptor()) {}
@@ -167,33 +174,34 @@ Status ColumnStoreAccessMethod::BulkBuilder::commit(OperationContext* opCtx,
     return Status::OK();
 }
 
-void ColumnStoreAccessMethod::insertOne(OperationContext* opCtx,
-                                        SharedBufferFragmentBuilder& pooledBufferBuilder,
-                                        StringData path,
-                                        const column_keygen::UnencodedCellView& cell,
-                                        const RecordId& rid) {
-    PooledFragmentBuilder buf(pooledBufferBuilder);
-    column_keygen::writeEncodedCell(cell, &buf);
-    _store->insert(opCtx, path, rid, CellView{buf.buf(), static_cast<size_t>(buf.len())});
-}
-
 Status ColumnStoreAccessMethod::insert(OperationContext* opCtx,
                                        SharedBufferFragmentBuilder& pooledBufferBuilder,
                                        const CollectionPtr& coll,
                                        const std::vector<BsonRecord>& bsonRecords,
                                        const InsertDeleteOptions& options,
                                        int64_t* keysInsertedOut) {
-    int64_t numInserted = 0;
-    for (auto&& bsonRecord : bsonRecords) {
+    try {
+        PooledFragmentBuilder buf(pooledBufferBuilder);
+        auto cursor = _store->newWriteCursor(opCtx);
         column_keygen::visitCellsForInsert(
-            *bsonRecord.docPtr, [&](StringData path, const column_keygen::UnencodedCellView& cell) {
-                insertOne(opCtx, pooledBufferBuilder, path, cell, bsonRecord.id);
-                ++numInserted;
+            bsonRecords,
+            [&](StringData path,
+                const BsonRecord& rec,
+                const column_keygen::UnencodedCellView& cell) {
+                if (!rec.ts.isNull()) {
+                    uassertStatusOK(opCtx->recoveryUnit()->setTimestamp(rec.ts));
+                }
+
+                buf.reset();
+                column_keygen::writeEncodedCell(cell, &buf);
+                cursor->insert(path, rec.id, CellView{buf.buf(), size_t(buf.len())});
+
+                inc(keysInsertedOut);
             });
+        return Status::OK();
+    } catch (const AssertionException& ex) {
+        return ex.toStatus();
     }
-    if (keysInsertedOut)
-        *keysInsertedOut = numInserted;
-    return Status::OK();
 }
 
 void ColumnStoreAccessMethod::remove(OperationContext* opCtx,
@@ -205,11 +213,10 @@ void ColumnStoreAccessMethod::remove(OperationContext* opCtx,
                                      const InsertDeleteOptions& options,
                                      int64_t* keysDeletedOut,
                                      CheckRecordId checkRecordId) {
+    auto cursor = _store->newWriteCursor(opCtx);
     column_keygen::visitPathsForDelete(obj, [&](StringData path) {
-        _store->remove(opCtx, path, rid);
-        if (keysDeletedOut) {
-            ++*keysDeletedOut;
-        }
+        cursor->remove(path, rid);
+        inc(keysDeletedOut);
     });
 }
 
@@ -222,31 +229,33 @@ Status ColumnStoreAccessMethod::update(OperationContext* opCtx,
                                        const InsertDeleteOptions& options,
                                        int64_t* keysInsertedOut,
                                        int64_t* keysDeletedOut) {
-    auto removeAndNote = [&](StringData path) {
-        _store->remove(opCtx, path, rid);
-        if (keysDeletedOut)
-            ++keysDeletedOut;
-    };
-    column_keygen::visitDiffForUpdate(oldDoc,
-                                      newDoc,
-                                      [&](column_keygen::DiffAction diffAction,
-                                          StringData path,
-                                          const column_keygen::UnencodedCellView* cell) {
-                                          switch (diffAction) {
-                                              case column_keygen::DiffAction::kDelete:
-                                                  return removeAndNote(path);
-                                              case column_keygen::DiffAction::kUpdate:
-                                                  removeAndNote(path);
-                                                  [[fallthrough]];
-                                              case column_keygen::DiffAction::kInsert: {
-                                                  invariant(cell);
-                                                  insertOne(
-                                                      opCtx, pooledBufferBuilder, path, *cell, rid);
-                                                  if (keysInsertedOut)
-                                                      ++*keysInsertedOut;
-                                              }
-                                          }
-                                      });
+    PooledFragmentBuilder buf(pooledBufferBuilder);
+    auto cursor = _store->newWriteCursor(opCtx);
+    column_keygen::visitDiffForUpdate(
+        oldDoc,
+        newDoc,
+        [&](column_keygen::DiffAction diffAction,
+            StringData path,
+            const column_keygen::UnencodedCellView* cell) {
+            if (diffAction == column_keygen::DiffAction::kDelete) {
+                cursor->remove(path, rid);
+                inc(keysDeletedOut);
+                return;
+            }
+
+            // kInsert and kUpdate are handled almost identically. If we switch to using
+            // `overwrite=true` cursors in WT, we could consider making them the same, although that
+            // might disadvantage other implementations of the storage engine API.
+            buf.reset();
+            column_keygen::writeEncodedCell(*cell, &buf);
+
+            const auto method = diffAction == column_keygen::DiffAction::kInsert
+                ? &ColumnStore::WriteCursor::insert
+                : &ColumnStore::WriteCursor::update;
+            (cursor.get()->*method)(path, rid, CellView{buf.buf(), size_t(buf.len())});
+
+            inc(keysInsertedOut);
+        });
     return Status::OK();
 }
 
diff --git a/src/mongo/db/index/columns_access_method.h b/src/mongo/db/index/columns_access_method.h
index 2aafb0cc8fa..f9ef8699c40 100644
--- a/src/mongo/db/index/columns_access_method.h
+++ b/src/mongo/db/index/columns_access_method.h
@@ -108,12 +108,6 @@ public:
     class BulkBuilder;
 
 private:
-    void insertOne(OperationContext*,
-                   SharedBufferFragmentBuilder&,
-                   StringData path,
-                   const column_keygen::UnencodedCellView& cell,
-                   const RecordId&);
-
     const std::unique_ptr<ColumnStore> _store;
     IndexCatalogEntry* const _indexCatalogEntry;  // owned by IndexCatalog
     const IndexDescriptor* const _descriptor;
diff --git a/src/mongo/db/storage/column_store.h b/src/mongo/db/storage/column_store.h
index 52367496611..58e3183b24d 100644
--- a/src/mongo/db/storage/column_store.h
+++ b/src/mongo/db/storage/column_store.h
@@ -63,9 +63,9 @@ public:
     class WriteCursor {
     public:
         virtual ~WriteCursor() = default;
-        virtual void insert(PathView, RecordId, CellView) = 0;
-        virtual void remove(PathView, RecordId) = 0;
-        virtual void update(PathView, RecordId, CellView) = 0;
+        virtual void insert(PathView, const RecordId&, CellView) = 0;
+        virtual void remove(PathView, const RecordId&) = 0;
+        virtual void update(PathView, const RecordId&, CellView) = 0;
     };
 
     class CursorForPath {
@@ -77,10 +77,10 @@ public:
                 return {};
             return handleResult(_cursor->next());
         }
-        boost::optional<FullCellView> seekAtOrPast(RecordId rid) {
+        boost::optional<FullCellView> seekAtOrPast(const RecordId& rid) {
             return handleResult(_cursor->seekAtOrPast(_path, rid));
         }
-        boost::optional<FullCellView> seekExact(RecordId rid) {
+        boost::optional<FullCellView> seekExact(const RecordId& rid) {
             return handleResult(_cursor->seekExact(_path, rid));
         }
 
@@ -127,7 +127,7 @@ public:
     class BulkBuilder {
     public:
         virtual ~BulkBuilder() = default;
-        virtual void addCell(PathView, RecordId, CellView) = 0;
+        virtual void addCell(PathView, const RecordId&, CellView) = 0;
     };
 
     /**
@@ -263,9 +263,9 @@ public:
     // CRUD
     //
     virtual std::unique_ptr<WriteCursor> newWriteCursor(OperationContext*) = 0;
-    virtual void insert(OperationContext*, PathView, RecordId, CellView) = 0;
-    virtual void remove(OperationContext*, PathView, RecordId) = 0;
-    virtual void update(OperationContext*, PathView, RecordId, CellView) = 0;
+    virtual void insert(OperationContext*, PathView, const RecordId&, CellView) = 0;
+    virtual void remove(OperationContext*, PathView, const RecordId&) = 0;
+    virtual void update(OperationContext*, PathView, const RecordId&, CellView) = 0;
     virtual std::unique_ptr<Cursor> newCursor(OperationContext*) const = 0;
     std::unique_ptr<CursorForPath> newCursor(OperationContext* opCtx, PathView path) const {
         return std::make_unique<CursorForPath>(path, newCursor(opCtx));
@@ -354,8 +354,8 @@ protected:
     public:
         virtual ~Cursor() = default;
         virtual boost::optional<FullCellView> next() = 0;
-        virtual boost::optional<FullCellView> seekAtOrPast(PathView, RecordId) = 0;
-        virtual boost::optional<FullCellView> seekExact(PathView, RecordId) = 0;
+        virtual boost::optional<FullCellView> seekAtOrPast(PathView, const RecordId&) = 0;
+        virtual boost::optional<FullCellView> seekExact(PathView, const RecordId&) = 0;
 
         virtual void save() = 0;
         virtual void saveUnpositioned() {
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_column_store.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_column_store.cpp
index 9cd9acb83b9..a86f715ad61 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_column_store.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_column_store.cpp
@@ -115,7 +115,9 @@ WiredTigerColumnStore::WiredTigerColumnStore(OperationContext* ctx,
       _desc(desc),
       _indexName(desc->indexName()) {}
 
-std::string& WiredTigerColumnStore::makeKey(std::string& buffer, PathView path, RecordId rid) {
+std::string& WiredTigerColumnStore::makeKey(std::string& buffer,
+                                            PathView path,
+                                            const RecordId& rid) {
     const auto ridSize =
         rid.withFormat([](RecordId::Null) -> unsigned long { return 0; },
                        [](int64_t) -> unsigned long { return sizeof(int64_t); },
@@ -143,9 +145,9 @@ public:
         _curwrap.assertInActiveTxn();
     }
 
-    void insert(PathView, RecordId, CellView) override;
-    void remove(PathView, RecordId) override;
-    void update(PathView, RecordId, CellView) override;
+    void insert(PathView, const RecordId&, CellView) override;
+    void remove(PathView, const RecordId&) override;
+    void update(PathView, const RecordId&, CellView) override;
 
     WT_CURSOR* c() {
         return _curwrap.get();
@@ -163,11 +165,11 @@ std::unique_ptr<ColumnStore::WriteCursor> WiredTigerColumnStore::newWriteCursor(
 
 void WiredTigerColumnStore::insert(OperationContext* opCtx,
                                    PathView path,
-                                   RecordId rid,
+                                   const RecordId& rid,
                                    CellView cell) {
     WriteCursor(opCtx, _uri, _tableId).insert(path, rid, cell);
 }
-void WiredTigerColumnStore::WriteCursor::insert(PathView path, RecordId rid, CellView cell) {
+void WiredTigerColumnStore::WriteCursor::insert(PathView path, const RecordId& rid, CellView cell) {
     dassert(_opCtx->lockState()->isWriteLocked());
 
     auto key = makeKey(path, rid);
@@ -187,10 +189,10 @@ void WiredTigerColumnStore::WriteCursor::insert(PathView path, RecordId rid, Cel
     }
 }
 
-void WiredTigerColumnStore::remove(OperationContext* opCtx, PathView path, RecordId rid) {
+void WiredTigerColumnStore::remove(OperationContext* opCtx, PathView path, const RecordId& rid) {
     WriteCursor(opCtx, _uri, _tableId).remove(path, rid);
 }
-void WiredTigerColumnStore::WriteCursor::remove(PathView path, RecordId rid) {
+void WiredTigerColumnStore::WriteCursor::remove(PathView path, const RecordId& rid) {
     dassert(_opCtx->lockState()->isWriteLocked());
 
     auto key = makeKey(path, rid);
@@ -207,11 +209,11 @@ void WiredTigerColumnStore::WriteCursor::remove(PathView path, RecordId rid) {
 }
 void WiredTigerColumnStore::update(OperationContext* opCtx,
                                    PathView path,
-                                   RecordId rid,
+                                   const RecordId& rid,
                                    CellView cell) {
     WriteCursor(opCtx, _uri, _tableId).update(path, rid, cell);
 }
-void WiredTigerColumnStore::WriteCursor::update(PathView path, RecordId rid, CellView cell) {
+void WiredTigerColumnStore::WriteCursor::update(PathView path, const RecordId& rid, CellView cell) {
     dassert(_opCtx->lockState()->isWriteLocked());
 
     auto key = makeKey(path, rid);
@@ -255,12 +257,12 @@ public:
 
         return curr();
     }
-    boost::optional<FullCellView> seekAtOrPast(PathView path, RecordId rid) override {
+    boost::optional<FullCellView> seekAtOrPast(PathView path, const RecordId& rid) override {
         makeKey(_buffer, path, rid);
         seekWTCursor();
         return curr();
     }
-    boost::optional<FullCellView> seekExact(PathView path, RecordId rid) override {
+    boost::optional<FullCellView> seekExact(PathView path, const RecordId& rid) override {
         makeKey(_buffer, path, rid);
         seekWTCursor(/*exactOnly*/ true);
         return curr();
@@ -399,7 +401,7 @@ public:
         _cursor->close(_cursor);
     }
 
-    void addCell(PathView path, RecordId rid, CellView cell) override {
+    void addCell(PathView path, const RecordId& rid, CellView cell) override {
         uasserted(ErrorCodes::NotImplemented, "WiredTigerColumnStore bulk builder");
     }
 
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_column_store.h b/src/mongo/db/storage/wiredtiger/wiredtiger_column_store.h
index ad1f2e2a4ca..887dfea9fbe 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_column_store.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_column_store.h
@@ -59,9 +59,9 @@ public:
     // CRUD
     //
     std::unique_ptr<ColumnStore::WriteCursor> newWriteCursor(OperationContext*) override;
-    void insert(OperationContext*, PathView, RecordId, CellView) override;
-    void remove(OperationContext*, PathView, RecordId) override;
-    void update(OperationContext*, PathView, RecordId, CellView) override;
+    void insert(OperationContext*, PathView, const RecordId&, CellView) override;
+    void remove(OperationContext*, PathView, const RecordId&) override;
+    void update(OperationContext*, PathView, const RecordId&, CellView) override;
     std::unique_ptr<ColumnStore::Cursor> newCursor(OperationContext*) const override;
 
     std::unique_ptr<ColumnStore::BulkBuilder> makeBulkBuilder(OperationContext* opCtx) override;
@@ -83,7 +83,7 @@ public:
 
     bool isEmpty(OperationContext* opCtx) override;
 
-    static std::string makeKey_ForTest(PathView path, RecordId id) {
+    static std::string makeKey_ForTest(PathView path, const RecordId& id) {
         return makeKey(path, id);
     }
 
@@ -92,8 +92,8 @@ private:
         return _uri;
     }
 
-    static std::string& makeKey(std::string& buffer, PathView, RecordId);
-    static std::string makeKey(PathView path, RecordId rid) {
+    static std::string& makeKey(std::string& buffer, PathView, const RecordId&);
+    static std::string makeKey(PathView path, const RecordId& rid) {
         std::string out;
         makeKey(out, path, rid);
         return out;