feat(ctl): add API to detect bottleneck actors #21149

KeXiangWang · 2025-03-28T02:06:58Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

Highest Release Version: 2.3

What's changed and what's your intention?

Add an API in risectl to using await tree to locate the bottleneck in streaming graphs. Currently, the API will first pull for a new await tree dump and analyze on it. Ideally, it should also be able to read from an existing diagnose file or await tree file. Will add this later.

Checklist

I have written necessary rustdoc comments.
I have added necessary unit tests and integration tests.
I have added test labels as necessary.
I have added fuzzing tests or opened an issue to track them.
My PR contains breaking changes.
My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

My PR needs documentation updates.

Release note

kwannoel

It will be better to decouple the API for fetching the await-tree json, and the API to detect bottleneck actors.

That's because our bottleneck-detecting strategy may not be perfect. So we may wish to fetch the await-tree json from the user cluster first.

Then we can iterate on the bottleneck detecting algorithm, and tweak the tool to suit the await-tree.

Besides that, great work!

xxchan · 2025-03-28T04:26:56Z

src/meta/src/dashboard/mod.rs

+            let result = client
+                .stack_trace(StackTraceRequest::default())
+                .await
+                .map_err(err)?;


We may also allow specifying format here in the meta dashboard.

And can do analyze there.

It will be better to decouple the API for fetching the await-tree json, and the API to detect bottleneck actors.
That's because our bottleneck-detecting strategy may not be perfect. So we may wish to fetch the await-tree json from the user cluster first.

@kwannoel read my mind. I choose to write this in risectl is for decoupling dumping and analyzing. But of course we can allow one to specify format through meta's API.

src/meta/src/manager/diagnose.rs

xxchan · 2025-03-28T04:36:06Z

Actually I prefer a js/web based tool to analyze the bottleneck. 🤔 That would be faster to iterate and demonstrate. But it's also fine to have one in risectl.

src/ctl/src/lib.rs

kwannoel · 2025-03-28T06:21:14Z

Tested with this SQL (release build):

create table t(id int, v1 int);

create table t2(id int, v1 int);

insert into t select id, 1 from generate_series(1, 10000) t(id);

insert into t2 select id, 1 from generate_series(1, 10000) t(id);

flush;

create materialized view mv as select t.id, count(t.v1) from t join t2 on t.v1 = t2.v1 group by t.id;

Works well for me:

>> Actor 129
Actor 129: `mv` [21.285s]  <== current
  Epoch 8251479171792896 [!!! 21.283s]
    Materialize 8100000007 [!!! 21.283s]
      Project 8100000006 [!!! 21.273s]
        HashAgg 8100000005 [!!! 21.273s]
          Merge 8100000004 [0.001s]
[Detached 3]
  LocalInput (actor 134) [0.001s]

>> Actor 130
Actor 130: `mv` [21.285s]  <== current
  Epoch 8251479171792896 [!!! 21.284s]
    Materialize 8200000007 [!!! 21.284s]
      Project 8200000006 [!!! 21.273s]
        HashAgg 8200000005 [!!! 21.273s]
          Merge 8200000004 [0.009s]
[Detached 3]
  LocalInput (actor 134) [0.009s]

>> Actor 131
Actor 131: `mv` [21.285s]  <== current
  Epoch 8251479171792896 [!!! 21.283s]
    Materialize 8300000007 [!!! 21.283s]
      Project 8300000006 [!!! 21.279s]
        HashAgg 8300000005 [!!! 21.279s]
          Merge 8300000004 [0.007s]
[Detached 3]
  LocalInput (actor 134) [0.007s]

>> Actor 132
Actor 132: `mv` [21.285s]
  Epoch 8251479171792896 [!!! 21.283s]
    Materialize 8400000007 [!!! 21.283s]
      Project 8400000006 [!!! 21.280s]
        HashAgg 8400000005 [!!! 21.280s]
          Merge 8400000004 [0.000s]  <== current

kwannoel · 2025-03-28T09:28:36Z

Ideally, it should also be able to read from an existing diagnose file or await tree file.

+1 for this, it takes a long time for users to upgrade.

xxchan

Thanks for the work! Mainly just nit picking.

What about we first get the changes to get JSON format merged (maybe can also cherry-picked). And refine the analyzer part later?

await-tree.txt

xxchan · 2025-03-31T03:05:33Z

src/compute/src/rpc/service/monitor_service.rs

+                        if req.actor_traces_format.is_some()
+                            && req.actor_traces_format.as_ref().unwrap() == "text"
+                        {
+                            v.to_string()
+                        } else {
+                            serde_json::to_string(&v).unwrap()
+                        },


The logic is different from the comment: "// Allowed values: "json", "text". Default is "text"."

Here it defaults to json.

Maybe better to define an enum instead of using string

this is not resolved

src/ctl/src/lib.rs

src/ctl/src/cmd_impl/await_tree/analyze.rs

src/ctl/src/cmd_impl/await_tree/dump.rs

src/ctl/src/cmd_impl/await_tree/utils.rs

BugenZhao · 2025-03-31T08:02:00Z

src/ctl/src/cmd_impl/await_tree/transcribe.rs

+use crate::cmd_impl::await_tree::tree::TreeView;
+use crate::cmd_impl::await_tree::utils::extract_actor_traces;
+
+pub fn transcribe(path: String) -> anyhow::Result<()> {


I'm thinking that this functionality can be provided by the library itself.

I'm OK with that.

BugenZhao · 2025-03-31T08:05:13Z

src/ctl/src/cmd_impl/await_tree/analyze.rs

+    /// the bottleneck actor is still yielding output to downstream actors. A typical
+    /// case is JOIN amplification. So the corresponding actors are actively processing
+    /// the data but the EPOCH span is blocked.
+    pub fn is_bottleneck(&self) -> bool {


Hmmm, the algorithm looks much easier than I thought initially. So there's no need to traverse between actors (based on the Input and Output spans) at all?

To be honest, I'm not entirely sure this algorithm works in every situation. However, in most cases where there's only one bottleneck actor, it should perform as expected. I can think of a few edge cases:

If the bottleneck's span doesn't call instrument_await, the bottleneck's tree might not exhibit the typical "slow parent with fast children" signal.

If an MV or the entire graph has multiple fragments acting as bottlenecks, I'm uncertain what the outcome would be.

At present, this tool doesn't completely eliminate manual effort. It definitely requires ongoing optimization and the addition of new rules.

2. I'm uncertain what the outcome would be.

Shall we output all of them, ordering by some sort of confidence or probablity?

Tend to leave it for future. At present, we don't have too much experience dealing with these edge cases. We can add some new rules when we can summarize some tips for them. For now, if the current bottleneck_detect return nothing, the on-caller should just use transcribe to convert the json await tree to text and mannually analyse the text await tree.

KeXiangWang · 2025-04-02T22:42:57Z

src/ctl/Cargo.toml

@@ -31,6 +31,7 @@ risingwave_pb = { workspace = true }
 risingwave_rpc_client = { workspace = true }
 risingwave_storage = { workspace = true }
 risingwave_stream = { workspace = true }
+rw-diagnose-tools = { git = "https://github.com/risingwavelabs/rw-diagnose-tools" }


Use github url for now. Need @BugenZhao 's help to publish the repo in crates.io.

xxchan

Others LGTM. Thanks for the work

await-tree.txt

xxchan · 2025-04-03T04:37:02Z

src/compute/src/rpc/service/monitor_service.rs

+                        if req.actor_traces_format.is_some()
+                            && req.actor_traces_format.as_ref().unwrap() == "text"
+                        {
+                            v.to_string()
+                        } else {
+                            serde_json::to_string(&v).unwrap()
+                        },


this is not resolved

xxchan · 2025-04-03T04:39:34Z

src/meta/src/dashboard/mod.rs

-            let result = client.stack_trace().await.map_err(err)?;
+            let result = client
+                .stack_trace(StackTraceRequest {
+                    actor_traces_format: Some("text".to_owned()),


Please add a parameter to the meta dashboard /await_tree call instead. Can default to text

Added. Example of a request:

curl "http://localhost:5691/api/monitor/await_tree/?format=text" > diagnose.txt

xxchan · 2025-04-03T04:41:11Z

src/meta/src/manager/diagnose.rs

-                && let Ok(result) = client.stack_trace().await
+                && let Ok(result) = client
+                    .stack_trace(StackTraceRequest {
+                        actor_traces_format: Some("json".to_owned()),


ditto, please add a parameter to the /diagnose API instead.

Example:

curl http://localhost:5691/api/monitor/diagnose/\?actor_traces_format\=json > diagnose.txt

kwannoel

After address @xxchan comments, rest LGTM. Thanks for the work!

xxchan

Generally LGTM, Thanks! Should we cherry-pick to earlier releases?

github-actions · 2025-04-07T01:17:25Z

Warning

The need-cherry-pick-release-xx label is deprecated. Please use need-cherry-pick-since-release-xx instead for future PRs.

BugenZhao

LGTM

KeXiangWang requested review from BugenZhao and xxchan March 28, 2025 02:07

github-actions bot added type/feature ci/run-e2e-single-node-tests ci/run-e2e-test-other-backends labels Mar 28, 2025

KeXiangWang force-pushed the wkx/await-tree-improve branch from c3bffc5 to c94b84a Compare March 28, 2025 02:08

kwannoel reviewed Mar 28, 2025

View reviewed changes

KeXiangWang force-pushed the wkx/await-tree-improve branch from c94b84a to e9c79a7 Compare March 28, 2025 02:53

feat(ctl): add API to detect bottleneck actors

648ac9f

KeXiangWang force-pushed the wkx/await-tree-improve branch from e9c79a7 to 648ac9f Compare March 28, 2025 02:55

kwannoel mentioned this pull request Mar 28, 2025

ANALYZE ACTOR #21150

Open

xxchan reviewed Mar 28, 2025

View reviewed changes

src/meta/src/manager/diagnose.rs Outdated Show resolved Hide resolved

xxchan reviewed Mar 28, 2025

View reviewed changes

src/ctl/src/lib.rs Outdated Show resolved Hide resolved

KeXiangWang added 6 commits March 28, 2025 14:53

done

7c46f54

support parse from file

e7e9a7a

tmp

9e54606

set default to json

80d24c9

add support for parsing text format

b98634a

tmp

72e50ae

KeXiangWang marked this pull request as ready for review March 29, 2025 17:38

graphite-app bot requested a review from a team March 29, 2025 17:56

xxchan reviewed Mar 31, 2025

View reviewed changes

BugenZhao reviewed Mar 31, 2025

View reviewed changes

refactor

06af538

KeXiangWang requested a review from a team as a code owner April 2, 2025 22:33

KeXiangWang requested a review from cyliu0 April 2, 2025 22:33

small fix

dbc8a0a

KeXiangWang commented Apr 2, 2025

View reviewed changes

xxchan reviewed Apr 3, 2025

View reviewed changes

kwannoel approved these changes Apr 3, 2025

View reviewed changes

KeXiangWang force-pushed the wkx/await-tree-improve branch from ca3a490 to cb52394 Compare April 4, 2025 00:11

update meta dashboard await tree & diagnose

7b581ad

KeXiangWang force-pushed the wkx/await-tree-improve branch from cb52394 to 7b581ad Compare April 4, 2025 00:12

KeXiangWang added 2 commits April 3, 2025 20:16

tmp

0bf3a54

tmp

0d0f0d2

xxchan approved these changes Apr 4, 2025

View reviewed changes

KeXiangWang added the need-cherry-pick-release-2.3 [⚠️DEPRECATED] Use `..-since-release-..` instead label Apr 7, 2025

fix rw-diagnose-tools version

ffa7fe1

KeXiangWang added the need-cherry-pick-since-release-2.2 label Apr 7, 2025

KeXiangWang enabled auto-merge April 7, 2025 02:09

BugenZhao approved these changes Apr 7, 2025

View reviewed changes

Merge branch 'main' into wkx/await-tree-improve

4a833ea

KeXiangWang added this pull request to the merge queue Apr 8, 2025

Merged via the queue into main with commit f09d594 Apr 8, 2025
33 of 34 checks passed

KeXiangWang deleted the wkx/await-tree-improve branch April 8, 2025 04:06

This was referenced Apr 8, 2025

cherry-pick feat(ctl): add API to detect bottleneck actors (#21149) to branch release-2.3 #21271

Closed

cherry-pick feat(ctl): add API to detect bottleneck actors (#21149) to branch release-2.2 #21272

Closed

KeXiangWang added a commit that referenced this pull request Apr 8, 2025

feat(ctl): add API to detect bottleneck actors (#21149)

7069fbf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ctl): add API to detect bottleneck actors #21149

feat(ctl): add API to detect bottleneck actors #21149

KeXiangWang commented Mar 28, 2025

kwannoel left a comment

xxchan Mar 28, 2025 •

edited

Loading

KeXiangWang Mar 28, 2025

xxchan commented Mar 28, 2025

kwannoel commented Mar 28, 2025 •

edited

Loading

kwannoel commented Mar 28, 2025

xxchan left a comment

xxchan Mar 31, 2025

xxchan Apr 3, 2025

KeXiangWang Apr 3, 2025

BugenZhao Mar 31, 2025

KeXiangWang Apr 1, 2025

BugenZhao Mar 31, 2025

KeXiangWang Apr 1, 2025

BugenZhao Apr 1, 2025

KeXiangWang Apr 2, 2025

KeXiangWang Apr 2, 2025 •

edited

Loading

xxchan left a comment

xxchan Apr 3, 2025

xxchan Apr 3, 2025

KeXiangWang Apr 3, 2025

xxchan Apr 3, 2025

KeXiangWang Apr 4, 2025

kwannoel left a comment •

edited

Loading

xxchan left a comment •

edited

Loading

github-actions bot commented Apr 7, 2025

BugenZhao left a comment

feat(ctl): add API to detect bottleneck actors #21149

feat(ctl): add API to detect bottleneck actors #21149

Conversation

KeXiangWang commented Mar 28, 2025

What's changed and what's your intention?

Checklist

Documentation

kwannoel left a comment

Choose a reason for hiding this comment

xxchan Mar 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xxchan commented Mar 28, 2025

kwannoel commented Mar 28, 2025 • edited Loading

kwannoel commented Mar 28, 2025

xxchan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KeXiangWang Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

xxchan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwannoel left a comment • edited Loading

Choose a reason for hiding this comment

xxchan left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Apr 7, 2025

BugenZhao left a comment

Choose a reason for hiding this comment

xxchan Mar 28, 2025 •

edited

Loading

kwannoel commented Mar 28, 2025 •

edited

Loading

KeXiangWang Apr 2, 2025 •

edited

Loading

kwannoel left a comment •

edited

Loading

xxchan left a comment •

edited

Loading