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

[Feature](multi-catalog) Add memory tracker for orc reader/writer and arrow parquet writer。 #37257

Merged

Conversation

kaka11chen
Copy link
Contributor

@kaka11chen kaka11chen commented Jul 4, 2024

Proposed changes

backport #37234

@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.

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


#pragma once

#include "orc/MemoryPool.hh"
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'orc/MemoryPool.hh' file not found [clang-diagnostic-error]

#include "orc/MemoryPool.hh"
         ^

#endif // #if defined(USE_JEMALLOC) && defined(USE_MEM_TRACKER)
}

void free(char* p) override {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: pointer parameter 'p' can be pointer to const [readability-non-const-parameter]

Suggested change
void free(char* p) override {
void free(const char* p) override {

@@ -122,7 +123,12 @@ VOrcTransformer::VOrcTransformer(RuntimeState* state, doris::io::FileWriter* fil
set_compression_type(compress_type);
}

VOrcTransformer::~VOrcTransformer() {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use '= default' to define a trivial destructor [modernize-use-equals-default]

VOrcTransformer::~VOrcTransformer() {
                 ^

@@ -84,7 +84,7 @@ class VOrcTransformer final : public VFileFormatTransformer {
TFileCompressType::type compression,
const iceberg::Schema* iceberg_schema = nullptr);

~VOrcTransformer() = default;
~VOrcTransformer();
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]

Suggested change
~VOrcTransformer();
~VOrcTransformer() override;

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch 3 times, most recently from cf824f8 to 9d35789 Compare July 4, 2024 05:48
@kaka11chen
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.32% (9150/25194)
Line Coverage: 27.88% (74698/267971)
Region Coverage: 26.77% (38518/143874)
Branch Coverage: 23.47% (19520/83168)
Coverage Report: http://coverage.selectdb-in.cc/coverage/9d35789ab994b03d9c4e892ca6ab5befaa2d0260_9d35789ab994b03d9c4e892ca6ab5befaa2d0260/report/index.html

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from 9d35789 to bb95ae7 Compare July 5, 2024 03:36
@zgxme
Copy link
Contributor

zgxme commented Jul 5, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.31% (9151/25199)
Line Coverage: 27.87% (74696/268060)
Region Coverage: 26.76% (38517/143945)
Branch Coverage: 23.45% (19518/83230)
Coverage Report: http://coverage.selectdb-in.cc/coverage/bb95ae7145370a6f5f37949e561d0a51e23dfe73_bb95ae7145370a6f5f37949e561d0a51e23dfe73/report/index.html

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from bb95ae7 to 4ebdd75 Compare July 9, 2024 07:13
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

be/src/vec/common/allocator.cpp Show resolved Hide resolved
be/src/vec/common/allocator.h Show resolved Hide resolved
@kaka11chen
Copy link
Contributor Author

run buildall

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch 2 times, most recently from 37921e7 to 7df36ce Compare July 9, 2024 08:08
@kaka11chen
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

be/src/vec/common/allocator.cpp Show resolved Hide resolved
@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from 7df36ce to fa2511b Compare July 9, 2024 08:32
@kaka11chen
Copy link
Contributor Author

run buildall

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from fa2511b to a879062 Compare July 9, 2024 08:57
@kaka11chen
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.31% (9158/25219)
Line Coverage: 27.87% (74767/268237)
Region Coverage: 26.77% (38577/144101)
Branch Coverage: 23.47% (19548/83298)
Coverage Report: http://coverage.selectdb-in.cc/coverage/a879062fdfbff971bd9172f6d62c09943acb09c6_a879062fdfbff971bd9172f6d62c09943acb09c6/report/index.html

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from a879062 to 8d619bf Compare July 11, 2024 07:31
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

be/src/vec/common/allocator.cpp Show resolved Hide resolved
@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from 8d619bf to 78d16de Compare July 11, 2024 07:49
@kaka11chen
Copy link
Contributor Author

run buildall

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from 78d16de to 9b362e4 Compare July 11, 2024 08:48
@kaka11chen
Copy link
Contributor Author

run buildall

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from 9b362e4 to ed40639 Compare July 11, 2024 09:24
@kaka11chen
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.41% (9245/25393)
Line Coverage: 27.98% (75548/270023)
Region Coverage: 26.80% (38835/144923)
Branch Coverage: 23.55% (19717/83734)
Coverage Report: http://coverage.selectdb-in.cc/coverage/608f36c6fc49b4c63eccc19ac4ba38acccfb2bc3_608f36c6fc49b4c63eccc19ac4ba38acccfb2bc3/report/index.html

@kaka11chen
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

return arrow::Status::OK();
}

void ArrowAllocator::deallocate_aligned(uint8_t* ptr, int64_t size, int64_t alignment) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: pointer parameter 'ptr' can be pointer to const [readability-non-const-parameter]

Suggested change
void ArrowAllocator::deallocate_aligned(uint8_t* ptr, int64_t size, int64_t alignment) {
void ArrowAllocator::deallocate_aligned(const uint8_t* ptr, int64_t size, int64_t alignment) {

be/src/vec/exec/format/parquet/arrow_memory_pool.h:47:

-     void deallocate_aligned(uint8_t* ptr, int64_t size, int64_t alignment);
+     void deallocate_aligned(const uint8_t* ptr, int64_t size, int64_t alignment);

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch 2 times, most recently from 64a4a7e to bf1a1a9 Compare July 23, 2024 17:56
@kaka11chen
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

be/src/vec/common/allocator.h Show resolved Hide resolved
be/src/vec/common/allocator.h Show resolved Hide resolved
@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.39% (9242/25396)
Line Coverage: 27.97% (75531/270063)
Region Coverage: 26.79% (38835/144955)
Branch Coverage: 23.54% (19715/83762)
Coverage Report: http://coverage.selectdb-in.cc/coverage/bf1a1a9416d265e469de8b9021ee6d004adbe8ef_bf1a1a9416d265e469de8b9021ee6d004adbe8ef/report/index.html

Copy link
Contributor

@xinyiZzz xinyiZzz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Jul 24, 2024
Copy link
Contributor

PR approved by anyone and no changes requested.

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from bf1a1a9 to 8f3272d Compare July 24, 2024 06:19
@kaka11chen
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.40% (9243/25396)
Line Coverage: 27.97% (75534/270071)
Region Coverage: 26.79% (38830/144960)
Branch Coverage: 23.53% (19712/83764)
Coverage Report: http://coverage.selectdb-in.cc/coverage/8f3272d573f85eac5bf4ac38c04a24caaa92e5bf_8f3272d573f85eac5bf4ac38c04a24caaa92e5bf/report/index.html

@kaka11chen kaka11chen force-pushed the orc_parquet_memory_tracking_2.1 branch from 8f3272d to d746ddc Compare July 24, 2024 08:29
@kaka11chen
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.40% (9243/25396)
Line Coverage: 27.97% (75533/270094)
Region Coverage: 26.78% (38821/144981)
Branch Coverage: 23.53% (19711/83784)
Coverage Report: http://coverage.selectdb-in.cc/coverage/d746ddc44603b8c12ecf6a09c79c8f1681a95dbf_d746ddc44603b8c12ecf6a09c79c8f1681a95dbf/report/index.html

@morningman morningman merged commit a751372 into apache:branch-2.1 Jul 25, 2024
19 of 21 checks passed
yiguolei pushed a commit that referenced this pull request Aug 1, 2024
Backport #38562. Fix allocator.h compiling failed on mac which
introduced by #37257.
@yiguolei yiguolei mentioned this pull request Sep 5, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants