Skip to content

Improve lifetime management of projection part during query#81191

Merged
kssenii merged 3 commits intoClickHouse:masterfrom
amosbird:improve-projection-part-lifetime
Jun 9, 2025
Merged

Improve lifetime management of projection part during query#81191
kssenii merged 3 commits intoClickHouse:masterfrom
amosbird:improve-projection-part-lifetime

Conversation

@amosbird
Copy link
Copy Markdown
Collaborator

@amosbird amosbird commented Jun 3, 2025

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

The previous fix #63513 relied on excessive part locking during query planning, which impacted performance. After the recent refactoring PR #79417, the lifetime management can now be improved with a much simpler and more efficient approach.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

data_part->getParentPartName(), data_part->getDataPartStorage().getFullPath());
}

read_task_info.parent_part = part_with_ranges.parent_part;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Here is the improvement.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jun 3, 2025

Workflow [PR], commit [cc33163]

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jun 3, 2025
@amosbird amosbird force-pushed the improve-projection-part-lifetime branch from 84969b4 to 9459598 Compare June 3, 2025 07:58
@amosbird
Copy link
Copy Markdown
Collaborator Author

amosbird commented Jun 3, 2025

AST fuzzer (amd_ubsan)

#79451

@kssenii kssenii self-assigned this Jun 3, 2025
@kssenii
Copy link
Copy Markdown
Member

kssenii commented Jun 6, 2025

@amosbird please also check build failure, I guess you need to fetch recent changes from master

FAILED: src/CMakeFiles/dbms.dir/Processors/QueryPlan/Optimizations/projectionsCommon.cpp.o 
/usr/bin/sccache /usr/bin/clang++-19 --target=x86_64-linux-gnu --sysroot=/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/cmake/linux/../../contrib/sysroot/linux-x86_64/x86_64-linux-gnu/libc -DBOOST_ASIO_HAS_STD_INVOKE_RESULT=1 -DBOOST_ASIO_STANDALONE=1 -DBOOST_TIMER_ENABLE_DEPRECATED=1 -DDUMMY_BACKTRACE -DENABLE_MULTITARGET_CODE=1 -DFIU_ENABLE -DINCBIN_SILENCE_BITCODE_WARNING -DLZ4_FAST_DEC_LOOP=1 -DPOCO_ENABLE_CPP11 -DPOCO_HAVE_FD_EPOLL -DPOCO_OS_FAMILY_UNIX -DSTD_EXCEPTION_HAS_STACK_TRACE=1 -DUNALIGNED_OK -DUSE_CLICKHOUSE_THREADS=1 -DWITH_COVERAGE=0 -DWITH_GZFILEOP -DX86_64 -DZLIB_COMPAT -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -D_LIBUNWIND_IS_NATIVE_ONLY -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/ci/tmp/build/includes/configs -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/src -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/ci/tmp/build/src -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/ci/tmp/build/contrib/llvm-project/libcxx/include/c++/v1 -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/glibc-compatibility/memcpy -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/base/.. -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/ci/tmp/build/base/base/.. -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/cctz/include -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/re2 -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/pcg-random/. -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/libfiu/libfiu -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/miniselect/include -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/zstd/lib -I/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/lz4/lib -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/llvm-project/libcxx/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/llvm-project/libcxxabi/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/llvm-project/libunwind/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/libdivide-cmake/. -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/libdivide -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/jemalloc-cmake/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/abseil-cpp -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/croaring/cpp -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/croaring/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/sparsehash-c11 -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/incbin -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/llvm-project/compiler-rt/lib -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/cityhash102/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/boost -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/poco/Net/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/poco/Foundation/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/poco/Util/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/poco/JSON/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/poco/XML/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/replxx/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/fmtlib-cmake/../fmtlib/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/magic_enum/include/magic_enum -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/double-conversion -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/dragonbox/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/zlib-ng -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/ci/tmp/build/contrib/zlib-ng-cmake -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/pdqsort -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/xz/src/liblzma/api -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/ci/tmp/build/contrib/liburing/src/include-compat -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/ci/tmp/build/contrib/liburing/src/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/liburing/src/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/fast_float/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/simdjson/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/NuRaft/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/base/poco/Redis/include -isystem /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/contrib/consistent-hashing --gcc-toolchain=/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/cmake/linux/../../contrib/sysroot/linux-x86_64 -fdiagnostics-color=always -Xclang -fuse-ctor-homing -fsized-deallocation  -gdwarf-aranges -pipe -mssse3 -msse4.1 -msse4.2 -mpclmul -mpopcnt -fasynchronous-unwind-tables -ffile-prefix-map=/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse=. -falign-functions=32 -mbranches-within-32B-boundaries -ffp-contract=off  -fdiagnostics-absolute-paths -fstrict-vtable-pointers -fPIC -Wall -Wextra -Wframe-larger-than=65536 -Weverything -Wpedantic -Wvla-cxx-extension -Wno-return-type-c-linkage -Wno-zero-length-array -Wno-c++98-compat-pedantic -Wno-c++98-compat -Wno-c++20-compat -Wno-sign-conversion -Wno-implicit-int-conversion -Wno-implicit-int-float-conversion -Wno-ctad-maybe-unsupported -Wno-disabled-macro-expansion -Wno-documentation-unknown-command -Wno-double-promotion -Wno-exit-time-destructors -Wno-float-equal -Wno-global-constructors -Wno-missing-prototypes -Wno-missing-variable-declarations -Wno-padded -Wno-switch-enum -Wno-undefined-func-template -Wno-unused-template -Wno-weak-template-vtables -Wno-weak-vtables -Wno-thread-safety-negative -Wno-unsafe-buffer-usage -Wno-switch-default -O2 -g -DNDEBUG -O3 -g  -std=c++23   -D OS_LINUX -Werror -nostdinc++ -MD -MT src/CMakeFiles/dbms.dir/Processors/QueryPlan/Optimizations/projectionsCommon.cpp.o -MF src/CMakeFiles/dbms.dir/Processors/QueryPlan/Optimizations/projectionsCommon.cpp.o.d -o src/CMakeFiles/dbms.dir/Processors/QueryPlan/Optimizations/projectionsCommon.cpp.o -c /home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/src/Processors/QueryPlan/Optimizations/projectionsCommon.cpp
/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/src/Processors/QueryPlan/Optimizations/projectionsCommon.cpp:342:30: error: no matching constructor for initialization of 'RangesInDataPart'
  342 |             RangesInDataPart projection_part(
      |                              ^
  343 |                 it->second, part_with_ranges.part_index_in_query, part_with_ranges.part_starting_offset_in_query);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/src/Storages/MergeTree/RangesInDataPart.h:59:14: note: candidate constructor not viable: no known conversion from 'const size_t' (aka 'const unsigned long') to 'const DataPartPtr' (aka 'const shared_ptr<const IMergeTreeDataPart>') for 2nd argument
   59 |     explicit RangesInDataPart(
      |              ^
   60 |         const DataPartPtr & data_part_,
   61 |         const DataPartPtr & parent_part_ = nullptr,
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/src/Storages/MergeTree/RangesInDataPart.h:43:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 3 were provided
   43 | struct RangesInDataPart
      |        ^~~~~~~~~~~~~~~~
/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/src/Storages/MergeTree/RangesInDataPart.h:43:8: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 3 were provided
   43 | struct RangesInDataPart
      |        ^~~~~~~~~~~~~~~~
/home/ubuntu/actions-runner/_work/ClickHouse/ClickHouse/src/Storages/MergeTree/RangesInDataPart.h:52:5: note: candidate constructor not viable: requires 5 arguments, but 3 were provided
   52 |     RangesInDataPart(
      |     ^
   53 |         const DataPartPtr & data_part_,
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   54 |         const DataPartPtr & parent_part_,
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   55 |         size_t part_index_in_query_,
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   56 |         size_t part_starting_offset_in_query_,
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   57 |         const MarkRanges & ranges_);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~

@amosbird amosbird force-pushed the improve-projection-part-lifetime branch from 7fe3a0d to cc33163 Compare June 6, 2025 17:48
@kssenii kssenii enabled auto-merge June 6, 2025 17:56
@kssenii
Copy link
Copy Markdown
Member

kssenii commented Jun 9, 2025

Integration tests

test_storage_kafka

@kssenii kssenii added this pull request to the merge queue Jun 9, 2025
Merged via the queue into ClickHouse:master with commit 939f2fe Jun 9, 2025
118 of 121 checks passed
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jun 9, 2025
baibaichen pushed a commit to Kyligence/gluten that referenced this pull request Jun 10, 2025
baibaichen pushed a commit to Kyligence/gluten that referenced this pull request Jun 11, 2025
baibaichen pushed a commit to apache/gluten that referenced this pull request Jun 11, 2025
* [GLUTEN-1632][CH]Daily Update Clickhouse Version (20250611)

* Fix Build due to ClickHouse/ClickHouse#81191

* Fix UT due to ClickHouse/ClickHouse#79471

---------

Co-authored-by: kyligence-git <[email protected]>
Co-authored-by: Chang chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants