Address comments for 'Apply sst file manager in myrocks clone' (#1388)

Summary:
This change is to address the comments of Apply sst file manager in myrocks clone to add slow-rm during checkpoints removal (#1386). Will squash after commit.

Pull Request resolved: #1388

Differential Revision: D51107316

fbshipit-source-id: 69903b786972f8754afac7f3d8697d162266d34f

storage/rocksdb/clone/donor.cc

@@ -56,8 +56,8 @@ namespace {
 class [[nodiscard]] rdb_checkpoint final {
  public:
  rdb_checkpoint()
- : m_dir{
- make_dir_name(m_next_id.fetch_add(1, std::memory_order_relaxed))} {}
+ : m_prefix_dir{make_dir_prefix_name(
+ m_next_id.fetch_add(1, std::memory_order_relaxed))} {}
 
  ~rdb_checkpoint() {
  // Ignore the return value - at this point the clone operation is completing
@@ -68,9 +68,8 @@ class [[nodiscard]] rdb_checkpoint final {
  // Returns MySQL error code
  [[nodiscard]] int init() {
  assert(!m_active);
- m_full_dir = make_dir_full_name(
- m_dir.c_str(), m_next_sub_id.fetch_add(1, std::memory_order_relaxed));
- const auto result = myrocks::rocksdb_create_checkpoint(m_full_dir.c_str());
+ m_dir = make_dir_name(m_prefix_dir, m_next_sub_id++);
+ const auto result = myrocks::rocksdb_create_checkpoint(m_dir.c_str());
  m_active = (result == HA_EXIT_SUCCESS);
  return m_active ? 0 : ER_INTERNAL_ERROR;
  }
@@ -79,22 +78,21 @@ class [[nodiscard]] rdb_checkpoint final {
  [[nodiscard]] int cleanup() {
  if (!m_active) return 0;
  m_active = false;
- return myrocks::rocksdb_remove_checkpoint(m_full_dir.c_str()) ==
- HA_EXIT_SUCCESS
+ return myrocks::rocksdb_remove_checkpoint(m_dir.c_str()) == HA_EXIT_SUCCESS
  ? 0
  : ER_INTERNAL_ERROR;
  }
 
  [[nodiscard]] constexpr const std::string &get_dir() const noexcept {
  assert(m_active);
- return m_full_dir;
+ return m_dir;
  }
 
  [[nodiscard]] std::string path(const std::string &file_name) const {
  // We might be calling this for inactive checkpoint too, if the donor is in
  // the middle of a checkpoint roll. The caller will handle any ENOENTs as
  // needed.
- return myrocks::rdb_concat_paths(m_full_dir, file_name);
+ return myrocks::rdb_concat_paths(m_dir, file_name);
  }
 
  rdb_checkpoint(const rdb_checkpoint &) = delete;
@@ -103,17 +101,17 @@ class [[nodiscard]] rdb_checkpoint final {
  rdb_checkpoint &operator=(rdb_checkpoint &&) = delete;
 
  private:
- const std::string m_dir;
+ const std::string m_prefix_dir;
 
- std::string m_full_dir;
+ std::string m_dir;
 
  bool m_active = false;
 
  static std::atomic<std::uint64_t> m_next_id;
 
- std::atomic<std::uint64_t> m_next_sub_id{1};
+ std::uint64_t m_next_sub_id = 1;
 
- [[nodiscard]] static std::string make_dir_name(std::uint64_t id) {
+ [[nodiscard]] static std::string make_dir_prefix_name(std::uint64_t id) {
  const auto base_str = myrocks::clone::checkpoint_base_dir();
  const auto id_str = std::to_string(id);
  std::string result;
@@ -128,14 +126,14 @@ class [[nodiscard]] rdb_checkpoint final {
  return result;
  }
 
- [[nodiscard]] static std::string make_dir_full_name(std::string base_dir,
-   std::uint64_t id) {
+ [[nodiscard]] static std::string make_dir_name(
+ const std::string &dir_name_prefix, std::uint64_t id) {
  const auto id_str = std::to_string(id);
  std::string result;
- result.reserve(base_dir.length() + id_str.length() +
+ result.reserve(dir_name_prefix.length() + id_str.length() +
  1); // +1 for '-', the trailing
  // '\0' is accounted by the sizeof.
- result = base_dir;
+ result = dir_name_prefix;
  result += '-';
  result += id_str;
  return result;

storage/rocksdb/ha_rocksdb.cc

@@ -1290,6 +1290,26 @@ static void rocksdb_set_reset_stats(
  RDB_MUTEX_UNLOCK_CHECK(rdb_sysvars_mutex);
 }
 
+int rocksdb_remove_checkpoint(const char *checkpoint_dir_raw) {
+ const auto checkpoint_dir = rdb_normalize_dir(checkpoint_dir_raw);
+ LogPluginErrMsg(INFORMATION_LEVEL, ER_LOG_PRINTF_MSG,
+ "deleting temporary checkpoint in directory : %s\n",
+ checkpoint_dir.c_str());
+
+ auto op = rocksdb::Options();
+ op.sst_file_manager.reset(NewSstFileManager(
+ rocksdb_db_options->env, rocksdb_db_options->info_log, "",
+ rocksdb_sst_mgr_rate_bytes_per_sec, false /* delete_existing_trash */));
+ const auto status = rocksdb::DestroyDB(checkpoint_dir, op);
+
+ if (status.ok()) {
+ return HA_EXIT_SUCCESS;
+ }
+ my_error(ER_GET_ERRMSG, MYF(0), status.code(), status.ToString().c_str(),
+ rocksdb_hton_name);
+ return HA_EXIT_FAILURE;
+}
+
 #ifndef __APPLE__
 
 static void rocksdb_set_io_write_timeout(
@@ -1310,27 +1330,6 @@ static void rocksdb_set_io_write_timeout(
  RDB_MUTEX_UNLOCK_CHECK(rdb_sysvars_mutex);
 }
 
-int rocksdb_remove_checkpoint(const char *checkpoint_dir_raw) {
- const auto checkpoint_dir = rdb_normalize_dir(checkpoint_dir_raw);
- LogPluginErrMsg(INFORMATION_LEVEL, ER_LOG_PRINTF_MSG,
- "deleting temporary checkpoint in directory : %s\n",
- checkpoint_dir.c_str());
-
- std::string trash_dir = std::string(rocksdb_datadir) + "/trash";
- rocksdb::Options op = rocksdb::Options();
- op.sst_file_manager.reset(NewSstFileManager(
- rocksdb_db_options->env, rocksdb_db_options->info_log, trash_dir,
- rocksdb_sst_mgr_rate_bytes_per_sec, true /* delete_existing_trash */));
- const auto status = rocksdb::DestroyDB(checkpoint_dir, op);
-
- if (status.ok()) {
- return HA_EXIT_SUCCESS;
- }
- my_error(ER_GET_ERRMSG, MYF(0), status.code(), status.ToString().c_str(),
- rocksdb_hton_name);
- return HA_EXIT_FAILURE;
-}
-
 #endif // !__APPLE__
 
 enum rocksdb_flush_log_at_trx_commit_type : unsigned int {