Clarify wide column memory ownership in public APIs (#14626)

Summary:
We have seen multiple coding error with the usage of WideColumn API. Hence, improving the documentation to reduce the chance of this happening again.

Clarify that `WideColumn` and `WideColumns` are non-owning views over caller-managed memory.

Update the public `PutEntity()` API documentation to state that column name and value storage must remain valid until the call returns.

Add safe and unsafe usage examples in `wide_columns.h` to make lifetime requirements explicit.

## Testing

Not run. Documentation-only change.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/14626

Reviewed By: archang19

Differential Revision: D101280295

Pulled By: xingbowang

fbshipit-source-id: 076103321f1d54103dd1c2772aa9e84affdfd2ad

include/rocksdb/db.h

@@ -478,6 +478,8 @@ class DB {
   // Set the database entry for "key" in the column family specified by
   // "column_family" to the wide-column entity defined by "columns". If the key
   // already exists in the column family, it will be overwritten.
+  // `columns` is a non-owning view. The backing storage for each column name
+  // and value must remain valid until this method returns.
   //
   // Returns OK on success, and a non-OK status on error.
   virtual Status PutEntity(const WriteOptions& options,

include/rocksdb/sst_file_writer.h

@@ -117,7 +117,9 @@ class SstFileWriter {
   Status Put(const Slice& user_key, const Slice& timestamp, const Slice& value);
 
   // Add a PutEntity (key with the wide-column entity defined by "columns") to
-  // the currently opened file
+  // the currently opened file. `columns` is a non-owning view, so the backing
+  // storage for each column name and value must remain valid until this method
+  // returns.
   Status PutEntity(const Slice& user_key, const WideColumns& columns);
 
   // Add a Merge key with value to currently opened file

include/rocksdb/utilities/transaction.h

@@ -549,6 +549,8 @@ class Transaction {
                      const bool assume_tracked = false) = 0;
   virtual Status Put(const SliceParts& key, const SliceParts& value) = 0;
 
+  // `columns` is a non-owning view, so the backing storage for each column
+  // name and value must remain valid until this method returns.
   virtual Status PutEntity(ColumnFamilyHandle* column_family, const Slice& key,
                            const WideColumns& columns,
                            bool assume_tracked = false) = 0;

include/rocksdb/wide_columns.h

@@ -21,25 +21,47 @@ class DBImpl;
 
 // Class representing a wide column, which is defined as a pair of column name
 // and column value.
+//
+// WideColumn is a non-owning view. Both the column name and column value are
+// stored as Slices that reference caller-managed memory. The backing storage
+// must remain valid for as long as the WideColumn is used. In particular,
+// passing temporary std::string objects is unsafe. When passing WideColumn or
+// WideColumns to APIs like PutEntity(), keep the backing storage alive until
+// that method returns.
 class WideColumn {
  public:
   WideColumn() = default;
 
   // Initializes a WideColumn object by forwarding the name and value
-  // arguments to the corresponding member Slices. This makes it possible to
-  // construct a WideColumn using combinations of const char*, const
-  // std::string&, const Slice& etc., for example:
+  // arguments to the corresponding member Slices without copying the bytes.
+  // The resulting WideColumn does not own the referenced data. Construction is
+  // decoupled from the eventual PutEntity() call, but lifetime is not: any
+  // buffers referenced here must remain valid until the PutEntity() call that
+  // consumes this WideColumn or WideColumns collection returns. This makes it
+  // possible to construct a WideColumn using combinations of const char*,
+  // const std::string&, const Slice& etc., for example:
   //
-  // constexpr char foo[] = "foo";
-  // const std::string bar("bar");
-  // WideColumn column(foo, bar);
+  // std::string column_name = "attr";
+  // std::string column_value = "value";
+  // WideColumn column(column_name, column_value);
+  // WideColumns columns{column};
+  // ASSERT_OK(db->PutEntity(write_options, column_family, key, columns));
+  // // column_name and column_value must stay alive until PutEntity() returns.
+  //
+  // // Unsafe: the temporary std::string storage is destroyed before
+  // PutEntity(). WideColumns bad_columns{
+  //     {std::string("attr"), std::string("value")},
+  // };
   template <typename N, typename V>
   WideColumn(N&& name, V&& value)
       : name_(std::forward<N>(name)), value_(std::forward<V>(value)) {}
 
-  // Initializes a WideColumn object by forwarding the elements of
-  // name_tuple and value_tuple to the constructors of the corresponding member
-  // Slices. This makes it possible to initialize the Slices using the Slice
+  // Initializes a WideColumn object by forwarding the elements of name_tuple
+  // and value_tuple to the constructors of the corresponding member Slices
+  // without copying the bytes. As above, the caller retains ownership of the
+  // referenced bytes, so any buffers used here must stay alive for as long as
+  // the WideColumn is used and until any consuming PutEntity() call returns.
+  // This makes it possible to initialize the Slices using the Slice
   // constructors that take more than one argument, for example:
   //
   // constexpr char foo_name[] = "foo_name";

include/rocksdb/write_batch.h

@@ -109,7 +109,8 @@ class WriteBatch : public WriteBatchBase {
                   const Slice& value, uint64_t write_unix_time) override;
 
   // Store the mapping "key->{column1:value1, column2:value2, ...}" in the
-  // column family specified by "column_family".
+  // column family specified by "column_family". See
+  // WriteBatchBase::PutEntity() for the lifetime requirements on `columns`.
   using WriteBatchBase::PutEntity;
   Status PutEntity(ColumnFamilyHandle* column_family, const Slice& key,
                    const WideColumns& columns) override;

include/rocksdb/write_batch_base.h

@@ -59,7 +59,9 @@ class WriteBatchBase {
                           const Slice& value, uint64_t write_unix_time) = 0;
 
   // Store the mapping "key->{column1:value1, column2:value2, ...}" in the
-  // column family specified by "column_family".
+  // column family specified by "column_family". `columns` is a non-owning
+  // view, so the backing storage for each column name and value must remain
+  // valid until this method returns.
   virtual Status PutEntity(ColumnFamilyHandle* column_family, const Slice& key,
                            const WideColumns& columns) = 0;