Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: fb-mysql-8.0.28
Choose a base branch
from

Conversation

sunshine-Chun
Copy link
Contributor

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.

…ints removal

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D50758286
@sunshine-Chun sunshine-Chun changed the base branch from fb-mysql-5.6.35 to fb-mysql-8.0.28 November 2, 2023 06:57
@facebook-github-bot
Copy link

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

facebook-github-bot pushed a commit that referenced this pull request Nov 3, 2023
…ints 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
@laurynas-biveinis
Copy link
Contributor

@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
Copy link
Contributor Author

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

@laurynas-biveinis
Copy link
Contributor

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

It's a linker error during server build

storage/rocksdb/clone/donor.cc Outdated Show resolved Hide resolved
storage/rocksdb/clone/donor.cc Outdated Show resolved Hide resolved
storage/rocksdb/clone/donor.cc Outdated Show resolved Hide resolved
storage/rocksdb/clone/donor.cc Outdated Show resolved Hide resolved
storage/rocksdb/ha_rocksdb.cc Outdated Show resolved Hide resolved
"deleting temporary checkpoint in directory : %s\n",
checkpoint_dir.c_str());

std::string trash_dir = std::string(rocksdb_datadir) + "/trash";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this is important to deduplicate with the other trash dir occurence in rocksdb_init_internal, please extract a helper to get the trash dir.

Suggested change
std::string trash_dir = std::string(rocksdb_datadir) + "/trash";
const auto trash_dir = std::string(rocksdb_datadir) + "/trash";

storage/rocksdb/ha_rocksdb.cc Outdated Show resolved Hide resolved

std::string trash_dir = std::string(rocksdb_datadir) + "/trash";
rocksdb::Options op = rocksdb::Options();
op.sst_file_manager.reset(NewSstFileManager(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an expert in SST file manager so open questions:

  1. can we save reuse the existing manager from rocksdb_init_internal as singleton?
  2. if no, is it really correct to pass delete_existing_trash = true to this one? Wouldn't this be stepping on the toes of the other SST file manager instances?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked with Rocksdb team. The trash_dir has actually been deprecated. It's the old way of managing trash files. Nowadays Rocksdb team manage them by renaming the sst files. I will pass nullptr to trash dir and set the delete_existing_trash to false since we are not using them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked with Rocksdb team. The trash_dir has actually been deprecated. It's the old way of managing trash files. Nowadays Rocksdb team manage them by renaming the sst files. I will pass nullptr to trash dir and set the delete_existing_trash to false since we are not using them.

I see, thank you. Should we convert the existing rocksdb_init_internal usage as well, in this or a follow-up PR? And can we reuse that same manager object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can convert the existing rocksdb_init_internal usage in a follow-up PR.

And can we reuse that same manager object?

I am open for suggestion. My original thought is that we only have two places use sst_file_manager. Does it worth to maintain a static variable for the usage of these two places?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am open for suggestion. My original thought is that we only have two places use sst_file_manager. Does it worth to maintain a static variable for the usage of these two places?

I am not sure neither. Clone can run many times, and rotate the checkpoint many times too, so dynamically it could be much more than two places. Also the init options are repeated, and probably should be in sync after the rocksdb_init_internal follow-up. A static variable for this would be closely related to rdb static var, which looks fine to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot one more thing, is reusing the same object even safe in the first place?

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Fix comment

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@facebook-github-bot
Copy link

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

@laurynas-biveinis
Copy link
Contributor

@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
Copy link
Contributor Author

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

facebook-github-bot pushed a commit that referenced this pull request Nov 14, 2023
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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 10, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 13, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 15, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 16, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 17, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 17, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 21, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 21, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request May 30, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 19, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 19, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 30, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Jul 31, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Aug 2, 2024
…ints removal (facebook#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: facebook#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
inikep pushed a commit to inikep/mysql-5.6 that referenced this pull request Aug 6, 2024
…ints removal (facebook#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: facebook#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants