Skip to content

[Improvement](pipeline) Do parallel preparation for multiple fragments#33989

Closed
Gabriel39 wants to merge 1 commit intoapache:masterfrom
Gabriel39:imp_04221
Closed

[Improvement](pipeline) Do parallel preparation for multiple fragments#33989
Gabriel39 wants to merge 1 commit intoapache:masterfrom
Gabriel39:imp_04221

Conversation

@Gabriel39
Copy link
Contributor

Proposed changes

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@Gabriel39
Copy link
Contributor Author

run buildall

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions


template <typename Params>
Status FragmentMgr::_get_query_ctx(const Params& params, TUniqueId query_id, bool pipeline,
Status FragmentMgr::_get_query_ctx(Params& params, TUniqueId query_id, bool pipeline,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function '_get_query_ctx' has cognitive complexity of 103 (threshold 50) [readability-function-cognitive-complexity]

Status FragmentMgr::_get_query_ctx(Params& params, TUniqueId query_id, bool pipeline,
                    ^
Additional context

be/src/runtime/fragment_mgr.cpp:627: +1

    const bool enable_pipeline_x = params.query_options.__isset.enable_pipeline_x_engine &&
                                                                                         ^

be/src/runtime/fragment_mgr.cpp:629: +1, including nesting penalty of 0, nesting level increased to 1

    if (enable_pipeline_x && params.__isset.desc_tbl) {
    ^

be/src/runtime/fragment_mgr.cpp:629: +1

    if (enable_pipeline_x && params.__isset.desc_tbl) {
                          ^

be/src/runtime/fragment_mgr.cpp:632: +2, including nesting penalty of 1, nesting level increased to 2

        if (search == _query_ctx_map.end()) {
        ^

be/src/runtime/fragment_mgr.cpp:633: +3, including nesting penalty of 2, nesting level increased to 3

            if constexpr (std::is_same_v<Params, TPipelineFragmentParams>) {
            ^

be/src/runtime/fragment_mgr.cpp:643: +3, including nesting penalty of 2, nesting level increased to 3

            RETURN_IF_ERROR(DescriptorTbl::create(&(query_ctx->obj_pool), params.desc_tbl,
            ^

be/src/common/status.h:541: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/runtime/fragment_mgr.cpp:643: +4, including nesting penalty of 3, nesting level increased to 4

            RETURN_IF_ERROR(DescriptorTbl::create(&(query_ctx->obj_pool), params.desc_tbl,
            ^

be/src/common/status.h:543: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/runtime/fragment_mgr.cpp:646: +3, including nesting penalty of 2, nesting level increased to 3

            if (params.__isset.file_scan_params) {
            ^

be/src/runtime/fragment_mgr.cpp:657: +3, including nesting penalty of 2, nesting level increased to 3

            if (params.__isset.resource_info) {
            ^

be/src/runtime/fragment_mgr.cpp:666: +3, including nesting penalty of 2, nesting level increased to 3

            if (params.__isset.workload_groups && !params.workload_groups.empty()) {
            ^

be/src/runtime/fragment_mgr.cpp:666: +1

            if (params.__isset.workload_groups && !params.workload_groups.empty()) {
                                               ^

be/src/runtime/fragment_mgr.cpp:670: +4, including nesting penalty of 3, nesting level increased to 4

                if (workload_group_ptr != nullptr) {
                ^

be/src/runtime/fragment_mgr.cpp:671: +5, including nesting penalty of 4, nesting level increased to 5

                    RETURN_IF_ERROR(workload_group_ptr->add_query(query_id, query_ctx));
                    ^

be/src/common/status.h:541: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/runtime/fragment_mgr.cpp:671: +6, including nesting penalty of 5, nesting level increased to 6

                    RETURN_IF_ERROR(workload_group_ptr->add_query(query_id, query_ctx));
                    ^

be/src/common/status.h:543: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/runtime/fragment_mgr.cpp:672: +5, including nesting penalty of 4, nesting level increased to 5

                    RETURN_IF_ERROR(query_ctx->set_workload_group(workload_group_ptr));
                    ^

be/src/common/status.h:541: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/runtime/fragment_mgr.cpp:672: +6, including nesting penalty of 5, nesting level increased to 6

                    RETURN_IF_ERROR(query_ctx->set_workload_group(workload_group_ptr));
                    ^

be/src/common/status.h:543: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/runtime/fragment_mgr.cpp:680: +1, nesting level increased to 4

                } else {
                  ^

be/src/runtime/fragment_mgr.cpp:685: +3, including nesting penalty of 2, nesting level increased to 3

            RETURN_IF_ERROR(DescriptorTbl::create(&(query_ctx->obj_pool), params.desc_tbl,
            ^

be/src/common/status.h:541: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/runtime/fragment_mgr.cpp:685: +4, including nesting penalty of 3, nesting level increased to 4

            RETURN_IF_ERROR(DescriptorTbl::create(&(query_ctx->obj_pool), params.desc_tbl,
            ^

be/src/common/status.h:543: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/runtime/fragment_mgr.cpp:693: +1, nesting level increased to 2

        } else {
          ^

be/src/runtime/fragment_mgr.cpp:694: +3, including nesting penalty of 2, nesting level increased to 3

            if constexpr (std::is_same_v<Params, TPipelineFragmentParams>) {
            ^

be/src/runtime/fragment_mgr.cpp:701: +1, including nesting penalty of 0, nesting level increased to 1

    if (params.is_simplified_param) {
    ^

be/src/runtime/fragment_mgr.cpp:705: +2, including nesting penalty of 1, nesting level increased to 2

        if (search == _query_ctx_map.end()) {
        ^

be/src/runtime/fragment_mgr.cpp:712: +1, nesting level increased to 1

    } else {
      ^

be/src/runtime/fragment_mgr.cpp:717: +2, including nesting penalty of 1, nesting level increased to 2

        if (search != _query_ctx_map.end()) {
        ^

be/src/runtime/fragment_mgr.cpp:728: +2, including nesting penalty of 1, nesting level increased to 2

        RETURN_IF_ERROR(DescriptorTbl::create(&(query_ctx->obj_pool), params.desc_tbl,
        ^

be/src/common/status.h:541: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/runtime/fragment_mgr.cpp:728: +3, including nesting penalty of 2, nesting level increased to 3

        RETURN_IF_ERROR(DescriptorTbl::create(&(query_ctx->obj_pool), params.desc_tbl,
        ^

be/src/common/status.h:543: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/runtime/fragment_mgr.cpp:731: +2, including nesting penalty of 1, nesting level increased to 2

        if (params.__isset.file_scan_params) {
        ^

be/src/runtime/fragment_mgr.cpp:741: +2, including nesting penalty of 1, nesting level increased to 2

        if (params.__isset.resource_info) {
        ^

be/src/runtime/fragment_mgr.cpp:751: +2, including nesting penalty of 1, nesting level increased to 2

        if (params.__isset.workload_groups && !params.workload_groups.empty()) {
        ^

be/src/runtime/fragment_mgr.cpp:751: +1

        if (params.__isset.workload_groups && !params.workload_groups.empty()) {
                                           ^

be/src/runtime/fragment_mgr.cpp:755: +3, including nesting penalty of 2, nesting level increased to 3

            if (workload_group_ptr != nullptr) {
            ^

be/src/runtime/fragment_mgr.cpp:756: +4, including nesting penalty of 3, nesting level increased to 4

                RETURN_IF_ERROR(workload_group_ptr->add_query(query_id, query_ctx));
                ^

be/src/common/status.h:541: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/runtime/fragment_mgr.cpp:756: +5, including nesting penalty of 4, nesting level increased to 5

                RETURN_IF_ERROR(workload_group_ptr->add_query(query_id, query_ctx));
                ^

be/src/common/status.h:543: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/runtime/fragment_mgr.cpp:757: +4, including nesting penalty of 3, nesting level increased to 4

                RETURN_IF_ERROR(query_ctx->set_workload_group(workload_group_ptr));
                ^

be/src/common/status.h:541: expanded from macro 'RETURN_IF_ERROR'

    do {                                \
    ^

be/src/runtime/fragment_mgr.cpp:757: +5, including nesting penalty of 4, nesting level increased to 5

                RETURN_IF_ERROR(query_ctx->set_workload_group(workload_group_ptr));
                ^

be/src/common/status.h:543: expanded from macro 'RETURN_IF_ERROR'

        if (UNLIKELY(!_status_.ok())) { \
        ^

be/src/runtime/fragment_mgr.cpp:766: +1, nesting level increased to 3

            } else {
              ^


template <typename Params>
Status FragmentMgr::_get_query_ctx(const Params& params, TUniqueId query_id, bool pipeline,
Status FragmentMgr::_get_query_ctx(Params& params, TUniqueId query_id, bool pipeline,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function '_get_query_ctx' exceeds recommended size/complexity thresholds [readability-function-size]

Status FragmentMgr::_get_query_ctx(Params& params, TUniqueId query_id, bool pipeline,
                    ^
Additional context

be/src/runtime/fragment_mgr.cpp:625: 153 lines including whitespace and comments (threshold 80)

Status FragmentMgr::_get_query_ctx(Params& params, TUniqueId query_id, bool pipeline,
                    ^

@doris-robot
Copy link

TPC-H: Total hot run time: 38377 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 08ca82e9b15d9a37d011d7a77a1dfec153aeff6b, data reload: false

------ Round 1 ----------------------------------
q1	17640	4395	4258	4258
q2	2003	181	186	181
q3	10457	1202	1144	1144
q4	10191	811	789	789
q5	7484	2674	2616	2616
q6	218	130	134	130
q7	1039	606	602	602
q8	9212	2057	2035	2035
q9	7309	6579	6535	6535
q10	8557	3501	3495	3495
q11	440	227	214	214
q12	426	219	214	214
q13	17760	2921	2936	2921
q14	271	225	239	225
q15	512	475	459	459
q16	520	398	379	379
q17	956	652	686	652
q18	7357	6791	6658	6658
q19	5848	1443	1538	1443
q20	628	306	292	292
q21	3408	2839	2878	2839
q22	360	305	296	296
Total cold run time: 112596 ms
Total hot run time: 38377 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4373	4220	4158	4158
q2	375	255	263	255
q3	3029	2736	2748	2736
q4	1822	1577	1570	1570
q5	5289	5329	5291	5291
q6	209	123	125	123
q7	2263	1826	1863	1826
q8	3188	3313	3342	3313
q9	8546	8575	8574	8574
q10	3885	3904	3831	3831
q11	597	501	495	495
q12	781	622	655	622
q13	17417	3167	3116	3116
q14	305	312	285	285
q15	532	482	479	479
q16	503	438	432	432
q17	1790	1502	1487	1487
q18	8383	7909	7978	7909
q19	1685	1567	1520	1520
q20	2057	1877	1830	1830
q21	9196	5046	4847	4847
q22	545	457	455	455
Total cold run time: 76770 ms
Total hot run time: 55154 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.37% (8916/25210)
Line Coverage: 27.08% (73318/270704)
Region Coverage: 26.24% (37885/144405)
Branch Coverage: 23.04% (19287/83722)
Coverage Report: http://coverage.selectdb-in.cc/coverage/08ca82e9b15d9a37d011d7a77a1dfec153aeff6b_08ca82e9b15d9a37d011d7a77a1dfec153aeff6b/report/index.html

@Gabriel39
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.36% (8915/25210)
Line Coverage: 27.08% (73317/270705)
Region Coverage: 26.23% (37875/144404)
Branch Coverage: 23.03% (19285/83722)
Coverage Report: http://coverage.selectdb-in.cc/coverage/8bcc611fb62d94e0b78dcecee58bef0f1b68543a_8bcc611fb62d94e0b78dcecee58bef0f1b68543a/report/index.html

@Gabriel39
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.17% (8913/25340)
Line Coverage: 26.95% (73318/272037)
Region Coverage: 26.12% (37864/144936)
Branch Coverage: 22.96% (19285/83992)
Coverage Report: http://coverage.selectdb-in.cc/coverage/b9653b52761e180e116b90ae8436b9b0633c8f89_b9653b52761e180e116b90ae8436b9b0633c8f89/report/index.html

@Gabriel39 Gabriel39 closed this Sep 18, 2024
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.

2 participants