From fabb8c21656781d603954bafe15c2125e7d67cb1 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Sun, 22 Oct 2023 16:53:16 +0000 Subject: [PATCH] rgw/sfs: init bucket mtime and store it Signed-off-by: Joao Eduardo Luis --- src/rgw/driver/sfs/bucket.cc | 8 ++-- src/rgw/driver/sfs/bucket.h | 8 ++++ .../sfs/sqlite/buckets/bucket_conversions.cc | 2 + .../sfs/sqlite/buckets/bucket_definitions.h | 3 +- src/rgw/driver/sfs/sqlite/dbconn.h | 1 + src/rgw/driver/sfs/types.h | 9 ++-- src/rgw/rgw_sal_sfs.h | 5 ++- src/test/rgw/sfs/test_rgw_sfs_concurrency.cc | 3 +- .../sfs/test_rgw_sfs_object_state_machine.cc | 3 +- src/test/rgw/sfs/test_rgw_sfs_sfs_bucket.cc | 43 +++++++++++++++++-- .../rgw/sfs/test_rgw_sfs_wal_checkpoint.cc | 3 +- 11 files changed, 73 insertions(+), 15 deletions(-) diff --git a/src/rgw/driver/sfs/bucket.cc b/src/rgw/driver/sfs/bucket.cc index 93e906ad3db2c4..e38c6adf110822 100644 --- a/src/rgw/driver/sfs/bucket.cc +++ b/src/rgw/driver/sfs/bucket.cc @@ -47,6 +47,7 @@ namespace rgw::sal { SFSBucket::SFSBucket(SFStore* _store, sfs::BucketRef _bucket) : StoreBucket(_bucket->get_info()), store(_store), bucket(_bucket) { set_attrs(bucket->get_attrs()); + mtime = _bucket->get_mtime(); auto it = attrs.find(RGW_ATTR_ACL); if (it != attrs.end()) { @@ -342,6 +343,7 @@ int SFSBucket::remove_bucket( return -ENOENT; } db_bucket->deleted = true; + db_bucket->mtime = ceph::real_time::clock::now(); db_buckets.store_bucket(*db_bucket); store->_delete_bucket(get_name()); return 0; @@ -374,7 +376,7 @@ int SFSBucket::set_acl( attrs[RGW_ATTR_ACL] = aclp_bl; sfs::get_meta_buckets(get_store().db_conn) - ->store_bucket(sfs::sqlite::DBOPBucketInfo(get_info(), get_attrs())); + ->store_bucket(get_db_op_bucket_info()); store->_refresh_buckets_safe(); return 0; @@ -425,7 +427,7 @@ int SFSBucket::merge_and_store_attrs( } sfs::get_meta_buckets(get_store().db_conn) - ->store_bucket(sfs::sqlite::DBOPBucketInfo(get_info(), get_attrs())); + ->store_bucket(get_db_op_bucket_info()); store->_refresh_buckets_safe(); return 0; @@ -642,7 +644,7 @@ int SFSBucket::put_info( } sfs::get_meta_buckets(get_store().db_conn) - ->store_bucket(sfs::sqlite::DBOPBucketInfo(get_info(), get_attrs())); + ->store_bucket(get_db_op_bucket_info()); store->_refresh_buckets_safe(); return 0; diff --git a/src/rgw/driver/sfs/bucket.h b/src/rgw/driver/sfs/bucket.h index f970e426af4997..96ba1926abc759 100644 --- a/src/rgw/driver/sfs/bucket.h +++ b/src/rgw/driver/sfs/bucket.h @@ -23,6 +23,7 @@ #include "driver/sfs/types.h" #include "rgw_sal.h" #include "rgw_sal_store.h" +#include "sqlite/buckets/bucket_definitions.h" namespace rgw::sal { @@ -198,6 +199,13 @@ class SFSBucket : public StoreBucket { SFStore& get_store() { return *store; } const SFStore& get_store() const { return *store; } + + private: + sfs::sqlite::DBOPBucketInfo get_db_op_bucket_info() { + auto ret = sfs::sqlite::DBOPBucketInfo(get_info(), get_attrs()); + ret.mtime = ceph::real_time::clock::now(); + return ret; + } }; } // namespace rgw::sal diff --git a/src/rgw/driver/sfs/sqlite/buckets/bucket_conversions.cc b/src/rgw/driver/sfs/sqlite/buckets/bucket_conversions.cc index 670d7ae74e774c..a3f64a355568e4 100644 --- a/src/rgw/driver/sfs/sqlite/buckets/bucket_conversions.cc +++ b/src/rgw/driver/sfs/sqlite/buckets/bucket_conversions.cc @@ -39,6 +39,7 @@ DBOPBucketInfo get_rgw_bucket(const DBBucket& bucket) { assign_optional_value(bucket.object_lock, rgw_bucket.binfo.obj_lock); assign_optional_value(bucket.bucket_attrs, rgw_bucket.battrs); rgw_bucket.deleted = bucket.deleted; + rgw_bucket.mtime = bucket.mtime; return rgw_bucket; } @@ -63,6 +64,7 @@ DBBucket get_db_bucket(const DBOPBucketInfo& bucket) { assign_db_value(bucket.binfo.obj_lock, db_bucket.object_lock); assign_db_value(bucket.battrs, db_bucket.bucket_attrs); db_bucket.deleted = bucket.deleted; + db_bucket.mtime = bucket.mtime; return db_bucket; } diff --git a/src/rgw/driver/sfs/sqlite/buckets/bucket_definitions.h b/src/rgw/driver/sfs/sqlite/buckets/bucket_definitions.h index 6d1b2e3e1c8730..04743283b3621b 100644 --- a/src/rgw/driver/sfs/sqlite/buckets/bucket_definitions.h +++ b/src/rgw/driver/sfs/sqlite/buckets/bucket_definitions.h @@ -55,7 +55,7 @@ struct DBBucket { std::optional bucket_attrs; std::optional bucket_version; std::optional bucket_version_tag; - std::optional mtime; + ceph::real_time mtime; bool deleted; }; @@ -64,6 +64,7 @@ struct DBOPBucketInfo { RGWBucketInfo binfo; Attrs battrs; bool deleted{false}; + ceph::real_time mtime; DBOPBucketInfo() = default; diff --git a/src/rgw/driver/sfs/sqlite/dbconn.h b/src/rgw/driver/sfs/sqlite/dbconn.h index ed7da5f8cd945b..a868f9aef31ca1 100644 --- a/src/rgw/driver/sfs/sqlite/dbconn.h +++ b/src/rgw/driver/sfs/sqlite/dbconn.h @@ -135,6 +135,7 @@ inline auto _make_storage(const std::string& path) { sqlite_orm::make_column("deleted", &DBBucket::deleted), sqlite_orm::make_column("bucket_attrs", &DBBucket::bucket_attrs), sqlite_orm::make_column("object_lock", &DBBucket::object_lock), + sqlite_orm::make_column("mtime", &DBBucket::mtime), sqlite_orm::foreign_key(&DBBucket::owner_id) .references(&DBUser::user_id) ), diff --git a/src/rgw/driver/sfs/types.h b/src/rgw/driver/sfs/types.h index 06ad92c87ca845..9b2b8ba459851f 100644 --- a/src/rgw/driver/sfs/types.h +++ b/src/rgw/driver/sfs/types.h @@ -136,6 +136,7 @@ class Bucket { RGWBucketInfo info; rgw::sal::Attrs attrs; bool deleted{false}; + ceph::real_time mtime; public: ceph::mutex multipart_map_lock = ceph::make_mutex("multipart_map_lock"); @@ -168,13 +169,14 @@ class Bucket { Bucket( CephContext* _cct, rgw::sal::SFStore* _store, const RGWBucketInfo& _bucket_info, const RGWUserInfo& _owner, - const rgw::sal::Attrs& _attrs + const rgw::sal::Attrs& _attrs, ceph::real_time& _mtime ) : cct(_cct), store(_store), owner(_owner), info(_bucket_info), - attrs(_attrs) {} + attrs(_attrs), + mtime(_mtime) {} const RGWBucketInfo& get_info() const { return info; } @@ -202,7 +204,8 @@ class Bucket { uint32_t get_flags() const { return info.flags; } - public: + ceph::real_time get_mtime() const { return mtime; } + /// Create object version for key ObjectRef create_version(const rgw_obj_key& key) const; diff --git a/src/rgw/rgw_sal_sfs.h b/src/rgw/rgw_sal_sfs.h index c02394f961d36b..5a11ce1761d21b 100644 --- a/src/rgw/rgw_sal_sfs.h +++ b/src/rgw/rgw_sal_sfs.h @@ -424,12 +424,13 @@ class SFStore : public StoreDriver { info.requester_pays = false; info.quota = db_binfo.binfo.quota; db_binfo.battrs = attrs; + db_binfo.mtime = ceph::real_time::clock::now(); auto meta_buckets = sfs::get_meta_buckets(db_conn); meta_buckets->store_bucket(db_binfo); sfs::BucketRef b = std::make_shared( - ctx(), this, db_binfo.binfo, owner, db_binfo.battrs + ctx(), this, db_binfo.binfo, owner, db_binfo.battrs, db_binfo.mtime ); buckets[bucket.name] = b; return b; @@ -449,7 +450,7 @@ class SFStore : public StoreDriver { if (!b.deleted) { auto user = users.get_user(b.binfo.owner.id); sfs::BucketRef ref = std::make_shared( - ctx(), this, b.binfo, user->uinfo, b.battrs + ctx(), this, b.binfo, user->uinfo, b.battrs, b.mtime ); buckets[b.binfo.bucket.name] = ref; } diff --git a/src/test/rgw/sfs/test_rgw_sfs_concurrency.cc b/src/test/rgw/sfs/test_rgw_sfs_concurrency.cc index 918d3349ed2659..4360dbefe3f682 100644 --- a/src/test/rgw/sfs/test_rgw_sfs_concurrency.cc +++ b/src/test/rgw/sfs/test_rgw_sfs_concurrency.cc @@ -102,7 +102,8 @@ class TestSFSConcurrency RGWUserInfo bucket_owner; bucket = std::make_shared( - cct.get(), store.get(), db_binfo.binfo, bucket_owner, db_binfo.battrs + cct.get(), store.get(), db_binfo.binfo, bucket_owner, db_binfo.battrs, + db_binfo.mtime ); predef_object = bucket->create_version(rgw_obj_key("predef_object")); diff --git a/src/test/rgw/sfs/test_rgw_sfs_object_state_machine.cc b/src/test/rgw/sfs/test_rgw_sfs_object_state_machine.cc index 67ca56510c9e67..5b9592991368a9 100644 --- a/src/test/rgw/sfs/test_rgw_sfs_object_state_machine.cc +++ b/src/test/rgw/sfs/test_rgw_sfs_object_state_machine.cc @@ -61,7 +61,8 @@ class TestSFSObjectStateMachine : public ::testing::Test { db_buckets.store_bucket(db_binfo); bucket = std::make_shared( - cct, store.get(), db_binfo.binfo, bucket_owner, db_binfo.battrs + cct, store.get(), db_binfo.binfo, bucket_owner, db_binfo.battrs, + db_binfo.mtime ); } diff --git a/src/test/rgw/sfs/test_rgw_sfs_sfs_bucket.cc b/src/test/rgw/sfs/test_rgw_sfs_sfs_bucket.cc index e37781f2c0ab5b..a61c2f5531a7c0 100644 --- a/src/test/rgw/sfs/test_rgw_sfs_sfs_bucket.cc +++ b/src/test/rgw/sfs/test_rgw_sfs_sfs_bucket.cc @@ -75,7 +75,7 @@ class TestSFSBucket : public ::testing::Test { void createTestBucket( const std::string& bucket_id, const std::string& user_id, DBConnRef conn, - bool versioned = false + bool versioned = false, ceph::real_time* mtime = nullptr ) { SQLiteBuckets db_buckets(conn); DBOPBucketInfo bucket; @@ -83,6 +83,10 @@ class TestSFSBucket : public ::testing::Test { bucket.binfo.bucket.bucket_id = bucket_id; bucket.binfo.owner.id = user_id; bucket.deleted = false; + bucket.mtime = ceph::real_time::clock::now(); + if (mtime != nullptr) { + *mtime = bucket.mtime; + } if (versioned) { bucket.binfo.flags |= BUCKET_VERSIONED; } @@ -223,6 +227,8 @@ TEST_F(TestSFSBucket, UserCreateBucketCheckGotFromCreate) { } EXPECT_EQ(bucket_from_create->get_acl(), arg_aclp); + EXPECT_FALSE(real_clock::is_zero(bucket_from_create->get_modification_time()) + ); //@warning this triggers segfault //EXPECT_EQ(bucket_from_create->get_owner()->get_id().id, "usr_id"); @@ -307,6 +313,9 @@ TEST_F(TestSFSBucket, UserCreateBucketCheckGotFromStore) { } EXPECT_EQ(bucket_from_store->get_acl(), bucket_from_create->get_acl()); + EXPECT_TRUE( + bucket_from_store->get_modification_time().time_since_epoch().count() > 0 + ); } TEST_F(TestSFSBucket, BucketSetAcl) { @@ -364,6 +373,9 @@ TEST_F(TestSFSBucket, BucketSetAcl) { 0 ); + ceph::real_time create_mtime = bucket_from_create->get_modification_time(); + EXPECT_TRUE(create_mtime.time_since_epoch().count() > 0); + std::unique_ptr bucket_from_store; EXPECT_EQ( @@ -372,6 +384,7 @@ TEST_F(TestSFSBucket, BucketSetAcl) { ), 0 ); + EXPECT_EQ(bucket_from_store->get_modification_time(), create_mtime); RGWAccessControlPolicy arg_aclp_1 = get_aclp_1(); @@ -390,6 +403,7 @@ TEST_F(TestSFSBucket, BucketSetAcl) { ); EXPECT_EQ(bucket_from_store->get_acl(), bucket_from_store_1->get_acl()); + EXPECT_GT(bucket_from_store_1->get_modification_time(), create_mtime); } TEST_F(TestSFSBucket, BucketMergeAndStoreAttrs) { @@ -447,6 +461,8 @@ TEST_F(TestSFSBucket, BucketMergeAndStoreAttrs) { 0 ); + ceph::real_time create_mtime = bucket_from_create->get_modification_time(); + std::unique_ptr bucket_from_store; EXPECT_EQ( @@ -456,6 +472,8 @@ TEST_F(TestSFSBucket, BucketMergeAndStoreAttrs) { 0 ); + EXPECT_EQ(bucket_from_store->get_modification_time(), create_mtime); + rgw::sal::Attrs new_attrs; RGWAccessControlPolicy arg_aclp_1 = get_aclp_1(); { @@ -483,6 +501,7 @@ TEST_F(TestSFSBucket, BucketMergeAndStoreAttrs) { EXPECT_EQ(bucket_from_store_1->get_attrs(), new_attrs); EXPECT_EQ(bucket_from_store->get_acl(), bucket_from_store_1->get_acl()); + EXPECT_GT(bucket_from_store_1->get_modification_time(), create_mtime); } TEST_F(TestSFSBucket, DeleteBucket) { @@ -540,6 +559,8 @@ TEST_F(TestSFSBucket, DeleteBucket) { 0 ); + ceph::real_time create_mtime = bucket_from_create->get_modification_time(); + std::unique_ptr bucket_from_store; EXPECT_EQ( @@ -577,6 +598,10 @@ TEST_F(TestSFSBucket, DeleteBucket) { ASSERT_TRUE(b_metadata.has_value()); EXPECT_TRUE(b_metadata->deleted); + // mtime should have been updated + ceph::real_time b_mtime = b_metadata->mtime; + EXPECT_GT(b_mtime, create_mtime); + // now create the bucket again (should be ok, but bucket_id should differ) auto prev_bucket_id = bucket_from_create->get_bucket_id(); EXPECT_EQ( @@ -614,6 +639,9 @@ TEST_F(TestSFSBucket, DeleteBucket) { ASSERT_TRUE(b_metadata.has_value()); EXPECT_FALSE(b_metadata->deleted); + // mtime of new bucket must be greater than last mtime, no reuse + EXPECT_GT(b_metadata->mtime, b_mtime); + // if we query in metadata for buckets with the same name it should // return 2 entries. auto bucket_name = bucket_from_create->get_name(); @@ -636,8 +664,13 @@ TEST_F(TestSFSBucket, TestListObjectsAndVersions) { // create the test user createUser("test_user", store->db_conn); + // keep creation mtime + ceph::real_time create_mtime; + // create test bucket - createTestBucket("test_bucket", "test_user", store->db_conn, true); + createTestBucket( + "test_bucket", "test_user", store->db_conn, true, &create_mtime + ); // create a few objects in test_bucket with a few versions uint version_id = 1; @@ -677,6 +710,9 @@ TEST_F(TestSFSBucket, TestListObjectsAndVersions) { ASSERT_NE(bucket_from_store, nullptr); + // creating an object should not change the bucket's mtime + EXPECT_EQ(bucket_from_store->get_modification_time(), create_mtime); + rgw::sal::Bucket::ListParams params; // list with empty prefix @@ -1437,7 +1473,8 @@ TEST_F(TestSFSBucket, ListNamespaceMultipartsBasics) { .object_name = "object_name", .path_uuid = uuid, .meta_str = "metastr", - .mtime = now}; + .mtime = now + }; int id = multipart.insert(mpop); ASSERT_GE(id, 0); diff --git a/src/test/rgw/sfs/test_rgw_sfs_wal_checkpoint.cc b/src/test/rgw/sfs/test_rgw_sfs_wal_checkpoint.cc index f88810d569e854..02a5b00f235fbc 100644 --- a/src/test/rgw/sfs/test_rgw_sfs_wal_checkpoint.cc +++ b/src/test/rgw/sfs/test_rgw_sfs_wal_checkpoint.cc @@ -54,7 +54,8 @@ class TestSFSWALCheckpoint : public ::testing::Test { RGWUserInfo bucket_owner; bucket = std::make_shared( - cct.get(), store.get(), db_binfo.binfo, bucket_owner, db_binfo.battrs + cct.get(), store.get(), db_binfo.binfo, bucket_owner, db_binfo.battrs, + db_binfo.mtime ); }