Skip to content

Commit

Permalink
rgw/sfs: dbconn: make get_storage() return a pointer
Browse files Browse the repository at this point in the history 
This ensures we're not making multiple copies of the Storage object.
In this commit, I renamed Storage to StorageImpl, so I could do a build
and check for compiler errors to ensure I'd caught all the previous uses
of Storage throughout the codebase.

Signed-off-by: Tim Serong <[email protected]>
  • Loading branch information
@tserong
tserong committed Oct 19, 2023
1 parent bbbbfa0 commit 8ddd3b6
Show file tree
Hide file tree
Showing 20 changed files with 185 additions and 189 deletions.
20 changes: 9 additions & 11 deletions src/rgw/driver/sfs/sqlite/dbconn.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ DBConn::DBConn(CephContext* _cct)
cct(_cct),
profile_enabled(_cct->_conf.get_val<bool>("rgw_sfs_sqlite_profile")) {
sqlite3_config(SQLITE_CONFIG_LOG, &sqlite_error_callback, cct);
storage.on_open = [this](sqlite3* db) {
storage->on_open = [this](sqlite3* db) {
if (first_sqlite_conn == nullptr) {
first_sqlite_conn = db;
}
Expand Down Expand Up @@ -149,11 +149,11 @@ DBConn::DBConn(CephContext* _cct)
);
}
};
storage.open_forever();
storage.busy_timeout(5000);
storage->open_forever();
storage->busy_timeout(5000);
maybe_upgrade_metadata();
check_metadata_is_compatible();
storage.sync_schema();
storage->sync_schema();
}

void DBConn::check_metadata_is_compatible() const {
Expand Down Expand Up @@ -222,11 +222,9 @@ void DBConn::check_metadata_is_compatible() const {
}
}

static int get_version(
CephContext* cct, rgw::sal::sfs::sqlite::Storage& storage
) {
static int get_version(CephContext* cct, StorageRef storage) {
try {
return storage.pragma.user_version();
return storage->pragma.user_version();
} catch (const std::system_error& e) {
lsubdout(cct, rgw, -1) << "error opening db: " << e.code().message() << " ("
<< e.code().value() << "), " << e.what() << dendl;
Expand Down Expand Up @@ -324,7 +322,7 @@ static int upgrade_metadata_from_v2(sqlite3* db, std::string* errmsg) {
}

static void upgrade_metadata(
CephContext* cct, rgw::sal::sfs::sqlite::Storage& storage, sqlite3* db
CephContext* cct, StorageRef storage, sqlite3* db
) {
while (true) {
int cur_version = get_version(cct, storage);
Expand Down Expand Up @@ -356,7 +354,7 @@ static void upgrade_metadata(
cur_version + 1
)
<< dendl;
storage.pragma.user_version(cur_version + 1);
storage->pragma.user_version(cur_version + 1);
}
}

Expand All @@ -366,7 +364,7 @@ void DBConn::maybe_upgrade_metadata() {

if (db_version == 0) {
// must have just been created, set version!
storage.pragma.user_version(SFS_METADATA_VERSION);
storage->pragma.user_version(SFS_METADATA_VERSION);
} else if (db_version < SFS_METADATA_VERSION && db_version >= SFS_METADATA_MIN_VERSION) {
// perform schema update
upgrade_metadata(cct, storage, first_sqlite_conn);
Expand Down
7 changes: 4 additions & 3 deletions src/rgw/driver/sfs/sqlite/dbconn.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,12 @@ inline auto _make_storage(const std::string& path) {
);
}

using Storage = decltype(_make_storage(""));
using StorageImpl = decltype(_make_storage(""));
using StorageRef = StorageImpl*;

class DBConn {
private:
Storage storage;
StorageImpl storage;

public:
sqlite3* first_sqlite_conn;
Expand All @@ -268,7 +269,7 @@ class DBConn {
DBConn(const DBConn&) = delete;
DBConn& operator=(const DBConn&) = delete;

inline auto get_storage() const { return storage; }
inline auto get_storage() { return &storage; }

static std::string getDBPath(CephContext* cct) {
auto rgw_sfs_path = cct->_conf.get_val<std::string>("rgw_sfs_data_path");
Expand Down
34 changes: 17 additions & 17 deletions src/rgw/driver/sfs/sqlite/sqlite_buckets.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ std::optional<DBOPBucketInfo> SQLiteBuckets::get_bucket(
const std::string& bucket_id
) const {
auto storage = conn->get_storage();
auto bucket = storage.get_pointer<DBBucket>(bucket_id);
auto bucket = storage->get_pointer<DBBucket>(bucket_id);
std::optional<DBOPBucketInfo> ret_value;
if (bucket) {
ret_value = get_rgw_bucket(*bucket);
Expand All @@ -53,7 +53,7 @@ std::optional<std::pair<std::string, std::string>> SQLiteBuckets::get_owner(
const std::string& bucket_id
) const {
auto storage = conn->get_storage();
const auto rows = storage.select(
const auto rows = storage->select(
columns(&DBUser::user_id, &DBUser::display_name),
inner_join<DBUser>(on(is_equal(&DBBucket::owner_id, &DBUser::user_id))),
where(is_equal(&DBBucket::bucket_id, bucket_id))
Expand All @@ -70,59 +70,59 @@ std::vector<DBOPBucketInfo> SQLiteBuckets::get_bucket_by_name(
) const {
auto storage = conn->get_storage();
return get_rgw_buckets(
storage.get_all<DBBucket>(where(c(&DBBucket::bucket_name) = bucket_name))
storage->get_all<DBBucket>(where(c(&DBBucket::bucket_name) = bucket_name))
);
}

void SQLiteBuckets::store_bucket(const DBOPBucketInfo& bucket) const {
auto storage = conn->get_storage();
auto db_bucket = get_db_bucket(bucket);
storage.replace(db_bucket);
storage->replace(db_bucket);
}

void SQLiteBuckets::remove_bucket(const std::string& bucket_name) const {
auto storage = conn->get_storage();
storage.remove<DBBucket>(bucket_name);
storage->remove<DBBucket>(bucket_name);
}

std::vector<std::string> SQLiteBuckets::get_bucket_ids() const {
auto storage = conn->get_storage();
return storage.select(&DBBucket::bucket_name);
return storage->select(&DBBucket::bucket_name);
}

std::vector<std::string> SQLiteBuckets::get_bucket_ids(
const std::string& user_id
) const {
auto storage = conn->get_storage();
return storage.select(
return storage->select(
&DBBucket::bucket_name, where(c(&DBBucket::owner_id) = user_id)
);
}

std::vector<DBOPBucketInfo> SQLiteBuckets::get_buckets() const {
auto storage = conn->get_storage();
return get_rgw_buckets(storage.get_all<DBBucket>());
return get_rgw_buckets(storage->get_all<DBBucket>());
}

std::vector<DBOPBucketInfo> SQLiteBuckets::get_buckets(
const std::string& user_id
) const {
auto storage = conn->get_storage();
return get_rgw_buckets(
storage.get_all<DBBucket>(where(c(&DBBucket::owner_id) = user_id))
storage->get_all<DBBucket>(where(c(&DBBucket::owner_id) = user_id))
);
}

std::vector<std::string> SQLiteBuckets::get_deleted_buckets_ids() const {
auto storage = conn->get_storage();
return storage.select(
return storage->select(
&DBBucket::bucket_id, where(c(&DBBucket::deleted) = true)
);
}

bool SQLiteBuckets::bucket_empty(const std::string& bucket_id) const {
auto storage = conn->get_storage();
auto num_ids = storage.count<DBVersionedObject>(
auto num_ids = storage->count<DBVersionedObject>(
inner_join<DBObject>(
on(is_equal(&DBObject::uuid, &DBVersionedObject::object_id))
),
Expand All @@ -142,9 +142,9 @@ std::optional<DBDeletedObjectItems> SQLiteBuckets::delete_bucket_transact(
RetrySQLiteBusy<DBDeletedObjectItems> retry([&]() {
bucket_deleted = false;
DBDeletedObjectItems ret_values;
auto transaction = storage.transaction_guard();
auto transaction = storage->transaction_guard();
// first get all the objects and versions for that bucket
ret_values = storage.select(
ret_values = storage->select(
columns(&DBObject::uuid, &DBVersionedObject::id),
inner_join<DBObject>(
on(is_equal(&DBObject::uuid, &DBVersionedObject::object_id))
Expand All @@ -154,14 +154,14 @@ std::optional<DBDeletedObjectItems> SQLiteBuckets::delete_bucket_transact(
);
for (auto const& uuid_version : ret_values) {
// remove the versions first
storage.remove<DBVersionedObject>(std::get<1>(uuid_version));
storage->remove<DBVersionedObject>(std::get<1>(uuid_version));
// try to delete the object (it will throw an exception if it's not
// empty yet)
// possible errors when object is not empty are:
// SQLITE_CONSTRAINT: legacy sqlite error
// SQLITE_CONSTRAINT_FOREIGNKEY: extended sqlite error
try {
storage.remove<DBObject>(std::get<0>(uuid_version));
storage->remove<DBObject>(std::get<0>(uuid_version));
} catch (const std::system_error& e) {
if (e.code().value() != SQLITE_CONSTRAINT_FOREIGNKEY &&
e.code().value() != SQLITE_CONSTRAINT) {
Expand All @@ -171,7 +171,7 @@ std::optional<DBDeletedObjectItems> SQLiteBuckets::delete_bucket_transact(
}
// try to delete the bucket
try {
storage.remove<DBBucket>(bucket_id);
storage->remove<DBBucket>(bucket_id);
bucket_deleted = true;
} catch (const std::system_error& e) {
if (e.code().value() != SQLITE_CONSTRAINT_FOREIGNKEY &&
Expand All @@ -191,7 +191,7 @@ const std::optional<SQLiteBuckets::Stats> SQLiteBuckets::get_stats(
auto storage = conn->get_storage();
std::optional<SQLiteBuckets::Stats> stats;

auto res = storage.select(
auto res = storage->select(
columns(
count(&DBVersionedObject::object_id), sum(&DBVersionedObject::size)
),
Expand Down
18 changes: 9 additions & 9 deletions src/rgw/driver/sfs/sqlite/sqlite_lifecycle.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ SQLiteLifecycle::SQLiteLifecycle(DBConnRef _conn) : conn(_conn) {}

DBOPLCHead SQLiteLifecycle::get_head(const std::string& oid) const {
auto storage = conn->get_storage();
auto head = storage.get_pointer<DBOPLCHead>(oid);
auto head = storage->get_pointer<DBOPLCHead>(oid);
DBOPLCHead ret_value;
if (head) {
ret_value = *head;
Expand All @@ -29,27 +29,27 @@ DBOPLCHead SQLiteLifecycle::get_head(const std::string& oid) const {
// LC was not executed yet.
// create an empty entry
DBOPLCHead new_head{oid, "", 0};
storage.replace(new_head);
storage->replace(new_head);
ret_value = new_head;
}
return ret_value;
}

void SQLiteLifecycle::store_head(const DBOPLCHead& head) const {
auto storage = conn->get_storage();
storage.replace(head);
storage->replace(head);
}

void SQLiteLifecycle::remove_head(const std::string& oid) const {
auto storage = conn->get_storage();
storage.remove<DBOPLCHead>(oid);
storage->remove<DBOPLCHead>(oid);
}

std::optional<DBOPLCEntry> SQLiteLifecycle::get_entry(
const std::string& oid, const std::string& marker
) const {
auto storage = conn->get_storage();
auto db_entry = storage.get_pointer<DBOPLCEntry>(oid, marker);
auto db_entry = storage->get_pointer<DBOPLCEntry>(oid, marker);
std::optional<DBOPLCEntry> ret_value;
if (db_entry) {
ret_value = *db_entry;
Expand All @@ -61,7 +61,7 @@ std::optional<DBOPLCEntry> SQLiteLifecycle::get_next_entry(
const std::string& oid, const std::string& marker
) const {
auto storage = conn->get_storage();
auto db_entries = storage.get_all<DBOPLCEntry>(
auto db_entries = storage->get_all<DBOPLCEntry>(
where(
is_equal(&DBOPLCEntry::lc_index, oid) and
greater_than(&DBOPLCEntry::bucket_name, marker)
Expand All @@ -78,21 +78,21 @@ std::optional<DBOPLCEntry> SQLiteLifecycle::get_next_entry(

void SQLiteLifecycle::store_entry(const DBOPLCEntry& entry) const {
auto storage = conn->get_storage();
storage.replace(entry);
storage->replace(entry);
}

void SQLiteLifecycle::remove_entry(
const std::string& oid, const std::string& marker
) const {
auto storage = conn->get_storage();
storage.remove<DBOPLCEntry>(oid, marker);
storage->remove<DBOPLCEntry>(oid, marker);
}

std::vector<DBOPLCEntry> SQLiteLifecycle::list_entries(
const std::string& oid, const std::string& marker, uint32_t max_entries
) const {
auto storage = conn->get_storage();
return storage.get_all<DBOPLCEntry>(
return storage->get_all<DBOPLCEntry>(
where(
is_equal(&DBOPLCEntry::lc_index, oid) and
greater_than(&DBOPLCEntry::bucket_name, marker)
Expand Down
4 changes: 2 additions & 2 deletions src/rgw/driver/sfs/sqlite/sqlite_list.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ bool SQLiteList::objects(
// ListBucket does not care about versions/instances. don't populate
// key.instance
auto storage = conn->get_storage();
auto rows = storage.select(
auto rows = storage->select(
columns(
&DBObject::name, &DBVersionedObject::mtime, &DBVersionedObject::etag,
sum(&DBVersionedObject::size)
Expand Down Expand Up @@ -104,7 +104,7 @@ bool SQLiteList::versions(
const size_t query_limit = max + 1;

auto storage = conn->get_storage();
auto rows = storage.select(
auto rows = storage->select(
columns(
&DBObject::name, &DBVersionedObject::version_id,
&DBVersionedObject::mtime, &DBVersionedObject::etag,
Expand Down
Loading

0 comments on commit 8ddd3b6

Please sign in to comment.