Skip to content

Conversation

@abstractdog
Copy link
Contributor

@abstractdog abstractdog commented Jun 5, 2024

What changes were proposed in this pull request?

Added HTML format support to ProfileServlet.

Why are the changes needed?

Because HTML is the default format for creating flamegraphs.

Does this PR introduce any user-facing change?

No, profile servlet is more like a dev tool.

Is the change a dependency upgrade?

No.

How was this patch tested?

set web port for hs2 in hive-site.xml

+<property>
+  <name>hive.server2.webui.port</name>
+  <value>10003</value>
+</property>
#tried with both 2.9 and 3.0
export ASYNC_PROFILER_HOME=/Users/laszlobodor/Downloads/async-profiler-2.9-macos
export ASYNC_PROFILER_HOME=/Users/laszlobodor/Downloads/async-profiler-3.0-macos

 mvn install -Dtest=StartMiniHS2Cluster -DminiHS2.clusterType=llap -DminiHS2.conf="target/testconf/llap/hive-site.xml"  -DminiHS2.run=true -DminiHS2.usePortsFromConf=true -Dpackaging.minimizeJar=false -T 1C -DskipShade -Dremoteresources.skip=true -Dmaven.javadoc.skip=true -Denforcer.skip=true -pl itests/hive-unit -Pitests -nsu

called localhost:10003/prof and get the flamegraph successfully
https://issues.apache.org/jira/secure/attachment/13069348/async-prof-pid-21327-cpu-2.html

@abstractdog abstractdog changed the title HIVE-28305: ProfileServlet: add html to output formats HIVE-28305: ProfileServlet: add html to output formats and prepare for profiler 3.0 Jun 6, 2024
@abstractdog abstractdog requested a review from ayushtkn June 6, 2024 09:06
Copy link
Member

@deniskuzZ deniskuzZ left a comment

Choose a reason for hiding this comment

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

LGTM, minor comment

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 6, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

LGTM

return Output.valueOf(outputArg.trim().toUpperCase());
} catch (IllegalArgumentException e) {
return Output.SVG;
LOG.warn("Output format value is invalid, returning with default HTML");
Copy link
Member

Choose a reason for hiding this comment

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

Can you add what value is actually configured in the log as well

        LOG.warn("Output format value {} is invalid, returning with default HTML", outputArg);

Copy link
Contributor Author

@abstractdog abstractdog Jun 6, 2024

Choose a reason for hiding this comment

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

makes sense!
took care in 681036c

@abstractdog
Copy link
Contributor Author

@abstractdog abstractdog merged commit 521c5fd into apache:master Jun 6, 2024
asfgit pushed a commit that referenced this pull request Sep 23, 2024
…r profiler 3.0 (#5281) (Laszlo Bodor reviewed by Denys Kuzmenko, Ayush Saxena, Kokila N)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants