You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "eldenmoon (via GitHub)" <gi...@apache.org> on 2026/03/29 08:31:47 UTC
[PR] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
eldenmoon opened a new pull request, #61851:
URL: https://github.com/apache/doris/pull/61851
(no comment)
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #61851:
URL: https://github.com/apache/doris/pull/61851#discussion_r3005893106
##########
be/src/util/pprof_utils.cpp:
##########
@@ -128,6 +128,30 @@ Status PprofUtils::get_readable_profile(const std::string& file_or_content, bool
return Status::OK();
}
+Status PprofUtils::get_svg_profile(const std::string& profile_file, std::string* output) {
+ std::string pprof_cmd;
+ RETURN_IF_ERROR(PprofUtils::get_pprof_cmd(&pprof_cmd));
+
+ std::string self_cmdline;
+ RETURN_IF_ERROR(PprofUtils::get_self_cmdline(&self_cmdline));
+
+ std::string cmd_output;
+ std::string final_cmd =
+ pprof_cmd + absl::Substitute(" --svg $0 $1", self_cmdline, profile_file);
+ AgentUtils util;
+ LOG(INFO) << "begin to run command: " << final_cmd;
+ bool rc = util.exec_cmd(final_cmd, &cmd_output, false);
+
+ static_cast<void>(io::global_local_filesystem()->delete_file(profile_file));
+
+ if (!rc) {
+ return Status::InternalError("Failed to execute command: {}", cmd_output);
+ }
+
Review Comment:
`get_svg_profile()` unconditionally deletes `profile_file` (and does so before checking `rc`). This is a surprising side effect for a function that appears to just “get”/convert a profile, and it also makes failures harder to debug because the input is removed even when `pprof` fails. Consider moving deletion to the caller, or adding a `delete_after_parse` flag and only deleting on success (or when explicitly requested).
```suggestion
if (!rc) {
return Status::InternalError("Failed to execute command: {}", cmd_output);
}
// Best-effort cleanup of the profile file after successful conversion.
static_cast<void>(io::global_local_filesystem()->delete_file(profile_file));
```
##########
be/src/service/http/action/pprof_actions.cpp:
##########
@@ -157,52 +157,43 @@ void ProfileAction::handle(HttpRequest* req) {
}
const std::string& type_str = req->param("type");
- if (type_str != "flamegraph") {
- // use pprof the sample the CPU
- std::ostringstream tmp_prof_file_name;
- tmp_prof_file_name << config::pprof_profile_dir << "/doris_profile." << getpid() << "."
- << rand();
- ProfilerStart(tmp_prof_file_name.str().c_str());
- sleep(seconds);
- ProfilerStop();
-
- if (type_str != "text") {
- // return raw content via http response directly
- std::ifstream prof_file(tmp_prof_file_name.str().c_str(), std::ios::in);
- std::stringstream ss;
- if (!prof_file.is_open()) {
- ss << "Unable to open cpu profile: " << tmp_prof_file_name.str();
- std::string str = ss.str();
- HttpChannel::send_reply(req, str);
- return;
- }
- ss << prof_file.rdbuf();
- prof_file.close();
- std::string str = ss.str();
- HttpChannel::send_reply(req, str);
- return;
- }
+ std::ostringstream tmp_prof_file_name;
+ tmp_prof_file_name << config::pprof_profile_dir << "/doris_profile." << getpid() << "."
+ << rand();
+ ProfilerStart(tmp_prof_file_name.str().c_str());
+ sleep(seconds);
+ ProfilerStop();
- // text type. we will return readable content via http response
+ if (type_str == "text") {
std::stringstream readable_res;
Status st = PprofUtils::get_readable_profile(tmp_prof_file_name.str(), true, &readable_res);
if (!st.ok()) {
HttpChannel::send_reply(req, st.to_string());
} else {
HttpChannel::send_reply(req, readable_res.str());
}
- } else {
- // generate flamegraph
+ } else if (type_str == "flamegraph") {
std::string svg_file_content;
- std::string flamegraph_install_dir =
- std::string(std::getenv("DORIS_HOME")) + "/tools/FlameGraph/";
- Status st = PprofUtils::generate_flamegraph(seconds, flamegraph_install_dir, false,
- &svg_file_content);
+ Status st = PprofUtils::get_svg_profile(tmp_prof_file_name.str(), &svg_file_content);
if (!st.ok()) {
HttpChannel::send_reply(req, st.to_string());
} else {
HttpChannel::send_reply(req, svg_file_content);
}
+ } else {
+ std::ifstream prof_file(tmp_prof_file_name.str().c_str(), std::ios::in);
+ std::stringstream ss;
+ if (!prof_file.is_open()) {
+ ss << "Unable to open cpu profile: " << tmp_prof_file_name.str();
+ std::string str = ss.str();
+ HttpChannel::send_reply(req, str);
Review Comment:
In the raw-profile branch, when `prof_file.is_open()` fails the function returns without attempting to clean up the temporary profile file. Since this file was just created by `ProfilerStart/Stop`, consider deleting it (best-effort) on this error path as well to avoid leaving behind temp files in `pprof_profile_dir`.
```suggestion
HttpChannel::send_reply(req, str);
static_cast<void>(io::global_local_filesystem()->delete_file(tmp_prof_file_name.str()));
```
##########
be/src/util/pprof_utils.cpp:
##########
@@ -128,6 +128,30 @@ Status PprofUtils::get_readable_profile(const std::string& file_or_content, bool
return Status::OK();
}
+Status PprofUtils::get_svg_profile(const std::string& profile_file, std::string* output) {
+ std::string pprof_cmd;
+ RETURN_IF_ERROR(PprofUtils::get_pprof_cmd(&pprof_cmd));
+
+ std::string self_cmdline;
+ RETURN_IF_ERROR(PprofUtils::get_self_cmdline(&self_cmdline));
+
+ std::string cmd_output;
+ std::string final_cmd =
+ pprof_cmd + absl::Substitute(" --svg $0 $1", self_cmdline, profile_file);
+ AgentUtils util;
+ LOG(INFO) << "begin to run command: " << final_cmd;
+ bool rc = util.exec_cmd(final_cmd, &cmd_output, false);
Review Comment:
`get_svg_profile()` builds a shell command by concatenating `self_cmdline` and `profile_file` into a single string and executes it via `popen()`. Because `popen()` invokes `/bin/sh -c`, any spaces or shell metacharacters in these paths (e.g., from `config::pprof_profile_dir`) can break the command or be interpreted by the shell. Prefer executing without a shell (execve/posix_spawn with argv), or at minimum apply robust shell-escaping/quoting to both arguments before concatenation.
##########
be/src/util/pprof_utils.h:
##########
@@ -44,6 +44,9 @@ class PprofUtils {
static Status get_readable_profile(const std::string& file_or_content, bool is_file,
std::stringstream* output);
+ /// generate svg output by `pprof --svg doris_be profile`.
Review Comment:
The new `get_svg_profile()` API deletes the input file as an implicit side effect (per implementation). Please document this behavior in the header comment (or adjust the API to avoid deleting caller-owned files), so callers don’t accidentally pass a non-temporary profile they intend to keep.
```suggestion
/// generate svg output by `pprof --svg doris_be profile_file`.
/// NOTE: `profile_file` is expected to be a temporary file. Implementations of this API
/// may delete `profile_file` after generating the SVG output, so callers must not pass
/// a profile file they intend to keep.
```
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #61851:
URL: https://github.com/apache/doris/pull/61851#issuecomment-4149740399
<details>
<summary>TPC-H: <b>Total hot run time: 26586 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 8664e157270f4373a3848b4778e32d7869e4cc26, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17668 4561 4289 4289
q2 q3 10643 778 514 514
q4 4674 360 254 254
q5 7563 1205 1030 1030
q6 186 177 145 145
q7 791 830 684 684
q8 9785 1444 1313 1313
q9 5230 4748 4701 4701
q10 6330 1974 1633 1633
q11 487 261 236 236
q12 764 583 459 459
q13 18033 2675 1931 1931
q14 235 231 207 207
q15 q16 726 734 672 672
q17 736 819 438 438
q18 6057 5320 5209 5209
q19 1483 966 627 627
q20 517 487 372 372
q21 4635 1847 1567 1567
q22 464 321 305 305
Total cold run time: 97007 ms
Total hot run time: 26586 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4881 4721 4687 4687
q2 q3 3907 4380 3791 3791
q4 909 1230 784 784
q5 4047 4354 4333 4333
q6 183 171 139 139
q7 1827 1662 1601 1601
q8 2545 2714 2541 2541
q9 7598 7384 7513 7384
q10 3772 3989 3617 3617
q11 513 431 423 423
q12 525 610 440 440
q13 2513 3034 2230 2230
q14 299 309 283 283
q15 q16 772 807 703 703
q17 1216 1379 1393 1379
q18 7178 6800 6698 6698
q19 1022 908 858 858
q20 2096 2195 1983 1983
q21 4018 3490 3373 3373
q22 453 421 394 394
Total cold run time: 50274 ms
Total hot run time: 47641 ms
```
</details>
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #61851:
URL: https://github.com/apache/doris/pull/61851#issuecomment-4149818766
# BE UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/8664e157270f4373a3848b4778e32d7869e4cc26_8664e157270f4373a3848b4778e32d7869e4cc26/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/8664e157270f4373a3848b4778e32d7869e4cc26_8664e157270f4373a3848b4778e32d7869e4cc26/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.89% (19936/37690) |
| Line Coverage | 36.41% (186785/512996) |
| Region Coverage | 32.68% (144903/443386) |
| Branch Coverage | 33.87% (63522/187544) |
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61851:
URL: https://github.com/apache/doris/pull/61851#issuecomment-4149935535
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/61851_8664e157270f4373a3848b4778e32d7869e4cc26_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/61851_8664e157270f4373a3848b4778e32d7869e4cc26_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.46% (27117/36912) |
| Line Coverage | 56.97% (291367/511458) |
| Region Coverage | 54.29% (242955/447508) |
| Branch Coverage | 56.00% (105347/188110) |
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #61851:
URL: https://github.com/apache/doris/pull/61851#issuecomment-4149707772
/review
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon closed pull request #61851: [improvement](be) Use gperftools CPU profile for pprof flamegraph
URL: https://github.com/apache/doris/pull/61851
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #61851:
URL: https://github.com/apache/doris/pull/61851#issuecomment-4149756346
<details>
<summary>TPC-DS: <b>Total hot run time: 169050 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 8664e157270f4373a3848b4778e32d7869e4cc26, data reload: false
query5 4343 643 488 488
query6 339 223 201 201
query7 4225 459 259 259
query8 343 243 227 227
query9 8703 2685 2675 2675
query10 506 378 322 322
query11 7006 5111 4840 4840
query12 187 128 125 125
query13 1277 496 351 351
query14 5774 3707 3482 3482
query14_1 2879 2823 2830 2823
query15 202 192 176 176
query16 982 469 461 461
query17 1143 721 638 638
query18 2471 448 359 359
query19 224 206 204 204
query20 135 131 125 125
query21 214 132 113 113
query22 13191 14102 14484 14102
query23 17328 16496 16085 16085
query23_1 16644 15948 15761 15761
query24 7258 1599 1217 1217
query24_1 1229 1224 1241 1224
query25 539 457 402 402
query26 1242 263 155 155
query27 2767 481 293 293
query28 4496 1836 1830 1830
query29 865 557 488 488
query30 300 226 186 186
query31 1044 971 870 870
query32 84 69 70 69
query33 507 337 276 276
query34 895 868 525 525
query35 651 681 600 600
query36 1059 1120 968 968
query37 133 94 82 82
query38 2982 2956 2957 2956
query39 866 832 841 832
query39_1 798 793 784 784
query40 230 149 132 132
query41 61 58 57 57
query42 259 246 250 246
query43 239 245 215 215
query44
query45 201 192 187 187
query46 892 983 621 621
query47 2546 2596 2055 2055
query48 318 327 228 228
query49 665 451 371 371
query50 700 284 213 213
query51 4044 4068 4032 4032
query52 267 268 254 254
query53 281 329 285 285
query54 312 278 261 261
query55 89 90 88 88
query56 318 309 310 309
query57 1910 1863 1710 1710
query58 281 274 269 269
query59 2783 2948 2704 2704
query60 342 333 320 320
query61 164 150 155 150
query62 632 563 541 541
query63 310 271 274 271
query64 5200 1273 1021 1021
query65
query66 1473 453 353 353
query67 24219 24293 24181 24181
query68
query69 401 308 283 283
query70 986 978 961 961
query71 335 303 300 300
query72 2856 2836 2582 2582
query73 545 543 316 316
query74 9665 9568 9382 9382
query75 2890 2757 2497 2497
query76 2283 1052 673 673
query77 367 388 322 322
query78 10946 11067 10508 10508
query79 3108 796 607 607
query80 1748 601 522 522
query81 586 263 222 222
query82 995 154 129 129
query83 337 264 247 247
query84 261 118 94 94
query85 935 507 456 456
query86 565 322 298 298
query87 3275 3101 2982 2982
query88 3546 2626 2625 2625
query89 431 365 342 342
query90 2048 180 175 175
query91 166 162 136 136
query92 83 77 67 67
query93 1493 838 510 510
query94 658 298 289 289
query95 595 337 315 315
query96 645 516 226 226
query97 2446 2492 2395 2395
query98 242 220 218 218
query99 1001 1013 906 906
Total cold run time: 255552 ms
Total hot run time: 169050 ms
```
</details>
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "eldenmoon (via GitHub)" <gi...@apache.org>.
eldenmoon commented on PR #61851:
URL: https://github.com/apache/doris/pull/61851#issuecomment-4149702265
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "Thearas (via GitHub)" <gi...@apache.org>.
Thearas commented on PR #61851:
URL: https://github.com/apache/doris/pull/61851#issuecomment-4149702069
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).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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] [improvement](be) Use gperftools CPU profile for pprof flamegraph [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #61851:
URL: https://github.com/apache/doris/pull/61851#discussion_r3005900325
##########
be/src/util/pprof_utils.cpp:
##########
@@ -128,6 +128,30 @@ Status PprofUtils::get_readable_profile(const std::string& file_or_content, bool
return Status::OK();
}
+Status PprofUtils::get_svg_profile(const std::string& profile_file, std::string* output) {
+ std::string pprof_cmd;
+ RETURN_IF_ERROR(PprofUtils::get_pprof_cmd(&pprof_cmd));
Review Comment:
**Temp file leak on early error return**: If `get_pprof_cmd()` or `get_self_cmdline()` (line 136) returns an error, `RETURN_IF_ERROR` will exit the function without deleting `profile_file`. The caller (`ProfileAction::handle`) has already created this temp file via `ProfilerStart`/`ProfilerStop`.
I acknowledge this follows the same pattern as the existing `get_readable_profile()` above, but since this is new code, consider cleaning up the file on early exit, e.g.:
```cpp
auto cleanup = Defer([&profile_file]() {
static_cast<void>(io::global_local_filesystem()->delete_file(profile_file));
});
```
at the top of the function (and remove the explicit `delete_file` call at line 145).
##########
be/src/util/pprof_utils.cpp:
##########
@@ -128,6 +128,30 @@ Status PprofUtils::get_readable_profile(const std::string& file_or_content, bool
return Status::OK();
}
+Status PprofUtils::get_svg_profile(const std::string& profile_file, std::string* output) {
+ std::string pprof_cmd;
+ RETURN_IF_ERROR(PprofUtils::get_pprof_cmd(&pprof_cmd));
+
+ std::string self_cmdline;
+ RETURN_IF_ERROR(PprofUtils::get_self_cmdline(&self_cmdline));
+
+ std::string cmd_output;
+ std::string final_cmd =
+ pprof_cmd + absl::Substitute(" --svg $0 $1", self_cmdline, profile_file);
+ AgentUtils util;
+ LOG(INFO) << "begin to run command: " << final_cmd;
+ bool rc = util.exec_cmd(final_cmd, &cmd_output, false);
+
+ static_cast<void>(io::global_local_filesystem()->delete_file(profile_file));
+
+ if (!rc) {
+ return Status::InternalError("Failed to execute command: {}", cmd_output);
+ }
+
+ *output = std::move(cmd_output);
+ return Status::OK();
+}
+
Status PprofUtils::generate_flamegraph(int32_t sample_seconds,
Review Comment:
**Dead code**: `generate_flamegraph()` is no longer called anywhere after this PR removes its only call site in `pprof_actions.cpp`. Similarly, `get_perf_cmd()` (defined at line 57) is only used inside `generate_flamegraph()`. Both functions and their declarations in `pprof_utils.h` should be removed to keep the codebase clean.
##########
be/src/service/http/action/pprof_actions.cpp:
##########
@@ -157,52 +157,43 @@ void ProfileAction::handle(HttpRequest* req) {
}
const std::string& type_str = req->param("type");
- if (type_str != "flamegraph") {
- // use pprof the sample the CPU
- std::ostringstream tmp_prof_file_name;
- tmp_prof_file_name << config::pprof_profile_dir << "/doris_profile." << getpid() << "."
- << rand();
- ProfilerStart(tmp_prof_file_name.str().c_str());
- sleep(seconds);
- ProfilerStop();
-
- if (type_str != "text") {
- // return raw content via http response directly
- std::ifstream prof_file(tmp_prof_file_name.str().c_str(), std::ios::in);
- std::stringstream ss;
- if (!prof_file.is_open()) {
- ss << "Unable to open cpu profile: " << tmp_prof_file_name.str();
- std::string str = ss.str();
- HttpChannel::send_reply(req, str);
- return;
- }
- ss << prof_file.rdbuf();
- prof_file.close();
- std::string str = ss.str();
- HttpChannel::send_reply(req, str);
- return;
- }
+ std::ostringstream tmp_prof_file_name;
+ tmp_prof_file_name << config::pprof_profile_dir << "/doris_profile." << getpid() << "."
+ << rand();
+ ProfilerStart(tmp_prof_file_name.str().c_str());
+ sleep(seconds);
+ ProfilerStop();
- // text type. we will return readable content via http response
+ if (type_str == "text") {
std::stringstream readable_res;
Status st = PprofUtils::get_readable_profile(tmp_prof_file_name.str(), true, &readable_res);
if (!st.ok()) {
HttpChannel::send_reply(req, st.to_string());
} else {
HttpChannel::send_reply(req, readable_res.str());
}
- } else {
- // generate flamegraph
+ } else if (type_str == "flamegraph") {
std::string svg_file_content;
- std::string flamegraph_install_dir =
- std::string(std::getenv("DORIS_HOME")) + "/tools/FlameGraph/";
- Status st = PprofUtils::generate_flamegraph(seconds, flamegraph_install_dir, false,
- &svg_file_content);
+ Status st = PprofUtils::get_svg_profile(tmp_prof_file_name.str(), &svg_file_content);
if (!st.ok()) {
HttpChannel::send_reply(req, st.to_string());
} else {
HttpChannel::send_reply(req, svg_file_content);
}
+ } else {
+ std::ifstream prof_file(tmp_prof_file_name.str().c_str(), std::ios::in);
+ std::stringstream ss;
+ if (!prof_file.is_open()) {
+ ss << "Unable to open cpu profile: " << tmp_prof_file_name.str();
+ std::string str = ss.str();
+ HttpChannel::send_reply(req, str);
+ return;
Review Comment:
Minor: This early `return` exits without attempting to delete the temp file created at line 163. While the file likely doesn't exist if it can't be opened, it would be more robust to attempt cleanup here too:
```cpp
static_cast<void>(io::global_local_filesystem()->delete_file(tmp_prof_file_name.str()));
```
before returning.
--
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