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

Presto UI Stage Performance does not show operators which refer to plan nodes created by workers #24054

Open
LeonidChistov opened this issue Nov 15, 2024 · 3 comments

Comments

@LeonidChistov
Copy link

Presto UI Stage Performance tab does not show operators which refer to plan nodes which were created on worker side during plan rewriting and are not present in original plan.

Example of code that does plan rewrite on worker side:

sourceNodes[i] = std::make_shared<core::ProjectNode>(

Here the new plan node is created.

Your Environment

  • Presto version used: custom build based on October 11 Presto codebase snapshot
  • Data source and connector used: Hive

Expected Behavior

All operators present in latestAttemptExecutionInfo.stats.operatorSummaries section need to be visible in UI to allow performance analysis.

Current Behavior

Only operators that have planNodeId set to id of plan node present in the original query plan are shown in Stage Performance tab.

It can be seen that this is due to Presto UI implementation that considers for rendering only operators with plan nodes ids from the original plan:

let nodeOperators = operatorMap.get(planNode.id);

Possible Solution

Simplest solution would be to change Presto UI logic to use only operatorSummaries section for pipelines rendering and avoid referring to query pan.

In this case operator ids will be used to determine order of operators in the pipeline.

It is not clear though, why this approach was not initially implemented and whether this change may break anything,

Steps to Reproduce

Problem was reproduced when running TPCH Q1 with Hive connector.

Relevant fragment of execution JSON dump: https://pastebin.com/J9vZjGmk

Here FilterProject operator with plan node id 442.0 is present:

 {
          "stageId" : 0,
          "stageExecutionId" : 0,
          "pipelineId" : 1,
          "operatorId" : 1,
          "planNodeId" : "442.0",
          "operatorType" : "FilterProject",
...

while in the original plan there is only a node with id 442:

       "source" : {
            "@type" : "com.facebook.presto.sql.planner.plan.ExchangeNode",
            "sourceLocation" : {
              "line" : 19,
              "column" : 1
            },
            "id" : "442",
            "type" : "GATHER",
            "scope" : "LOCAL",

Screenshots (if appropriate)

Screenshot from 2024-11-15 13-43-59

@github-project-automation github-project-automation bot moved this to 🆕 Unprioritized in Bugs and support requests Nov 15, 2024
@tdcmeehan tdcmeehan moved this from 🆕 Unprioritized to 📋 Prioritized Backlog in Bugs and support requests Nov 15, 2024
@yhwang
Copy link
Member

yhwang commented Nov 15, 2024

@LeonidChistov the existing UI logic is to display the plan tree as a DAG graph. As you mentioned, showing the operators that are not in the original plan would help the analysis. I suggest you try to add those operator nodes to the graph and see what it looks like. I don't think it would break anything. FYI, the existing code does parse the whole list of the operatorSummaries here.

@LeonidChistov
Copy link
Author

@yhwang

I am not sure that I understand the statement about DAG graph. What I see in the Stage Performance tab is that it renders pipelines, which are just chains of operators.

Also AFAICS operator ids reflect their order in the chain. For example, when Velox creates operator chains it assigns operator ids in ascending order https://github.com/facebookincubator/velox/blob/c13b8ed888aa785bff88d061f5b3bc00e9231961/velox/exec/LocalPlanner.cpp#L428 and when initial chain is mutated, there is a special code to fix the order:
https://github.com/facebookincubator/velox/blob/c13b8ed888aa785bff88d061f5b3bc00e9231961/velox/exec/LocalPlanner.cpp#L657

Question which I have is: why it is not possible to avoid looking on Plan nodes and matching them against operators, but instead just to look on the operator ids and use them to create operator chains in the correct order in the UI.

Please let me know, if I am missing something here.

@yhwang
Copy link
Member

yhwang commented Nov 20, 2024

I believe the way it constructs the graph is based on the old data struct and the data struct changes and more data is added. If you like, you can update the graph and how it composes it and see if it can display all operators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Prioritized Backlog
Development

No branches or pull requests

3 participants