Skip to content

Commit 09383ab

Browse files
AliMasarwehAliMasarweh
committed
RGW/standalone: review fixes
Signed-off-by: Ali Masarwa <[email protected]>
1 parent 8ab3694 commit 09383ab

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+175
-576
lines changed

‎src/rgw/driver/dbstore/config/sqlite.cc‎

Lines changed: 3 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -872,84 +872,11 @@ int SQLiteConfigStore::delete_period(const DoutPrefixProvider* dpp,
872872
return 0;
873873
}
874874

875-
int SQLiteConfigStore::read_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, std::string_view period_id,
876-
uint32_t& epoch, RGWObjVersionTracker* objv, RGWPeriod& info)
875+
int SQLiteConfigStore::update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
876+
std::string_view period_id, uint32_t epoch)
877877
{
878878
Prefix prefix{*dpp, "dbconfig:sqlite:read_latest_epoch "}; dpp = &prefix;
879-
880-
if (period_id.empty()) {
881-
ldpp_dout(dpp, 0) << "requires a period id" << dendl;
882-
return -EINVAL;
883-
}
884-
885-
try {
886-
auto conn = impl->get(dpp);
887-
period_select_epoch(dpp, *conn, period_id, epoch, info);
888-
} catch (const buffer::error& e) {
889-
ldpp_dout(dpp, 20) << "period decode failed: " << e.what() << dendl;
890-
return -EIO;
891-
} catch (const sqlite::error& e) {
892-
ldpp_dout(dpp, 20) << "period select failed: " << e.what() << dendl;
893-
if (e.code() == sqlite::errc::done) {
894-
return -ENOENT;
895-
} else if (e.code() == sqlite::errc::busy) {
896-
return -EBUSY;
897-
}
898-
return -EIO;
899-
}
900-
return 0;
901-
}
902-
903-
int SQLiteConfigStore::write_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive,
904-
std::string_view period_id, uint32_t epoch, RGWObjVersionTracker* objv,
905-
const RGWPeriod& info)
906-
{
907-
Prefix prefix{*dpp, "dbconfig:sqlite:write_latest_epoch "}; dpp = &prefix;
908-
909-
if (info.id.empty()) {
910-
ldpp_dout(dpp, 0) << "period cannot have an empty id" << dendl;
911-
return -EINVAL;
912-
}
913-
914-
bufferlist bl;
915-
encode(info, bl);
916-
const auto data = std::string_view{bl.c_str(), bl.length()};
917-
918-
try {
919-
auto conn = impl->get(dpp);
920-
sqlite::stmt_ptr* stmt = nullptr;
921-
if (exclusive) {
922-
stmt = &conn->statements["period_ins"];
923-
if (!*stmt) {
924-
const std::string sql = fmt::format(schema::period_insert4,
925-
P1, P2, P3, P4);
926-
*stmt = sqlite::prepare_statement(dpp, conn->db.get(), sql);
927-
}
928-
} else {
929-
stmt = &conn->statements["period_ups"];
930-
if (!*stmt) {
931-
const std::string sql = fmt::format(schema::period_upsert4,
932-
P1, P2, P3, P4);
933-
*stmt = sqlite::prepare_statement(dpp, conn->db.get(), sql);
934-
}
935-
}
936-
auto binding = sqlite::stmt_binding{stmt->get()};
937-
sqlite::bind_text(dpp, binding, P1, info.id);
938-
sqlite::bind_int(dpp, binding, P2, info.epoch);
939-
sqlite::bind_text(dpp, binding, P3, info.realm_id);
940-
sqlite::bind_text(dpp, binding, P4, data);
941-
942-
auto reset = sqlite::stmt_execution{stmt->get()};
943-
sqlite::eval0(dpp, reset);
944-
} catch (const sqlite::error& e) {
945-
ldpp_dout(dpp, 20) << "period insert failed: " << e.what() << dendl;
946-
if (e.code() == sqlite::errc::foreign_key_constraint) {
947-
return -EINVAL; // refers to nonexistent RealmID
948-
} else if (e.code() == sqlite::errc::busy) {
949-
return -EBUSY;
950-
}
951-
return -EIO;
952-
}
879+
// TODO: implement it later
953880
return 0;
954881
}
955882

‎src/rgw/driver/dbstore/config/sqlite.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,8 @@ class SQLiteConfigStore : public sal::ConfigStore {
7979
optional_yield y, const std::string& marker,
8080
std::span<std::string> entries,
8181
sal::ListResult<std::string>& result) override;
82-
int read_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, std::string_view period_id,
83-
uint32_t& epoch, RGWObjVersionTracker* objv, RGWPeriod& info) override;
84-
int write_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive, std::string_view period_id,
85-
uint32_t epoch, RGWObjVersionTracker* objv, const RGWPeriod& info) override;
82+
int update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
83+
std::string_view period_id, uint32_t epoch) override;
8684

8785
int write_default_zonegroup_id(const DoutPrefixProvider* dpp,
8886
optional_yield y, bool exclusive,

‎src/rgw/driver/immutable_config/store.cc‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,8 @@ int ImmutableConfigStore::list_period_ids(const DoutPrefixProvider* dpp,
137137
return 0;
138138
}
139139

140-
int ImmutableConfigStore::read_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, std::string_view period_id,
141-
uint32_t& epoch, RGWObjVersionTracker* objv, RGWPeriod& info)
142-
{
143-
return -ENOENT;
144-
}
145-
146-
int ImmutableConfigStore::write_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive,
147-
std::string_view period_id, uint32_t epoch, RGWObjVersionTracker* objv,
148-
const RGWPeriod& info)
140+
int ImmutableConfigStore::update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
141+
std::string_view period_id, uint32_t epoch)
149142
{
150143
return -EROFS;
151144
}

‎src/rgw/driver/immutable_config/store.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,8 @@ class ImmutableConfigStore : public ConfigStore {
7878
optional_yield y, const std::string& marker,
7979
std::span<std::string> entries,
8080
ListResult<std::string>& result) override;
81-
virtual int read_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, std::string_view period_id,
82-
uint32_t& epoch, RGWObjVersionTracker* objv, RGWPeriod& info) override;
83-
virtual int write_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive, std::string_view period_id,
84-
uint32_t epoch, RGWObjVersionTracker* objv, const RGWPeriod& info) override;
81+
virtual int update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
82+
std::string_view period_id, uint32_t epoch) override;
8583

8684
// ZoneGroup
8785
virtual int write_default_zonegroup_id(const DoutPrefixProvider* dpp,

‎src/rgw/driver/rados/config/period.cc‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ static std::string latest_epoch_oid(const ceph::common::ConfigProxy& conf,
4444
period_latest_epoch_info_oid));
4545
}
4646

47-
int RadosConfigStore::read_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, std::string_view period_id,
48-
uint32_t& epoch, RGWObjVersionTracker* objv, RGWPeriod& info)
47+
static int read_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
48+
ConfigImpl* impl, std::string_view period_id,
49+
uint32_t& epoch, RGWObjVersionTracker* objv)
4950
{
5051
const auto& pool = impl->period_pool;
5152
const auto latest_oid = latest_epoch_oid(dpp->get_cct()->_conf, period_id);
@@ -57,9 +58,10 @@ int RadosConfigStore::read_latest_epoch(const DoutPrefixProvider* dpp, optional_
5758
return r;
5859
}
5960

60-
int RadosConfigStore::write_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive,
61-
std::string_view period_id, uint32_t epoch, RGWObjVersionTracker* objv,
62-
const RGWPeriod& info)
61+
static int write_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
62+
ConfigImpl* impl, bool exclusive,
63+
std::string_view period_id, uint32_t epoch,
64+
RGWObjVersionTracker* objv)
6365
{
6466
const auto& pool = impl->period_pool;
6567
const auto latest_oid = latest_epoch_oid(dpp->get_cct()->_conf, period_id);
@@ -77,9 +79,8 @@ static int delete_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
7779
return impl->remove(dpp, y, pool, latest_oid, objv);
7880
}
7981

80-
static int update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
81-
RadosConfigStore& rados_config_store, std::string_view period_id,
82-
uint32_t epoch, RGWPeriod& info)
82+
int RadosConfigStore::update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
83+
std::string_view period_id, uint32_t epoch)
8384
{
8485
static constexpr int MAX_RETRIES = 20;
8586

@@ -89,7 +90,7 @@ static int update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
8990
bool exclusive = false;
9091

9192
// read existing epoch
92-
int r = rados_config_store.read_latest_epoch(dpp, y, period_id, existing_epoch, &objv, info);
93+
int r = read_latest_epoch(dpp, y, impl.get(), period_id, existing_epoch, &objv);
9394
if (r == -ENOENT) {
9495
// use an exclusive create to set the epoch atomically
9596
exclusive = true;
@@ -109,7 +110,7 @@ static int update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
109110
<< " -> " << epoch << " on period=" << period_id << dendl;
110111
}
111112

112-
r = rados_config_store.write_latest_epoch(dpp, y, exclusive, period_id, epoch, &objv, info);
113+
r = write_latest_epoch(dpp, y, impl.get(), exclusive, period_id, epoch, &objv);
113114
if (r == -EEXIST) {
114115
continue; // exclusive create raced with another update, retry
115116
} else if (r == -ECANCELED) {
@@ -148,8 +149,7 @@ int RadosConfigStore::create_period(const DoutPrefixProvider* dpp,
148149
}
149150

150151
// non const RGWPeriod
151-
RGWPeriod info_copy = info;
152-
(void) update_latest_epoch(dpp, y, *this, info.get_id(), info.get_epoch(), info_copy);
152+
(void) this->update_latest_epoch(dpp, y, info.get_id(), info.get_epoch());
153153
return 0;
154154
}
155155

@@ -162,7 +162,7 @@ int RadosConfigStore::read_period(const DoutPrefixProvider* dpp,
162162
int r = 0;
163163
if (!epoch) {
164164
epoch = 0;
165-
r = read_latest_epoch(dpp, y, period_id, *epoch, nullptr, info);
165+
r = read_latest_epoch(dpp, y, impl.get(), period_id, *epoch, nullptr);
166166
if (r < 0) {
167167
return r;
168168
}
@@ -182,8 +182,7 @@ int RadosConfigStore::delete_period(const DoutPrefixProvider* dpp,
182182
// read the latest_epoch
183183
uint32_t latest_epoch = 0;
184184
RGWObjVersionTracker latest_objv;
185-
RGWPeriod period; // not used in RadosConfigStore, but needed in the API
186-
int r = read_latest_epoch(dpp, y, period_id, latest_epoch, &latest_objv, period);
185+
int r = read_latest_epoch(dpp, y, impl.get(), period_id, latest_epoch, &latest_objv);
187186
if (r < 0 && r != -ENOENT) { // just delete epoch=0 on ENOENT
188187
ldpp_dout(dpp, 0) << "failed to read latest epoch for period "
189188
<< period_id << ": " << cpp_strerror(r) << dendl;

‎src/rgw/driver/rados/config/store.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,8 @@ class RadosConfigStore : public sal::ConfigStore {
8585
optional_yield y, const std::string& marker,
8686
std::span<std::string> entries,
8787
sal::ListResult<std::string>& result) override;
88-
virtual int read_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, std::string_view period_id,
89-
uint32_t& epoch, RGWObjVersionTracker* objv, RGWPeriod& info) override;
90-
virtual int write_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y, bool exclusive, std::string_view period_id,
91-
uint32_t epoch, RGWObjVersionTracker* objv, const RGWPeriod& info)override;
88+
virtual int update_latest_epoch(const DoutPrefixProvider* dpp, optional_yield y,
89+
std::string_view period_id, uint32_t epoch) override;
9290

9391
// ZoneGroup
9492
virtual int write_default_zonegroup_id(const DoutPrefixProvider* dpp,

‎src/rgw/driver/rados/rgw_data_sync.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3273,7 +3273,7 @@ void RGWRemoteDataLog::wakeup(int shard_id, bc::flat_set<rgw_data_notify_entry>&
32733273
data_sync_cr->wakeup(shard_id, entries);
32743274
}
32753275

3276-
int RGWRemoteDataLog::run_sync(const DoutPrefixProvider *dpp, int num_shards)
3276+
int RGWRemoteDataLog::run_sync(const DoutPrefixProvider *dpp, int num_shards, rgw::sal::ConfigStore* cfgstore)
32773277
{
32783278
// construct and start bid manager for data sync fairness
32793279
const auto& control_pool = sc.env->driver->svc()->zone->get_zone_params().control_pool;

‎src/rgw/driver/rados/rgw_data_sync.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ class RGWRemoteDataLog : public RGWCoroutinesManager {
445445
int read_recovering_shards(const DoutPrefixProvider *dpp, const int num_shards, std::set<int>& recovering_shards);
446446
int read_shard_status(const DoutPrefixProvider *dpp, int shard_id, std::set<std::string>& lagging_buckets,std::set<std::string>& recovering_buckets, rgw_data_sync_marker* sync_marker, const int max_entries);
447447
int init_sync_status(const DoutPrefixProvider *dpp, int num_shards);
448-
int run_sync(const DoutPrefixProvider *dpp, int num_shards);
448+
int run_sync(const DoutPrefixProvider *dpp, int num_shards, rgw::sal::ConfigStore* cfgstore);
449449

450450
void wakeup(int shard_id, bc::flat_set<rgw_data_notify_entry>& entries);
451451
};
@@ -512,7 +512,7 @@ class RGWDataSyncStatusManager : public DoutPrefixProvider {
512512
return source_log.read_source_log_shards_next(dpp, shard_markers, result);
513513
}
514514

515-
int run(const DoutPrefixProvider *dpp) { return source_log.run_sync(dpp, num_shards); }
515+
int run(const DoutPrefixProvider *dpp, rgw::sal::ConfigStore* cfgstore) { return source_log.run_sync(dpp, num_shards, cfgstore); }
516516

517517
void wakeup(int shard_id, bc::flat_set<rgw_data_notify_entry>& entries) { return source_log.wakeup(shard_id, entries); }
518518

0 commit comments

Comments
 (0)