[Enhancement] merge fragment-profile just in time #51001

murphyatwork · 2024-09-12T11:29:25Z

Why I'm doing:

Currently the query profile is generated in a hierarchy way:

Each backend would report the profiles in fragment instance level
FE would maintain all these profiles in memory before query finish
As a result, with more fragments, more backends, this running profile would take more memory
For a large-size cluster, this query profile can easily take hundreds MBs of FE memory

The tradeoff:

If we want to provide running profile: you have to maintain all instance profiles for running instances
If we don't need it, we only need to maintain the fragment-level, then consolidate them into a query profile when finish

What I'm doing:

Merge the instance-level profile into fragment-level after it finishes:

The correctness is based on that the counters would never change after it finishes
It can greatly reduce memory consumption for a large-size cluster

Fixes #issue

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
This is a backport pr

Bugfix cherry-pick branch check:

before-Sunrise · 2024-09-14T05:55:11Z

fe/fe-core/src/main/java/com/starrocks/qe/scheduler/QueryRuntimeProfile.java

+ if (params.isSetProfile() && params.isDone()) {
+ int fragmentIndex = execState.getFragmentIndex();
+ RuntimeProfile fragmentProfile = mergedFragmentProfiles.get(fragmentIndex);
+ fragmentProfile.merge(params.profile);


is this thread-safe?

…tion Signed-off-by: Murphy <[email protected]>

Signed-off-by: Murphy <[email protected]>

before-Sunrise · 2024-10-18T06:06:00Z

fe/fe-core/src/main/java/com/starrocks/common/util/RuntimeProfile.java

+ }
+ }
+
+ // Remove counters from this fragment-profile


so after merge one instance profile into fragment profile, you actually merged counter and remove counters from instance profile, leave others same as before, right?

yes. add a purgeInstanceRunningProfile to clarify this process

before-Sunrise · 2024-10-18T06:10:03Z

fe/fe-core/src/main/java/com/starrocks/qe/scheduler/QueryRuntimeProfile.java

@@ -337,6 +344,15 @@ public void updateProfile(FragmentInstanceExecState execState, TReportExecStatus
 saveRunningProfile(profilingPlan, profile);
 LOG.debug("update profile, profilingPlan: {}, profile: {}", profilingPlan, profile);
 }
+
+ // Merge it into fragment-profile if this instance already finished
+ if (params.isSetProfile() && params.isDone()) {


so without consider runtime profile uploading, before this pr, every fragment instance profile will keep until merged by profile-worker thread. after this pr, fragment instance profile's counters will merged into fragment immediately in here. If some instance profile upload very slow or profile-worker is super busy, then your pr will help to reduce memory?

change the strategy to handle this case: once a fragment-instance finish, purse that profile and merge it into fragment-profile, it can reduce memory

before-Sunrise · 2024-10-18T06:24:12Z

suggest to add a fe config, so if anything goes wong, we can set it as false

Signed-off-by: Murphy <[email protected]>

sonarcloud · 2024-10-18T11:04:54Z

Quality Gate failed

Failed conditions
4.7% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

github-actions · 2024-10-18T12:48:42Z

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2024-10-18T12:49:18Z

[FE Incremental Coverage Report]

✅ pass : 219 / 259 (84.56%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	com/starrocks/qe/StmtExecutor.java	0	6	00.00%	[951, 952, 953, 954, 955, 956]
🔵	com/starrocks/qe/scheduler/dag/FragmentInstanceExecState.java	3	12	25.00%	[319, 320, 321, 322, 323, 325, 326, 327, 331]
🔵	com/starrocks/qe/scheduler/QueryRuntimeProfile.java	6	18	33.33%	[318, 319, 320, 321, 322, 323, 435, 436, 442, 443, 444, 445]
🔵	com/starrocks/qe/DefaultCoordinator.java	3	4	75.00%	[1040]
🔵	com/starrocks/common/profile/Counter.java	53	60	88.33%	[69, 70, 73, 74, 92, 93, 192]
🔵	com/starrocks/common/util/RuntimeProfile.java	127	132	96.21%	[149, 157, 161, 316, 364]
🔵	com/starrocks/common/Config.java	1	1	100.00%	[]
🔵	com/starrocks/common/profile/SummarizationCounter.java	26	26	100.00%	[]

github-actions · 2024-10-18T12:49:29Z

[BE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

murphyatwork requested a review from a team as a code owner September 12, 2024 11:29

mergify bot assigned murphyatwork Sep 12, 2024

github-actions bot added the 3.3 label Sep 13, 2024

murphyatwork force-pushed the murphy_opt_fragment_profile branch 2 times, most recently from d4789ca to 443638e Compare September 14, 2024 05:31

murphyatwork changed the title ~~[Enhancement] reduce runtime profile memory via merging fragment profile~~ [Enhancement] merge fragment-profile just in time Sep 14, 2024

before-Sunrise reviewed Sep 14, 2024

View reviewed changes

murphyatwork force-pushed the murphy_opt_fragment_profile branch 5 times, most recently from 91583b0 to 5feb28a Compare September 25, 2024 04:16

murphyatwork added 8 commits October 17, 2024 13:43

merge instance profile into fragment profile to reduce memory consump…

dcdd923

…tion Signed-off-by: Murphy <[email protected]>

debug log

64ec9dc

Signed-off-by: Murphy <[email protected]>

recursive copy

11d80f8

Signed-off-by: Murphy <[email protected]>

deal with BE merged MIN/MAX

f46408a

Signed-off-by: Murphy <[email protected]>

fix

b27e554

Signed-off-by: Murphy <[email protected]>

add test

bb29af7

Signed-off-by: Murphy <[email protected]>

refactor

75b9187

Signed-off-by: Murphy <[email protected]>

clear counters in thrift after merged

6c3d567

Signed-off-by: Murphy <[email protected]>

murphyatwork force-pushed the murphy_opt_fragment_profile branch from 5feb28a to 6c3d567 Compare October 17, 2024 05:43

murphyatwork added 2 commits October 17, 2024 19:29

fix MIN/MAX

91ee0bc

Signed-off-by: Murphy <[email protected]>

fix thread-safety

689a835

Signed-off-by: Murphy <[email protected]>

murphyatwork enabled auto-merge (squash) October 18, 2024 02:05

Seaven previously approved these changes Oct 18, 2024

View reviewed changes

before-Sunrise reviewed Oct 18, 2024

View reviewed changes

fix

5997d8d

Signed-off-by: Murphy <[email protected]>

murphyatwork dismissed Seaven’s stale review via 5997d8d October 18, 2024 09:37

sql test

5dafb8b

Signed-off-by: Murphy <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] merge fragment-profile just in time #51001

[Enhancement] merge fragment-profile just in time #51001

murphyatwork commented Sep 12, 2024 •

edited

Loading

before-Sunrise Sep 14, 2024

murphyatwork Oct 17, 2024

before-Sunrise Oct 18, 2024

murphyatwork Oct 18, 2024

before-Sunrise Oct 18, 2024

murphyatwork Oct 18, 2024

before-Sunrise commented Oct 18, 2024

sonarcloud bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

github-actions bot commented Oct 18, 2024

[Enhancement] merge fragment-profile just in time #51001

Are you sure you want to change the base?

[Enhancement] merge fragment-profile just in time #51001

Conversation

murphyatwork commented Sep 12, 2024 • edited Loading

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

before-Sunrise Sep 14, 2024

Choose a reason for hiding this comment

murphyatwork Oct 17, 2024

Choose a reason for hiding this comment

before-Sunrise Oct 18, 2024

Choose a reason for hiding this comment

murphyatwork Oct 18, 2024

Choose a reason for hiding this comment

before-Sunrise Oct 18, 2024

Choose a reason for hiding this comment

murphyatwork Oct 18, 2024

Choose a reason for hiding this comment

before-Sunrise commented Oct 18, 2024

sonarcloud bot commented Oct 18, 2024

Quality Gate failed

github-actions bot commented Oct 18, 2024

[Java-Extensions Incremental Coverage Report]

github-actions bot commented Oct 18, 2024

[FE Incremental Coverage Report]

file detail

github-actions bot commented Oct 18, 2024

[BE Incremental Coverage Report]

murphyatwork commented Sep 12, 2024 •

edited

Loading