facebook / mysql-5.6

Facebook's branch of the Oracle MySQL database. This includes MyRocks.
http://myrocks.io
Other
2.47k stars 711 forks source link

Apply sst file manager in myrocks clone to add slow-rm during checkpoints removal #1386

Open sunshine-Chun opened 8 months ago

sunshine-Chun commented 8 months ago

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.
facebook-github-bot commented 8 months ago

@sunshine-Chun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

laurynas-biveinis commented 8 months ago

@sunshine-Chun, I am getting linker errors now:

ld: Undefined symbols:
  myrocks::rocksdb_remove_checkpoint(char const*), referenced from:
      myrocks::rocksdb_close_connection(handlerton*, THD*) in librocksdb_se.a[2](ha_rocksdb.cc.o)
      myrocks::rocksdb_create_temporary_checkpoint_validate(THD*, SYS_VAR*, void*, st_mysql_value*) in librocksdb_se.a[2](ha_rocksdb.cc.o)
      (anonymous namespace)::rdb_checkpoint::cleanup() in librocksdb_se.a[25](donor.cc.o)
sunshine-Chun commented 8 months ago

@laurynas-biveinis Did you hit it during clone? Can you elaborate how you hit this error?

laurynas-biveinis commented 8 months ago

@laurynas-biveinis Did you hit it during clone? Can you elaborate how you hit this error?

It's a linker error during server build

facebook-github-bot commented 8 months ago

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

laurynas-biveinis commented 8 months ago

@sunshine-Chun , since the original PR has been merged already, I believe a new PR has to be made instead of pushing new commits to this one

sunshine-Chun commented 8 months ago

create a new pr: Address comments for 'Apply sst file manager in myrocks clone' #1388