rgw/sfs: Refactor types.h Object and Bucket classes

Improve Object and Bucket encapsulation to make it easy to remove the
objects cache and mutex.

Add specific named constructors (create_..) to Object that replace
construction + metadata_init calls.

Make Object create methods return pointers to allow callers to decided
how to handle ownership. Without the object cache, we no longer need
shared pointers of Object at all places.

Separate Object data / metadata deletion (delete_object_data,
delete_object_metadata)

Add get, set, update methods for Object attrs and meta. Make both
private fields.

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>

src/rgw/driver/sfs/bucket.cc

@@ -12,6 +12,7 @@
  * Foundation. See file COPYING.
  */
 #include <fstream>
+#include "driver/sfs/types.h"
 #include "rgw_sal_sfs.h"
 #include "driver/sfs/multipart.h"
 #include "driver/sfs/sqlite/sqlite_versioned_objects.h"
@@ -50,15 +51,15 @@ std::unique_ptr<Object> SFSBucket::get_object(const rgw_obj_key &key) {
 
   ldout(store->ceph_context(), 10) << "bucket::" << __func__
                                    << ": key" << key << dendl;
-  std::lock_guard l(bucket->obj_map_lock);
-  auto it = bucket->objects.find(key.name);
-  if (it == bucket->objects.end()) {
+  try {
+    auto objref = bucket->get(key.name);
+    return _get_object(objref);
+  } catch (const sfs::UnknownObjectException& _) {
     ldout(store->ceph_context(), 10) << "unable to find key " << key
       << " in bucket " << bucket->get_name() << dendl;
     // possibly a copy, return a placeholder
     return make_unique<SFSObject>(this->store, key, this, bucket);
   }
-  return _get_object(it->second);
 }
 
 /**
@@ -71,26 +72,24 @@ int SFSBucket::list(const DoutPrefixProvider *dpp, ListParams &params, int max,
   if (params.list_versions) {
     return list_versions(dpp, params, max, results, y);
   }
-
-  std::lock_guard l(bucket->obj_map_lock);
   sfs::sqlite::SQLiteVersionedObjects db_versioned_objects(store->db_conn);
   auto use_prefix = !params.prefix.empty();
-  for (const auto &[name, objref]: bucket->objects) {
-    if (use_prefix && name.rfind(params.prefix, 0) != 0) continue;
-    lsfs_dout(dpp, 10) << "object: " << name << dendl;
+  for (const auto& objref: bucket->get_all()) {
+    if (use_prefix && objref->name.rfind(params.prefix, 0) != 0) continue;
+    lsfs_dout(dpp, 10) << "object: " << objref->name << dendl;
 
     auto last_version = db_versioned_objects.get_last_versioned_object(objref->path.get_uuid());
     if (last_version->object_state == rgw::sal::ObjectState::COMMITTED) {
       // check for delimiter
-      if (check_add_common_prefix(dpp, name, params, 0, results, y)) {
+      if (check_add_common_prefix(dpp, objref->name, params, 0, results, y)) {
         continue;
       }
       auto obj = _get_object(objref);
       rgw_bucket_dir_entry dirent;
-      dirent.key = cls_rgw_obj_key(name, objref->instance);
+      dirent.key = cls_rgw_obj_key(objref->name, objref->instance);
       dirent.meta.accounted_size = obj->get_obj_size();
       dirent.meta.mtime = obj->get_mtime();
-      dirent.meta.etag = objref->meta.etag;
+      dirent.meta.etag = objref->get_meta().etag;
       dirent.meta.owner_display_name = bucket->get_owner().display_name;
       dirent.meta.owner = bucket->get_owner().user_id.id;
       results.objs.push_back(dirent);
@@ -103,13 +102,12 @@ int SFSBucket::list(const DoutPrefixProvider *dpp, ListParams &params, int max,
 
 int SFSBucket::list_versions(const DoutPrefixProvider *dpp, ListParams &params,
                       int, ListResults &results, optional_yield y) {
-  std::lock_guard l(bucket->obj_map_lock);
   auto use_prefix = !params.prefix.empty();
-  for (const auto &[name, objref]: bucket->objects) {
-    if (use_prefix && name.rfind(params.prefix, 0) != 0) continue;
-    lsfs_dout(dpp, 10) << "object: " << name << dendl;
+  for (const auto &objref: bucket->get_all()) {
+    if (use_prefix && objref->name.rfind(params.prefix, 0) != 0) continue;
+    lsfs_dout(dpp, 10) << "object: " << objref->name << dendl;
     // check for delimiter
-    if (check_add_common_prefix(dpp, name, params, 0, results, y)) {
+    if (check_add_common_prefix(dpp, objref->name, params, 0, results, y)) {
       continue;
     }
     // get all available versions from db
@@ -165,7 +163,6 @@ int SFSBucket::remove_bucket(const DoutPrefixProvider *dpp,
                                     bool delete_children,
                                     bool forward_to_master, req_info *req_info,
                                     optional_yield y) {
-  std::lock_guard l(bucket->obj_map_lock);
   if (!delete_children) {
     if (check_empty(dpp, y)) {
       return -ENOENT;

src/rgw/driver/sfs/multipart.cc

@@ -30,7 +30,7 @@ std::unique_ptr<rgw::sal::Object> SFSMultipartUpload::get_meta_obj() {
     );
   mmo->set_attrs(mp->attrs);
   mmo->set_object_ref(mp->objref);
-  mp->objref->meta.attrs = mp->attrs;
+  mp->objref->update_attrs(mp->attrs);
   return mmo;
 }
 
@@ -260,11 +260,13 @@ int SFSMultipartUpload::complete(
                      << ", offset: " << ofs
                      << ", etag: " << etag << dendl;
 
-  sfs::Object::Meta& meta = outobj->meta;
+
+  sfs::Object::Meta meta = outobj->get_meta();
   meta.size = accounted_size;
   meta.etag = etag;
   meta.mtime = ceph::real_clock::now();
-  meta.attrs = mp->attrs;
+  outobj->update_meta(meta);
+  outobj->update_attrs(mp->attrs);
 
   // remove all multipart objects. This should be done lazily in the future.
   for (const auto &[n, part] : parts) {
@@ -310,7 +312,7 @@ int SFSMultipartUpload::get_info(
   }
 
   if (attrs) {
-    *attrs = mp->objref->meta.attrs;
+    *attrs = mp->objref->get_attrs();
   }
 
   return 0;

src/rgw/driver/sfs/object.cc

@@ -11,6 +11,7 @@
  * License version 2.1, as published by the Free Software
  * Foundation. See file COPYING.
  */
+#include "driver/sfs/types.h"
 #include "rgw_sal_sfs.h"
 #include "driver/sfs/object.h"
 #include "driver/sfs/sqlite/sqlite_versioned_objects.h"
@@ -271,8 +272,9 @@ int SFSObject::copy_object(
     return -EIO;
   }
 
-  dstref->meta = objref->meta;
-  dstref->meta.mtime = ceph::real_clock::now();
+  auto dest_meta = objref->get_meta();
+  dest_meta.mtime = ceph::real_clock::now();
+  dstref->update_meta(dest_meta);
   dstref->metadata_finish(store);
 
   return 0;
@@ -318,22 +320,21 @@ int SFSObject::set_obj_attrs(const DoutPrefixProvider *dpp,
                              Attrs *delattrs,
                              optional_yield y) {
   ceph_assert(objref);
-  auto meta = objref->meta;
   map<string, bufferlist>::iterator iter;
 
   if(delattrs) {
     for(iter = delattrs->begin(); iter != delattrs->end(); ++iter) {
-      meta.attrs.erase(iter->first);
+      objref->del_attr(iter->first);
     }
   }
   if(setattrs) {
     for(iter = setattrs->begin(); iter != setattrs->end(); ++iter) {
-      meta.attrs[iter->first] = iter->second;
+      objref->set_attr(iter->first, iter->second);
     }
   }
 
   //synch attrs caches
-  state.attrset = attrs = meta.attrs;
+  state.attrset = attrs = objref->get_attrs();
   state.has_attrs = true;
 
   objref->metadata_flush_attrs(store);
@@ -351,11 +352,10 @@ int SFSObject::modify_obj_attrs(const char *attr_name,
     return 0;
   }
   ceph_assert(objref);
-  auto meta = objref->meta;
-  meta.attrs[attr_name] = attr_val;
+  objref->set_attr(attr_name, attr_val);
 
   //synch attrs caches
-  state.attrset = attrs = meta.attrs;
+  state.attrset = attrs = objref->get_attrs();
   state.has_attrs = true;
 
   objref->metadata_flush_attrs(store);
@@ -369,11 +369,10 @@ int SFSObject::delete_obj_attrs(const DoutPrefixProvider *dpp,
     return 0;
   }
   ceph_assert(objref);
-  auto meta = objref->meta;
-  if(meta.attrs.erase(attr_name)){
+  if(objref->del_attr(attr_name)){
 
     //synch attrs caches
-    state.attrset = attrs = meta.attrs;
+    state.attrset = attrs = objref->get_attrs();
     state.has_attrs = true;
 
     objref->metadata_flush_attrs(store);
@@ -484,7 +483,7 @@ void SFSObject::refresh_meta() {
     bucketref = store->get_bucket_ref(bucket->get_name());
   }
   try {
-    objref = bucketref->get(get_name());
+    objref = bucketref->get_unmutexed(get_name());
   } catch (sfs::UnknownObjectException &e) {
     // object probably not created yet?
     return;
@@ -494,23 +493,23 @@ void SFSObject::refresh_meta() {
 
 void SFSObject::_refresh_meta_from_object() {
   ceph_assert(objref);
-  auto meta = objref->meta;
+  auto meta = objref->get_meta();
   if (!get_instance().empty() && get_instance() != objref->instance) {
     // object specific version requested and it's not the last one
     sfs::sqlite::SQLiteVersionedObjects db_versioned_objects(store->db_conn);
     auto db_version = db_versioned_objects.get_versioned_object(get_instance());
     if (db_version.has_value()) {
       auto uuid = objref->path.get_uuid();
       auto deleted = db_version->object_state == ObjectState::DELETED;
-      objref = std::make_shared<sfs::Object>(get_name(), uuid, deleted);
-      objref->version_id = db_version->id;
+      objref.reset(std::move(sfs::Object::create_for_query(get_name(), uuid, deleted,
+                                                           db_version->id)));
       set_obj_size(db_version->size);
     }
   } else {
     set_obj_size(meta.size);
   }
-  attrs = meta.attrs;
-  set_attrs(meta.attrs);
+  attrs = objref->get_attrs();
+  set_attrs(attrs);
   state.mtime = meta.mtime;
 }
 

src/rgw/driver/sfs/sfs_gc.cc

@@ -13,6 +13,7 @@
  */
 #include "sfs_gc.h"
 
+#include "driver/sfs/types.h"
 #include "rgw/driver/sfs/sqlite/sqlite_objects.h"
 
 
@@ -81,8 +82,7 @@ void SFSGC::process_deleted_buckets() {
   sqlite::SQLiteBuckets db_buckets(store->db_conn);
   auto deleted_buckets = db_buckets.get_deleted_buckets_ids();
   lsfs_dout(this, 10) << "deleted buckets found = "
-                     << deleted_buckets.size() << dendl;
-  for (auto const& bucket_id : deleted_buckets) {
+                     << deleted_buckets.size() << dendl;  for (auto const& bucket_id : deleted_buckets) {
     if (max_objects <= 0) {
       break;
     }
@@ -97,21 +97,23 @@ void SFSGC::delete_objects(const std::string & bucket_id) {
     if (max_objects <= 0) {
       break;
     }
-    auto obj_instance = std::make_shared<Object>(object.name,
-                                                 object.uuid,
-                                                 true);
-    delete_object(obj_instance);
+    auto obj_instance = std::unique_ptr<Object>(
+        std::move(Object::create_for_immediate_deletion(object)));
+    delete_object(*obj_instance.get());
   }
 }
 
-void SFSGC::delete_versioned_objects(const std::shared_ptr<Object> & object) {
+void SFSGC::delete_versioned_objects(const Object& object) {
   sqlite::SQLiteVersionedObjects db_ver_objs(store->db_conn);
-  auto versions = db_ver_objs.get_versioned_objects(object->path.get_uuid());
+  auto versions = db_ver_objs.get_versioned_objects(object.path.get_uuid());
   for (auto const& version : versions) {
     if (max_objects <= 0) {
       break;
     }
-    delete_versioned_object(object, version.id);
+
+    Object to_be_deleted(object);
+    to_be_deleted.version_id = version.id;
+    delete_versioned_object(to_be_deleted);
   }
 }
 
@@ -128,26 +130,26 @@ void SFSGC::delete_bucket(const std::string & bucket_id) {
   }
 }
 
-void SFSGC::delete_object(const std::shared_ptr<Object> & object) {
+void SFSGC::delete_object(const Object& object) {
   // delete its versions first
   delete_versioned_objects(object);
   if (max_objects > 0) {
-    object->delete_object(store);
+    object.delete_object_metadata(store);
+    object.delete_object_data(store, true);
     lsfs_dout(this, 30) << "Deleted object: "
-                        << object->path.get_uuid()
+                        << object.path.get_uuid()
                         << dendl;
     --max_objects;
   }
 }
 
-void SFSGC::delete_versioned_object(const std::shared_ptr<Object> & object,
-                                    uint id) {
-  object->version_id = id;
-  object->delete_object_version(store);
+void SFSGC::delete_versioned_object(const Object& object) {
+  object.delete_object_version(store);
+  object.delete_object_data(store, false);
   lsfs_dout(this, 30) << "Deleted version: ("
-                      << object->path.get_uuid()
+                      << object.path.get_uuid()
                       << ","
-                      << id
+                      << object.version_id
                       << ")"
                       << dendl;
   --max_objects;

src/rgw/driver/sfs/sfs_gc.h

@@ -71,11 +71,11 @@ class SFSGC : public DoutPrefixProvider {
   void process_deleted_buckets();
 
   void delete_objects(const std::string & bucket_id);
-  void delete_versioned_objects(const std::shared_ptr<Object> & object);
+  void delete_versioned_objects(const Object& object);
 
   void delete_bucket(const std::string & bucket_id);
-  void delete_object(const std::shared_ptr<Object> & object);
-  void delete_versioned_object(const std::shared_ptr<Object> & object, uint id);
+  void delete_object(const Object& object);
+  void delete_versioned_object(const Object& object);
 
 };