Skip to content
This repository was archived by the owner on Mar 1, 2026. It is now read-only.

Commit sst in dedicated threads#1275

Open
rockeet wants to merge 2 commits into
facebook:fb-mysql-8.0.28from
rockeet:commit-sst-thread
Open

Commit sst in dedicated threads#1275
rockeet wants to merge 2 commits into
facebook:fb-mysql-8.0.28from
rockeet:commit-sst-thread

Conversation

@rockeet

@rockeet rockeet commented Mar 1, 2023

Copy link
Copy Markdown
Contributor

Execute Rdb_sst_info::commit_sst_file in dedicated threads, this improves performance:

  1. Rdb_sst_file_ordered::commit may use stack to reverse input data, this is time consuming
  2. m_sst_file_writer->Finish may be consuming, at least it need to call fsync

Execute Rdb_sst_info::commit_sst_file in dedicated threads increase parallelization with mimimal code changes.

}

void Rdb_sst_info::commit_sst_file(Rdb_sst_file_ordered *sst_file) {
m_commiting_threads_mutex.lock();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

consider std::lock_guardstd::mutex? we use RDB_MUTEX_LOCK_CHECK macros elsewhere in this file

close_curr_sst_file();
}

for (auto& thr : m_commiting_threads) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need to grab a log here too?

@bladepan

Copy link
Copy Markdown
Contributor

Execute Rdb_sst_info::commit_sst_file in dedicated threads, this improves performance:

1. `Rdb_sst_file_ordered::commit` may use stack to reverse input data, this is time consuming

2. `m_sst_file_writer->Finish` may be consuming, at least it need to call `fsync`

Execute Rdb_sst_info::commit_sst_file in dedicated threads increase parallelization with mimimal code changes.

  1. the keys are already ordered, why it would reverse input data?
  2. the fsync happens when writing sst file to disk?

could you please provide some rough calculations about the time savings?

@rockeet

rockeet commented Apr 26, 2023

Copy link
Copy Markdown
Contributor Author

Execute Rdb_sst_info::commit_sst_file in dedicated threads, this improves performance:

1. `Rdb_sst_file_ordered::commit` may use stack to reverse input data, this is time consuming

2. `m_sst_file_writer->Finish` may be consuming, at least it need to call `fsync`

Execute Rdb_sst_info::commit_sst_file in dedicated threads increase parallelization with mimimal code changes.

  1. the keys are already ordered, why it would reverse input data?
  2. the fsync happens when writing sst file to disk?

could you please provide some rough calculations about the time savings?

  1. In Rdb_sst_file_ordered::commit(), if m_use_stack is true, it will write kv from stack.
  2. In Rdb_sst_file_ordered::Rdb_sst_file::commit(), it calls m_sst_file_writer->Finish(), in which will write file and call fsync.

In our private branch, when m_use_stack is true(reverse cf), create index time reduces 30+%.

@bladepan

Copy link
Copy Markdown
Contributor
1. In `Rdb_sst_file_ordered::commit()`, if `m_use_stack` is true, it will write kv from stack.

2. In `Rdb_sst_file_ordered::Rdb_sst_file::commit()`, it calls `m_sst_file_writer->Finish()`, in which will write file and call fsync.

In our private branch, when m_use_stack is true(reverse cf), create index time reduces 30+%.

Rdb_index_merge and Rdb_sst_file_ordered both use cf's comparator, when Rdb_index_merge write to Rdb_sst_file_ordered, the keys should be in cf's increasing order, m_use_stack should be false in this case.

@rockeet

rockeet commented Apr 28, 2023

Copy link
Copy Markdown
Contributor Author
1. In `Rdb_sst_file_ordered::commit()`, if `m_use_stack` is true, it will write kv from stack.

2. In `Rdb_sst_file_ordered::Rdb_sst_file::commit()`, it calls `m_sst_file_writer->Finish()`, in which will write file and call fsync.

In our private branch, when m_use_stack is true(reverse cf), create index time reduces 30+%.

Rdb_index_merge and Rdb_sst_file_ordered both use cf's comparator, when Rdb_index_merge write to Rdb_sst_file_ordered, the keys should be in cf's increasing order, m_use_stack should be false in this case.

We removed merge_record::m_comparator since it is identical in each std::set element, and using Slice::operator< to avoid rocksdb::Comparator virtual function call, thus it is reversed with regard to rev cf and makes m_use_stack being true.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

3 participants