You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "zzzxl1993 (via GitHub)" <gi...@apache.org> on 2024/02/27 12:17:33 UTC
[PR] [opt](invert index) modify of time series compaction policy [doris]
zzzxl1993 opened a new pull request, #31488:
URL: https://github.com/apache/doris/pull/31488
## Proposed changes
Issue Number: close #xxx
<!--Describe your changes.-->
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #31488:
URL: https://github.com/apache/doris/pull/31488#discussion_r1508620667
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -288,7 +342,41 @@ void TimeSeriesCumulativeCompactionPolicy::update_cumulative_point(
return;
}
+ if (tablet->tablet_meta()->time_series_compaction_level_threshold() == 2 &&
+ output_rowset->rowset_meta()->compaction_level() < 2) {
+ return;
+ }
+
tablet->set_cumulative_layer_point(output_rowset->end_version() + 1);
}
+void TimeSeriesCumulativeCompactionPolicy::update_compaction_level(
Review Comment:
warning: method 'update_compaction_level' can be made static [readability-convert-member-functions-to-static]
```suggestion
static void TimeSeriesCumulativeCompactionPolicy::update_compaction_level(
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "zzzxl1993 (via GitHub)" <gi...@apache.org>.
zzzxl1993 commented on PR #31488:
URL: https://github.com/apache/doris/pull/31488#issuecomment-1980157720
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #31488:
URL: https://github.com/apache/doris/pull/31488#issuecomment-1982265453
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #31488:
URL: https://github.com/apache/doris/pull/31488#issuecomment-1980165453
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #31488:
URL: https://github.com/apache/doris/pull/31488#issuecomment-1980044295
TeamCity be ut coverage result:
Function Coverage: 35.40% (8559/24179)
Line Coverage: 27.16% (69492/255879)
Region Coverage: 26.37% (36044/136668)
Branch Coverage: 23.21% (18422/79366)
Coverage Report: http://coverage.selectdb-in.cc/coverage/9c776c240551a4623c37d210b21cda8026e2a019_9c776c240551a4623c37d210b21cda8026e2a019/report/index.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "xiaokang (via GitHub)" <gi...@apache.org>.
xiaokang merged PR #31488:
URL: https://github.com/apache/doris/pull/31488
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "zzzxl1993 (via GitHub)" <gi...@apache.org>.
zzzxl1993 commented on PR #31488:
URL: https://github.com/apache/doris/pull/31488#issuecomment-1979961072
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #31488:
URL: https://github.com/apache/doris/pull/31488#issuecomment-1980253636
TeamCity be ut coverage result:
Function Coverage: 35.40% (8561/24181)
Line Coverage: 27.16% (69502/255925)
Region Coverage: 26.37% (36049/136703)
Branch Coverage: 23.20% (18422/79390)
Coverage Report: http://coverage.selectdb-in.cc/coverage/1da987e54e390c7775830186fff51079aa02e88b_1da987e54e390c7775830186fff51079aa02e88b/report/index.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "xiaokang (via GitHub)" <gi...@apache.org>.
xiaokang commented on code in PR #31488:
URL: https://github.com/apache/doris/pull/31488#discussion_r1512076574
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -70,15 +80,32 @@ uint32_t TimeSeriesCumulativeCompactionPolicy::calc_cumulative_compaction_score(
return score;
}
- // Condition 1: the size of input files for compaction meets the requirement of parameter compaction_goal_size
int64_t compaction_goal_size_mbytes =
tablet->tablet_meta()->time_series_compaction_goal_size_mbytes();
- if (total_size >= compaction_goal_size_mbytes * 1024 * 1024) {
+ if (tablet->tablet_meta()->time_series_compaction_level_threshold() == 2) {
Review Comment:
what about 3?
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -288,7 +349,40 @@ void TimeSeriesCumulativeCompactionPolicy::update_cumulative_point(
return;
}
+ if (tablet->tablet_meta()->time_series_compaction_level_threshold() == 2 &&
+ output_rowset->rowset_meta()->compaction_level() < 2) {
+ return;
+ }
+
tablet->set_cumulative_layer_point(output_rowset->end_version() + 1);
}
+void TimeSeriesCumulativeCompactionPolicy::update_compaction_level(
+ Tablet* tablet, const std::vector<RowsetSharedPtr>& input_rowsets,
+ RowsetSharedPtr output_rowset) {
+ if (tablet->tablet_state() != TABLET_RUNNING || output_rowset->num_segments() == 0) {
+ return;
+ }
+
+ int64_t first_level = 0;
+ for (size_t i = 0; i < input_rowsets.size(); i++) {
+ int64_t cur_level = input_rowsets[i]->rowset_meta()->compaction_level();
+ if (i == 0) {
+ first_level = cur_level;
+ } else {
+ if (first_level != cur_level) {
+ LOG(ERROR) << "Failed to check compaction level, first_level: " << first_level
+ << ", cur_level: " << cur_level;
+ }
+ }
+ }
+
+ if (output_rowset->rowset_meta()->total_disk_size() >=
+ tablet->tablet_meta()->time_series_compaction_goal_size_mbytes() * 1024 * 1024) {
Review Comment:
You can use goal_size * percent (eg. 0.6) to avoid compaction for 0.9 + 0.9
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -163,6 +190,13 @@ void TimeSeriesCumulativeCompactionPolicy::calculate_cumulative_point(
break;
}
+ // upgrade: [0 0 2 1 1 0 0]
+ if (!is_delete && tablet->tablet_meta()->time_series_compaction_level_threshold() == 2 &&
+ rs->compaction_level() == 1) {
Review Comment:
what about [2, 2, 0, 0, 0]
##########
be/src/agent/task_worker_pool.cpp:
##########
@@ -746,6 +746,16 @@ void update_tablet_meta_callback(StorageEngine& engine, const TAgentTaskRequest&
tablet_meta_info.time_series_compaction_empty_rowsets_threshold);
need_to_save = true;
}
+ if (tablet_meta_info.__isset.time_series_compaction_level_threshold) {
+ if (tablet->tablet_meta()->compaction_policy() != "time_series") {
Review Comment:
change "time_series" literal to a const
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -288,7 +349,40 @@ void TimeSeriesCumulativeCompactionPolicy::update_cumulative_point(
return;
}
+ if (tablet->tablet_meta()->time_series_compaction_level_threshold() == 2 &&
+ output_rowset->rowset_meta()->compaction_level() < 2) {
+ return;
+ }
+
tablet->set_cumulative_layer_point(output_rowset->end_version() + 1);
}
+void TimeSeriesCumulativeCompactionPolicy::update_compaction_level(
+ Tablet* tablet, const std::vector<RowsetSharedPtr>& input_rowsets,
+ RowsetSharedPtr output_rowset) {
+ if (tablet->tablet_state() != TABLET_RUNNING || output_rowset->num_segments() == 0) {
+ return;
+ }
+
+ int64_t first_level = 0;
+ for (size_t i = 0; i < input_rowsets.size(); i++) {
+ int64_t cur_level = input_rowsets[i]->rowset_meta()->compaction_level();
+ if (i == 0) {
+ first_level = cur_level;
+ } else {
+ if (first_level != cur_level) {
+ LOG(ERROR) << "Failed to check compaction level, first_level: " << first_level
Review Comment:
What's the effect of LOG(ERROR)? If it just add a log and continue, it's not enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #31488:
URL: https://github.com/apache/doris/pull/31488#discussion_r1504147882
##########
be/src/olap/tablet_meta.cpp:
##########
@@ -896,6 +905,8 @@ bool operator==(const TabletMeta& a, const TabletMeta& b) {
if (a._time_series_compaction_empty_rowsets_threshold !=
b._time_series_compaction_empty_rowsets_threshold)
return false;
+ if (a._time_series_compaction_level_threshold != b._time_series_compaction_level_threshold)
+ return false;
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
if (a._time_series_compaction_level_threshold != b._time_series_compaction_level_threshold) {
return false;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #31488:
URL: https://github.com/apache/doris/pull/31488#issuecomment-1966421211
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on code in PR #31488:
URL: https://github.com/apache/doris/pull/31488#discussion_r1507196225
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -47,8 +50,13 @@ uint32_t TimeSeriesCumulativeCompactionPolicy::calc_cumulative_compaction_score(
continue;
} else {
// collect the rowsets of cumulative part
- total_size += rs_meta->total_disk_size();
score += rs_meta->get_compaction_score();
+ if (rs_meta->compaction_level() == 0) {
+ level0_total_size += rs_meta->total_disk_size();
+ level0_score += rs_meta->get_compaction_score();
+ } else {
+ checked_rs_metas.push_back(rs_meta);
Review Comment:
这个函数只是计算下分数，可以在这直接计算 level1 是否满足条件
##########
be/src/olap/compaction.cpp:
##########
@@ -133,10 +133,13 @@ void Compaction::init_profile(const std::string& label) {
Status Compaction::merge_input_rowsets() {
std::vector<RowsetReaderSharedPtr> input_rs_readers;
input_rs_readers.reserve(_input_rowsets.size());
+ int64_t max_compaction_level = 0;
for (auto& rowset : _input_rowsets) {
RowsetReaderSharedPtr rs_reader;
RETURN_IF_ERROR(rowset->create_reader(&rs_reader));
input_rs_readers.push_back(std::move(rs_reader));
+ max_compaction_level =
Review Comment:
每次 compaction 执行时，应该同一层的合并，还是level0 和 level1 的可以混合？
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -193,6 +223,41 @@ int TimeSeriesCumulativeCompactionPolicy::pick_input_rowsets(
return 0;
}
+ int64_t compaction_goal_size_mbytes =
+ tablet->tablet_meta()->time_series_compaction_goal_size_mbytes();
+
+ std::vector<RowsetSharedPtr> level1_rowsets;
+ if (tablet->tablet_meta()->time_series_compaction_level_threshold() == 2) {
+ int64_t continuous_size = 0;
+ int64_t cumulative_point = tablet->cumulative_layer_point();
+ for (const auto& rowset : candidate_rowsets) {
+ const auto& rs_meta = rowset->rowset_meta();
+ if (rs_meta->end_version() < cumulative_point) {
Review Comment:
可以删掉，传进来的candidate_rowsets 的版本一定是大于等于cumulative_point的
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -193,6 +223,41 @@ int TimeSeriesCumulativeCompactionPolicy::pick_input_rowsets(
return 0;
}
+ int64_t compaction_goal_size_mbytes =
+ tablet->tablet_meta()->time_series_compaction_goal_size_mbytes();
+
+ std::vector<RowsetSharedPtr> level1_rowsets;
+ if (tablet->tablet_meta()->time_series_compaction_level_threshold() == 2) {
+ int64_t continuous_size = 0;
+ int64_t cumulative_point = tablet->cumulative_layer_point();
+ for (const auto& rowset : candidate_rowsets) {
+ const auto& rs_meta = rowset->rowset_meta();
+ if (rs_meta->end_version() < cumulative_point) {
+ continue;
+ }
+ if (rs_meta->compaction_level() == 0) {
+ break;
+ }
+ int64_t rs_meta_size = rs_meta->total_disk_size();
+ if (rs_meta_size >= compaction_goal_size_mbytes * 2 * 1024 * 1024) {
Review Comment:
为什么要乘以2
##########
be/src/olap/compaction.cpp:
##########
@@ -133,10 +133,13 @@ void Compaction::init_profile(const std::string& label) {
Status Compaction::merge_input_rowsets() {
std::vector<RowsetReaderSharedPtr> input_rs_readers;
input_rs_readers.reserve(_input_rowsets.size());
+ int64_t max_compaction_level = 0;
for (auto& rowset : _input_rowsets) {
RowsetReaderSharedPtr rs_reader;
RETURN_IF_ERROR(rowset->create_reader(&rs_reader));
input_rs_readers.push_back(std::move(rs_reader));
+ max_compaction_level =
+ std::max(max_compaction_level, rowset->rowset_meta()->compaction_level());
}
Review Comment:
如果所有的输入版本都为空，不需要提升 compation_level
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -70,15 +78,37 @@ uint32_t TimeSeriesCumulativeCompactionPolicy::calc_cumulative_compaction_score(
return score;
}
- // Condition 1: the size of input files for compaction meets the requirement of parameter compaction_goal_size
int64_t compaction_goal_size_mbytes =
tablet->tablet_meta()->time_series_compaction_goal_size_mbytes();
- if (total_size >= compaction_goal_size_mbytes * 1024 * 1024) {
+ if (tablet->tablet_meta()->time_series_compaction_level_threshold() == 2) {
+ int32_t rs_meta_count = 0;
+ int64_t continuous_size = 0;
+ for (const auto& rs_meta : checked_rs_metas) {
Review Comment:
上面 rs_metas 是无序的，checked_rs_metas 里面也是无序的
##########
be/src/olap/compaction.cpp:
##########
@@ -133,10 +133,13 @@ void Compaction::init_profile(const std::string& label) {
Status Compaction::merge_input_rowsets() {
std::vector<RowsetReaderSharedPtr> input_rs_readers;
input_rs_readers.reserve(_input_rowsets.size());
+ int64_t max_compaction_level = 0;
for (auto& rowset : _input_rowsets) {
RowsetReaderSharedPtr rs_reader;
RETURN_IF_ERROR(rowset->create_reader(&rs_reader));
input_rs_readers.push_back(std::move(rs_reader));
+ max_compaction_level =
Review Comment:
如果是只能同一层的合并，这里校验下level是否相等？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](invert index) modify of time series compaction policy [doris]
Posted by "csun5285 (via GitHub)" <gi...@apache.org>.
csun5285 commented on code in PR #31488:
URL: https://github.com/apache/doris/pull/31488#discussion_r1507196225
##########
be/src/olap/cumulative_compaction_time_series_policy.cpp:
##########
@@ -47,8 +50,13 @@ uint32_t TimeSeriesCumulativeCompactionPolicy::calc_cumulative_compaction_score(
continue;
} else {
// collect the rowsets of cumulative part
- total_size += rs_meta->total_disk_size();
score += rs_meta->get_compaction_score();
+ if (rs_meta->compaction_level() == 0) {
+ level0_total_size += rs_meta->total_disk_size();
+ level0_score += rs_meta->get_compaction_score();
+ } else {
+ checked_rs_metas.push_back(rs_meta);
Review Comment:
这个函数只是计算下分数，可以在这直接计算 level1 是否满足条件
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org