Skip to content

Commit

Permalink
[experimental] rgw/sfs: use only one sqlite database connection
Browse files Browse the repository at this point in the history 
Currently there is one long lived connection to the sqlite database,
but also many other threads use their own connections (for some
analysis of this, see aquarist-labs#201).

This PR changes all the copies of the storage object to references,
which means we're actually only using one db connection now.  It's a
bit irritating to do this, because it's way too easy to accidentally
make a copy if you leave the '&' off :-/  I'd really want to somehow
disable copy construction of the Storage object, but I didn't figure
out how to do that yet.

One interesting effect of this change is that, prior to this commit,
the SFS status page only showed SQLite stats for the connection from
the status thread, which is not overly helpful.  With this commit
(all threads using the same connection), the figures from the SQLite
stats will actually change over time while s3gw is being used.

Note that as we're building with -DSQLITE_THREADSAFE=1, i.e. we're
using Serialized mode, it's totally cool to have one connection
shared by multiple threads (see https://www.sqlite.org/threadsafe.html)

Signed-off-by: Tim Serong <[email protected]>
  • Loading branch information
@tserong
tserong committed Sep 25, 2023
1 parent 5e4e8c4 commit 946bf17
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 80 deletions.
2 changes: 1 addition & 1 deletion src/rgw/driver/sfs/sqlite/dbconn.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,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
24 changes: 12 additions & 12 deletions src/rgw/driver/sfs/sqlite/sqlite_buckets.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ std::vector<DBOPBucketInfo> get_rgw_buckets(
std::optional<DBOPBucketInfo> SQLiteBuckets::get_bucket(
const std::string& bucket_id
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto bucket = storage.get_pointer<DBBucket>(bucket_id);
std::optional<DBOPBucketInfo> ret_value;
if (bucket) {
Expand All @@ -52,7 +52,7 @@ std::optional<DBOPBucketInfo> SQLiteBuckets::get_bucket(
std::optional<std::pair<std::string, std::string>> SQLiteBuckets::get_owner(
const std::string& bucket_id
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
const auto rows = storage.select(
columns(&DBUser::user_id, &DBUser::display_name),
inner_join<DBUser>(on(is_equal(&DBBucket::owner_id, &DBUser::user_id))),
Expand All @@ -68,60 +68,60 @@ std::optional<std::pair<std::string, std::string>> SQLiteBuckets::get_owner(
std::vector<DBOPBucketInfo> SQLiteBuckets::get_bucket_by_name(
const std::string& bucket_name
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
return get_rgw_buckets(
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& storage = conn->get_storage();
auto db_bucket = get_db_bucket(bucket);
storage.replace(db_bucket);
}

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

std::vector<std::string> SQLiteBuckets::get_bucket_ids() const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
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();
auto& storage = conn->get_storage();
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();
auto& storage = conn->get_storage();
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();
auto& storage = conn->get_storage();
return get_rgw_buckets(
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();
auto& storage = conn->get_storage();
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& storage = conn->get_storage();
auto num_ids = storage.count<DBVersionedObject>(
inner_join<DBObject>(
on(is_equal(&DBObject::uuid, &DBVersionedObject::object_id))
Expand All @@ -138,7 +138,7 @@ bool SQLiteBuckets::bucket_empty(const std::string& bucket_id) const {
std::optional<DBDeletedObjectItems> SQLiteBuckets::delete_bucket_transact(
const std::string& bucket_id, uint max_objects, bool& bucket_deleted
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
RetrySQLite<DBDeletedObjectItems> retry([&]() {
bucket_deleted = false;
DBDeletedObjectItems ret_values;
Expand Down
16 changes: 8 additions & 8 deletions src/rgw/driver/sfs/sqlite/sqlite_lifecycle.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace rgw::sal::sfs::sqlite {
SQLiteLifecycle::SQLiteLifecycle(DBConnRef _conn) : conn(_conn) {}

DBOPLCHead SQLiteLifecycle::get_head(const std::string& oid) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto head = storage.get_pointer<DBOPLCHead>(oid);
DBOPLCHead ret_value;
if (head) {
Expand All @@ -36,19 +36,19 @@ DBOPLCHead SQLiteLifecycle::get_head(const std::string& oid) const {
}

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

void SQLiteLifecycle::remove_head(const std::string& oid) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
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& storage = conn->get_storage();
auto db_entry = storage.get_pointer<DBOPLCEntry>(oid, marker);
std::optional<DBOPLCEntry> ret_value;
if (db_entry) {
Expand All @@ -60,7 +60,7 @@ std::optional<DBOPLCEntry> SQLiteLifecycle::get_entry(
std::optional<DBOPLCEntry> SQLiteLifecycle::get_next_entry(
const std::string& oid, const std::string& marker
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto db_entries = storage.get_all<DBOPLCEntry>(
where(
is_equal(&DBOPLCEntry::lc_index, oid) and
Expand All @@ -77,21 +77,21 @@ std::optional<DBOPLCEntry> SQLiteLifecycle::get_next_entry(
}

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

void SQLiteLifecycle::remove_entry(
const std::string& oid, const std::string& marker
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
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();
auto& storage = conn->get_storage();
return storage.get_all<DBOPLCEntry>(
where(
is_equal(&DBOPLCEntry::lc_index, oid) and
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 @@ -39,7 +39,7 @@ bool SQLiteList::objects(

// ListBucket does not care about versions/instances. don't populate
// key.instance
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto rows = storage.select(
columns(
&DBObject::name, &DBVersionedObject::mtime, &DBVersionedObject::etag,
Expand Down Expand Up @@ -103,7 +103,7 @@ bool SQLiteList::versions(
ceph_assert(max < std::numeric_limits<size_t>::max());
const size_t query_limit = max + 1;

auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto rows = storage.select(
columns(
&DBObject::name, &DBVersionedObject::version_id,
Expand Down
42 changes: 21 additions & 21 deletions src/rgw/driver/sfs/sqlite/sqlite_multipart.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ std::optional<std::vector<DBMultipart>> SQLiteMultipart::list_multiparts(
const std::string& marker, const std::string& delim, const int& max_uploads,
bool* is_truncated
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();

auto bucket_entries = storage.get_all<DBBucket>(
where(is_equal(&DBBucket::bucket_name, bucket_name))
Expand All @@ -56,7 +56,7 @@ std::vector<DBMultipart> SQLiteMultipart::list_multiparts_by_bucket_id(
const std::string& marker, const std::string& /*delim*/,
const int& max_uploads, bool* is_truncated, bool get_all
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();

auto start_state = get_all ? MultipartState::NONE : MultipartState::INIT;
auto end_state =
Expand Down Expand Up @@ -90,7 +90,7 @@ std::vector<DBMultipart> SQLiteMultipart::list_multiparts_by_bucket_id(

int SQLiteMultipart::abort_multiparts_by_bucket_id(const std::string& bucket_id
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
uint64_t num_changes = 0;
storage.transaction([&]() mutable {
storage.update_all(
Expand All @@ -110,7 +110,7 @@ int SQLiteMultipart::abort_multiparts_by_bucket_id(const std::string& bucket_id
}

int SQLiteMultipart::abort_multiparts(const std::string& bucket_name) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto bucket_ids_vec = storage.select(
&DBBucket::bucket_id, where(is_equal(&DBBucket::bucket_name, bucket_name))
);
Expand All @@ -129,7 +129,7 @@ std::optional<DBMultipart> SQLiteMultipart::get_multipart(
return std::nullopt;
}

auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto entries = storage.get_all<DBMultipart>(
where(is_equal(&DBMultipart::upload_id, upload_id))
);
Expand All @@ -143,7 +143,7 @@ std::optional<DBMultipart> SQLiteMultipart::get_multipart(

std::optional<DBMultipart> SQLiteMultipart::get_multipart(int id) const {
ceph_assert(id >= 0);
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto entries =
storage.get_all<DBMultipart>(where(is_equal(&DBMultipart::id, id)));
ceph_assert(entries.size() <= 1); // primary key
Expand All @@ -155,15 +155,15 @@ std::optional<DBMultipart> SQLiteMultipart::get_multipart(int id) const {
}

uint SQLiteMultipart::insert(const DBMultipart& mp) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
return storage.insert(mp);
}

std::vector<DBMultipartPart> SQLiteMultipart::list_parts(
const std::string& upload_id, int num_parts, int marker, int* next_marker,
bool* truncated
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
std::vector<DBMultipartPart> db_entries;
db_entries = storage.get_all<DBMultipartPart>(
where(
Expand Down Expand Up @@ -194,7 +194,7 @@ std::vector<DBMultipartPart> SQLiteMultipart::list_parts(
std::vector<DBMultipartPart> SQLiteMultipart::get_parts(
const std::string& upload_id
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto db_entries = storage.get_all<DBMultipartPart>(
where(is_equal(&DBMultipartPart::upload_id, upload_id)),
order_by(&DBMultipartPart::part_num)
Expand All @@ -205,7 +205,7 @@ std::vector<DBMultipartPart> SQLiteMultipart::get_parts(
std::optional<DBMultipartPart> SQLiteMultipart::get_part(
const std::string& upload_id, uint32_t part_num
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto entries = storage.get_all<DBMultipartPart>(where(
is_equal(&DBMultipartPart::upload_id, upload_id) and
is_equal(&DBMultipartPart::part_num, part_num)
Expand All @@ -220,7 +220,7 @@ std::optional<DBMultipartPart> SQLiteMultipart::get_part(
std::optional<DBMultipartPart> SQLiteMultipart::create_or_reset_part(
const std::string& upload_id, uint32_t part_num, std::string* error_str
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
std::optional<DBMultipartPart> entry = std::nullopt;

storage.transaction([&]() mutable {
Expand Down Expand Up @@ -298,7 +298,7 @@ bool SQLiteMultipart::finish_part(
const std::string& upload_id, uint32_t part_num, const std::string& etag,
uint64_t bytes_written
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
bool committed = storage.transaction([&]() mutable {
storage.update_all(
set(c(&DBMultipartPart::etag) = etag,
Expand All @@ -319,7 +319,7 @@ bool SQLiteMultipart::finish_part(
}

bool SQLiteMultipart::abort(const std::string& upload_id) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto committed = storage.transaction([&]() mutable {
storage.update_all(
set(c(&DBMultipart::state) = MultipartState::ABORTED,
Expand Down Expand Up @@ -357,7 +357,7 @@ static int _mark_complete(
}

bool SQLiteMultipart::mark_complete(const std::string& upload_id) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto committed = storage.transaction([&]() mutable {
auto num_complete = _mark_complete(storage, upload_id);
if (num_complete == 0) {
Expand All @@ -374,7 +374,7 @@ bool SQLiteMultipart::mark_complete(
const std::string& upload_id, bool* duplicate
) const {
ceph_assert(duplicate != nullptr);
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto committed = storage.transaction([&]() mutable {
auto entries = storage.get_all<DBMultipart>(
where(is_equal(&DBMultipart::upload_id, upload_id))
Expand All @@ -400,7 +400,7 @@ bool SQLiteMultipart::mark_complete(
}

bool SQLiteMultipart::mark_aggregating(const std::string& upload_id) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto committed = storage.transaction([&]() mutable {
storage.update_all(
set(c(&DBMultipart::state) = MultipartState::AGGREGATING,
Expand All @@ -422,7 +422,7 @@ bool SQLiteMultipart::mark_aggregating(const std::string& upload_id) const {
}

bool SQLiteMultipart::mark_done(const std::string& upload_id) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
auto committed = storage.transaction([&]() mutable {
storage.update_all(
set(c(&DBMultipart::state) = MultipartState::DONE,
Expand All @@ -443,7 +443,7 @@ bool SQLiteMultipart::mark_done(const std::string& upload_id) const {
}

void SQLiteMultipart::remove_parts(const std::string& upload_id) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
storage.remove_all<DBMultipartPart>(
where(c(&DBMultipartPart::upload_id) = upload_id)
);
Expand All @@ -452,7 +452,7 @@ void SQLiteMultipart::remove_parts(const std::string& upload_id) const {
void SQLiteMultipart::remove_multiparts_by_bucket_id(
const std::string& bucket_id
) const {
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
storage.remove_all<DBMultipart>(where(c(&DBMultipart::bucket_id) = bucket_id)
);
}
Expand All @@ -462,7 +462,7 @@ SQLiteMultipart::remove_multiparts_by_bucket_id_transact(
const std::string& bucket_id, uint max_items
) const {
DBDeletedMultipartItems ret_parts;
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
RetrySQLite<DBDeletedMultipartItems> retry([&]() {
auto transaction = storage.transaction_guard();
// get first the list of parts to be deleted up to max_items
Expand Down Expand Up @@ -528,7 +528,7 @@ std::optional<DBDeletedMultipartItems>
SQLiteMultipart::remove_done_or_aborted_multiparts_transact(uint max_items
) const {
DBDeletedMultipartItems ret_parts;
auto storage = conn->get_storage();
auto& storage = conn->get_storage();
RetrySQLite<DBDeletedMultipartItems> retry([&]() {
auto transaction = storage.transaction_guard();
// get first the list of parts to be deleted up to max_items
Expand Down
Loading

0 comments on commit 946bf17

Please sign in to comment.