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

[Enhancement] Early release stale rowsets after compaction. #5056

Merged

Conversation

xiaoyong-z
Copy link
Contributor

@xiaoyong-z xiaoyong-z commented Apr 13, 2022

After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets metadata.

Signed-off-by: xyz a997647204@gmail.com

What type of PR is this:

  • bug
  • feature
  • enhancement
  • others

Which issues of this PR fixes :

Fixes #4873

Problem Summary(Required) :

After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still reading the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets metadata.

@xiaoyong-z
Copy link
Contributor Author

Experimental setup

Table contains 10800 columns, and divided into 100 bucket.
Create 100 broker load tasks, each task load 1 file contains 10,000 lines of data(1.2G).

Memory Before optimization

The peak of rss is about 81.7G, the peak of tablet meta memory is 34.6G, the peak of column_reader_index_mem is 17.8G, and the peak of column_reader_meta_mem is 16.8G.

image

Memory after sharing data field between ColumnReader optimiztion #4875

Rss peaks at about 74.2G, tablet meta memory peaks at 27.3G, column_reader_index_mem peaks at 15.3G, and column_reader_meta_mem peaks at 11.9G, the memory of column_reader_meta_mem significantly lower than before.
image

Memory after early releasing stale rowsets #5056

Rss peaks at about 40G, tablet meta memory peaks at 4.5G, the memory footprint of the tablet meta data is significantly reduced

image

@xiaoyong-z xiaoyong-z mentioned this pull request Apr 13, 2022
3 tasks
be/src/storage/meta_reader.cpp Show resolved Hide resolved
@@ -245,6 +245,10 @@ Status Compaction::_merge_rowsets_vertically(size_t segment_iterator_num, Statis
TRACE_COUNTER_SCOPE_LATENCY_US("merge_rowsets_latency_us");
auto mask_buffer = std::make_unique<RowSourceMaskBuffer>(_tablet->tablet_id(), _tablet->data_dir()->path());
auto source_masks = std::make_unique<std::vector<RowSourceMask>>();
// ensure all input rowsets are loaded into memory
for (auto& rowset : _input_rowsets) {
RETURN_IF_ERROR(rowset->load());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can this operation assure that rowset won't be released?
I think other thread can release these rowsets after loading()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only stale rowsets can be released and rowsets can only transfer from not-stale to stale. Now the compaction has not begun, the input rowsets are all not-stale. So i think no other threads can release these rowsets?
If the code is written correctly, rowset should be loaded into memory here. Just in case, I've added three lines safeguards here, because the segments of rowsets will be used directly later.

@chaoyli chaoyli changed the title [optimize] Early release stale rowsets after compaction. [Enhancement] Early release stale rowsets after compaction. Apr 13, 2022
@xiaoyong-z xiaoyong-z changed the title [Enhancement] Early release stale rowsets after compaction. [Enhancement] Early release stale rowsets after compaction. (WIP) Apr 14, 2022
@xiaoyong-z xiaoyong-z changed the title [Enhancement] Early release stale rowsets after compaction. (WIP) [Enhancement] Early release stale rowsets after compaction. Apr 14, 2022
@xiaoyong-z xiaoyong-z force-pushed the Stale-Rowsets-early-release-after-commpaction branch 3 times, most recently from a38f858 to 2ba39e8 Compare April 16, 2022 16:18
be/src/storage/rowset/rowset.h Outdated Show resolved Hide resolved
@xiaoyong-z xiaoyong-z requested review from chaoyli and imay April 18, 2022 08:56
chaoyli
chaoyli previously approved these changes Apr 20, 2022
@xiaoyong-z
Copy link
Contributor Author

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Apr 20, 2022

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/6)
Auto-merging be/src/storage/compaction.cpp
Auto-merging be/src/storage/rowset/rowset.h
CONFLICT (content): Merge conflict in be/src/storage/rowset/rowset.h
Auto-merging be/src/storage/tablet.cpp
error: could not apply 88fcfff9... [optimize] Early release stale rowsets after compaction.
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 88fcfff9... [optimize] Early release stale rowsets after compaction. After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.

err-code: 5F68D

After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.
@xiaoyong-z
Copy link
Contributor Author

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Apr 20, 2022

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/1)
Auto-merging be/src/storage/tablet_reader.cpp
CONFLICT (content): Merge conflict in be/src/storage/tablet_reader.cpp
error: could not apply 4e3060ce... [optimize] Early release stale rowsets after compaction.
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 4e3060ce... [optimize] Early release stale rowsets after compaction. After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.

err-code: D7F44

chaoyli
chaoyli previously approved these changes Apr 20, 2022
meegoo
meegoo previously approved these changes Apr 20, 2022
be/src/storage/meta_reader.cpp Show resolved Hide resolved
@@ -276,6 +276,7 @@ void Tablet::modify_rowsets(const std::vector<RowsetSharedPtr>& to_add, const st

// add rs_metas_to_delete to tracker
_timestamped_version_tracker.add_stale_path_version(rs_metas_to_delete);
Rowset::close_rowsets(to_delete);
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I don't think this is a good place to close_rowsets.
I think caller should do this things.
This function just do meta modification. It don't know if the rowsets will be closed.
I think the compaction manager will decide if these rowsets should be closed not this function.

be/src/storage/tablet_reader.cpp Show resolved Hide resolved
Signed-off-by: xyz <a997647204@gmail.com>
@xiaoyong-z xiaoyong-z dismissed stale reviews from meegoo and chaoyli via 739fc69 April 20, 2022 12:27
chaoyli
chaoyli previously approved these changes Apr 20, 2022
be/src/storage/tablet_reader.cpp Show resolved Hide resolved
@@ -59,6 +59,11 @@ Status VerticalCompactionTask::_vertical_compaction_data(Statistics* statistics)
"size:$1",
max_rows_per_segment, column_groups.size());

// ensure all input rowsets are loaded into memory
for (auto& rowset : _input_rowsets) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When these _input_rowsets is acquired and when is these rowsets is released?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I move these lines in tablet reader prepare() function. I will first acquire these rowsets and then loads them to ensure there input rowsets are not freed.
Also i move the tablet reader prepare() function before the direct use of rowset's segment() function. Thus these segments are always loaded into the memory.

Signed-off-by: xyz <a997647204@gmail.com>
Signed-off-by: xyz <a997647204@gmail.com>
@chaoyli
Copy link
Contributor

chaoyli commented Apr 21, 2022

run starrocks_clang-format

Copy link
Contributor

@imay imay left a comment

Choose a reason for hiding this comment

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

LGTM

@imay imay merged commit d3b8930 into StarRocks:main Apr 21, 2022
blackstar-baba pushed a commit to blackstar-baba/starrocks that referenced this pull request Apr 28, 2022
…s#5056)

* [optimize] Early release stale rowsets after compaction.
After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.
xiaoyong-z added a commit to xiaoyong-z/starrocks that referenced this pull request May 27, 2022
…s#5056)

* [optimize] Early release stale rowsets after compaction.
After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.
xiaoyong-z added a commit to xiaoyong-z/starrocks that referenced this pull request May 27, 2022
…s#5056)

* [optimize] Early release stale rowsets after compaction.
After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.
xiaoyong-z added a commit to xiaoyong-z/starrocks that referenced this pull request May 27, 2022
…s#5056)

* [optimize] Early release stale rowsets after compaction.
After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.
chaoyli pushed a commit that referenced this pull request May 28, 2022
* [optimize] Early release stale rowsets after compaction.
After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.
chaoyli pushed a commit that referenced this pull request May 28, 2022
* [optimize] Early release stale rowsets after compaction.
After compaction, the stale rowsets will still stay in memory for a long time before gc thread free them. And these stale rowsets take up a lot of memory. In fact, there may be a few old readers still read the stale rowsets, all new readers will read the new rowset generated by the compaction, so we can free the stale rowsets if there are no readers on it. Experiments show that this optimization significantly reduces the memory footprint of rowsets meta data.
@ZiheLiu ZiheLiu mentioned this pull request Nov 22, 2022
12 tasks
jaogoy pushed a commit to jaogoy/starrocks that referenced this pull request Nov 15, 2023
(cherry picked from commit 1f27dbb)

Co-authored-by: evelyn.zhaojie <everlyn.zhaojie@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compaction memory optimization
4 participants