Skip to content

Commit 68187dc

Browse files
sunshine-Chunfacebook-github-bot
authored andcommitted
Apply sst file manager in myrocks clone to add slow-rm during checkpoints removal (#1386)
Summary: This diff makes the following two changes: 1. Clone use rocksdb::DestroyDB() to clean the checkpoint. However, we didn't pass the sst file manager so didn't make use of the slow-rm feature provided by sst file manager. This will lead up to a discard spike when rolling checkpoints. This change pass the sst file manager when cleaning checkpoints. 2. During slow-rm sst files, the checkpoint directory may not be deleted immediately. In the rolling checkpoint function call, we will first delete the checkpoint directory, then create a new one with the same name. Since we are slow removing the sst files, it's possible that when we create the new checkpoints, the old directory hasn't been deleted yet. This will cause a 'directory exist' error when creating a new checkpoint directory. In this change, we add an additional suffix to differentiate rolling checkpoints directory to avoid this error. Pull Request resolved: #1386 Test Plan: Imported from GitHub, without a `Test Plan:` line. Directory creation and deletion test is covered by `rolling_checkpoint.test`. Did additional cp_clone to verify the correctness. ``` [root@udb18397.ftw5 /var/log]# less mysqld-3301.log | grep '.clone_checkpoint-1-' 2023-11-02T11:49:43.142720-07:00 1355 [Note] [MY-011071] [Server] Plugin rocksdb reported: 'creating checkpoint in directory: /data/mysql/3301/.rocksdb/.clone_checkpoint-1-1 ' 2023-11-02T11:49:46.671314-07:00 1355 [Note] [MY-011071] [Server] Plugin rocksdb reported: 'created checkpoint in directory: /data/mysql/3301/.rocksdb/.clone_checkpoint-1-1 ' 2023-11-02T11:58:20.464175-07:00 1355 [Note] [MY-011071] [Server] Plugin rocksdb reported: 'deleting temporary checkpoint in directory : /data/mysql/3301/.rocksdb/.clone_checkpoint-1-1 ' 2023-11-02T11:58:20.753155-07:00 1355 [Note] [MY-011071] [Server] Plugin rocksdb reported: 'creating checkpoint in directory: /data/mysql/3301/.rocksdb/.clone_checkpoint-1-2 ' 2023-11-02T11:58:22.045484-07:00 1355 [Note] [MY-011071] [Server] Plugin rocksdb reported: 'created checkpoint in directory: /data/mysql/3301/.rocksdb/.clone_checkpoint-1-2 ' 2023-11-02T11:58:23.794134-07:00 1355 [Note] [MY-011071] [Server] Plugin rocksdb reported: 'deleting temporary checkpoint in directory : /data/mysql/3301/.rocksdb/.clone_checkpoint-1-2 ' ``` Differential Revision: D50938261 fbshipit-source-id: d53058f53764335b556da9c6d18d0f4b1444b212
1 parent dd13d3d commit 68187dc

File tree

2 files changed

+45
-18
lines changed

2 files changed

+45
-18
lines changed

storage/rocksdb/clone/donor.cc

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ class [[nodiscard]] rdb_checkpoint final {
6868
// Returns MySQL error code
6969
[[nodiscard]] int init() {
7070
assert(!m_active);
71-
const auto result = myrocks::rocksdb_create_checkpoint(m_dir.c_str());
71+
m_full_dir = make_dir_full_name(
72+
m_dir.c_str(), m_next_sub_id.fetch_add(1, std::memory_order_relaxed));
73+
const auto result = myrocks::rocksdb_create_checkpoint(m_full_dir.c_str());
7274
m_active = (result == HA_EXIT_SUCCESS);
7375
return m_active ? 0 : ER_INTERNAL_ERROR;
7476
}
@@ -77,21 +79,22 @@ class [[nodiscard]] rdb_checkpoint final {
7779
[[nodiscard]] int cleanup() {
7880
if (!m_active) return 0;
7981
m_active = false;
80-
return myrocks::rocksdb_remove_checkpoint(m_dir.c_str()) == HA_EXIT_SUCCESS
82+
return myrocks::rocksdb_remove_checkpoint(m_full_dir.c_str()) ==
83+
HA_EXIT_SUCCESS
8184
? 0
8285
: ER_INTERNAL_ERROR;
8386
}
8487

8588
[[nodiscard]] constexpr const std::string &get_dir() const noexcept {
8689
assert(m_active);
87-
return m_dir;
90+
return m_full_dir;
8891
}
8992

9093
[[nodiscard]] std::string path(const std::string &file_name) const {
9194
// We might be calling this for inactive checkpoint too, if the donor is in
9295
// the middle of a checkpoint roll. The caller will handle any ENOENTs as
9396
// needed.
94-
return myrocks::rdb_concat_paths(m_dir, file_name);
97+
return myrocks::rdb_concat_paths(m_full_dir, file_name);
9598
}
9699

97100
rdb_checkpoint(const rdb_checkpoint &) = delete;
@@ -102,10 +105,14 @@ class [[nodiscard]] rdb_checkpoint final {
102105
private:
103106
const std::string m_dir;
104107

108+
std::string m_full_dir;
109+
105110
bool m_active = false;
106111

107112
static std::atomic<std::uint64_t> m_next_id;
108113

114+
std::atomic<std::uint64_t> m_next_sub_id{1};
115+
109116
[[nodiscard]] static std::string make_dir_name(std::uint64_t id) {
110117
const auto base_str = myrocks::clone::checkpoint_base_dir();
111118
const auto id_str = std::to_string(id);
@@ -120,6 +127,19 @@ class [[nodiscard]] rdb_checkpoint final {
120127
result += id_str;
121128
return result;
122129
}
130+
131+
[[nodiscard]] static std::string make_dir_full_name(std::string base_dir,
132+
std::uint64_t id) {
133+
const auto id_str = std::to_string(id);
134+
std::string result;
135+
result.reserve(base_dir.length() + id_str.length() +
136+
1); // +1 for '-', the trailing
137+
// '\0' is accounted by the sizeof.
138+
result = base_dir;
139+
result += '-';
140+
result += id_str;
141+
return result;
142+
}
123143
};
124144

125145
std::atomic<std::uint64_t> rdb_checkpoint::m_next_id{1};

storage/rocksdb/ha_rocksdb.cc

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -476,20 +476,6 @@ int rocksdb_create_checkpoint(const char *checkpoint_dir_raw) {
476476
return HA_EXIT_FAILURE;
477477
}
478478

479-
int rocksdb_remove_checkpoint(const char *checkpoint_dir_raw) {
480-
const auto checkpoint_dir = rdb_normalize_dir(checkpoint_dir_raw);
481-
LogPluginErrMsg(INFORMATION_LEVEL, ER_LOG_PRINTF_MSG,
482-
"deleting temporary checkpoint in directory : %s\n",
483-
checkpoint_dir.c_str());
484-
const auto status = rocksdb::DestroyDB(checkpoint_dir, rocksdb::Options());
485-
if (status.ok()) {
486-
return HA_EXIT_SUCCESS;
487-
}
488-
my_error(ER_GET_ERRMSG, MYF(0), status.code(), status.ToString().c_str(),
489-
rocksdb_hton_name);
490-
return HA_EXIT_FAILURE;
491-
}
492-
493479
static int rocksdb_create_checkpoint_validate(
494480
THD *const thd MY_ATTRIBUTE((__unused__)),
495481
struct SYS_VAR *const var MY_ATTRIBUTE((__unused__)),
@@ -1325,6 +1311,27 @@ static void rocksdb_set_io_write_timeout(
13251311
RDB_MUTEX_UNLOCK_CHECK(rdb_sysvars_mutex);
13261312
}
13271313

1314+
int rocksdb_remove_checkpoint(const char *checkpoint_dir_raw) {
1315+
const auto checkpoint_dir = rdb_normalize_dir(checkpoint_dir_raw);
1316+
LogPluginErrMsg(INFORMATION_LEVEL, ER_LOG_PRINTF_MSG,
1317+
"deleting temporary checkpoint in directory : %s\n",
1318+
checkpoint_dir.c_str());
1319+
1320+
std::string trash_dir = std::string(rocksdb_datadir) + "/trash";
1321+
rocksdb::Options op = rocksdb::Options();
1322+
op.sst_file_manager.reset(NewSstFileManager(
1323+
rocksdb_db_options->env, rocksdb_db_options->info_log, trash_dir,
1324+
rocksdb_sst_mgr_rate_bytes_per_sec, true /* delete_existing_trash */));
1325+
const auto status = rocksdb::DestroyDB(checkpoint_dir, op);
1326+
1327+
if (status.ok()) {
1328+
return HA_EXIT_SUCCESS;
1329+
}
1330+
my_error(ER_GET_ERRMSG, MYF(0), status.code(), status.ToString().c_str(),
1331+
rocksdb_hton_name);
1332+
return HA_EXIT_FAILURE;
1333+
}
1334+
13281335
#endif // !__APPLE__
13291336

13301337
enum rocksdb_flush_log_at_trx_commit_type : unsigned int {

0 commit comments

Comments
 (0)