Skip to content

TEZ-4740: Fix typo in ProfileServlet javadoc for method profiling#525

Merged
abstractdog merged 1 commit into
apache:masterfrom
Aggarwal-Raghav:TEZ-4740
Jul 20, 2026
Merged

TEZ-4740: Fix typo in ProfileServlet javadoc for method profiling#525
abstractdog merged 1 commit into
apache:masterfrom
Aggarwal-Raghav:TEZ-4740

Conversation

@Aggarwal-Raghav

@Aggarwal-Raghav Aggarwal-Raghav commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Method profiling is still supported in newer versions of async-profiler (including 3.x and 4.x),
but it is accessed via the '-e' flag instead of '-m' in the backend. Admins can continue to use
the '?method=' HTTP query parameter for method profiling. This commit simply updates the Javadoc
to reflect the correct async-profiler flag, while leaving the existing method profiling functionality
intact.

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 2m 22s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 3m 2s master passed
+1 💚 compile 2m 35s master passed
+1 💚 checkstyle 0m 21s master passed
+1 💚 javadoc 0m 19s master passed
+0 🆗 spotbugs 0m 35s tez-common in master has 13 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚 mvninstall 2m 11s the patch passed
+1 💚 codespell 0m 48s No new issues.
+1 💚 compile 2m 34s the patch passed
+1 💚 javac 2m 34s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 16s the patch passed
+1 💚 javadoc 0m 18s the patch passed
+1 💚 spotbugs 0m 39s the patch passed
_ Other Tests _
-1 ❌ unit 60m 4s /patch-unit-root.txt root in the patch passed.
+1 💚 asflicense 0m 20s The patch does not generate ASF License warnings.
77m 28s
Reason Tests
Failed junit tests tez.history.TestHistoryParser
Subsystem Report/Notes
Docker ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-525/1/artifact/out/Dockerfile
Optional Tests dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
uname Linux 33b347b5c041 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality tez-personality.sh
git revision master / b9d8c0f
Default Java Eclipse Adoptium-21.0.11+10-LTS
Test Results https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-525/1/testReport/
Max. process+thread count 1565 (vs. ulimit of 5500)
modules C: tez-common U: tez-common
Console output https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-525/1/console
versions git=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered by Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@Aggarwal-Raghav

Copy link
Copy Markdown
Contributor Author

@abstractdog , I verified that the ?method= parameter was maping to the -e flag and successfully generates FlameGraphs for that method in the async-profiler 4.4 i.e. latest

As per async-profiler readme:
https://github.com/async-profiler/async-profiler/blob/master/docs/ProfilingModes.md#java-method-profiling

-e ClassName.methodName option instruments the given Java method in order to record all invocations of this method with the stack traces.

I have a question, do we want to stop this method level profiling if yes then this PR is correct otherwise,
Do you want to keep the feature, cancel TEZ-4740, and just fix the comment typo from -m to -e flag?

BEFORE:
Screenshot 2026-07-19 at 3 27 53 PM

Screenshot 2026-07-19 at 3 28 13 PM

AFTER:
Screenshot 2026-07-19 at 3 58 00 PM

Screenshot 2026-07-19 at 4 00 16 PM

@abstractdog

Copy link
Copy Markdown
Contributor

fix the comment typo from -m to -e flag

thanks @Aggarwal-Raghav for investigating this: I was worried that maybe async profiler dropped the support of profiling methods, however, if it works with the -e, we can simply fix the typo definitely! the goal is to be very clear about the fact that in newer async profiler versions, method profiling is possible via the -e option
feel free to rename Jira accordingly

@Aggarwal-Raghav

Copy link
Copy Markdown
Contributor Author

fix the comment typo from -m to -e flag

thanks @Aggarwal-Raghav for investigating this: I was worried that maybe async profiler dropped the support of profiling methods, however, if it works with the -e, we can simply fix the typo definitely! the goal is to be very clear about the fact that in newer async profiler versions, method profiling is possible via the -e option feel free to rename Jira accordingly

i think older also it was -e since 2.0 https://github.com/async-profiler/async-profiler/blob/v2.0/README.md#java-method-profiling

So, now I'll rename this ticket to update the javadoc only and update the path to only support for 4.4 and drop support for 3.x?

/**
* Get the path of the profiler script to be executed.
* Before async-profiler 3.0, the script was named profiler.sh, and after 3.0 it's bin/asprof
* @return
*/
private String getProfilerScriptPath() {
Path defaultPath = Paths.get(asyncProfilerHome + "/bin/asprof");
return Files.exists(defaultPath)? defaultPath.toString() : asyncProfilerHome + "/profiler.sh";
}

@abstractdog

Copy link
Copy Markdown
Contributor

fix the comment typo from -m to -e flag

thanks @Aggarwal-Raghav for investigating this: I was worried that maybe async profiler dropped the support of profiling methods, however, if it works with the -e, we can simply fix the typo definitely! the goal is to be very clear about the fact that in newer async profiler versions, method profiling is possible via the -e option feel free to rename Jira accordingly

i think older also it was -e since 2.0 https://github.com/async-profiler/async-profiler/blob/v2.0/README.md#java-method-profiling

So, now I'll rename this ticket to update the javadoc only and update the path to only support for 4.4 and drop support for 3.x?

/**
* Get the path of the profiler script to be executed.
* Before async-profiler 3.0, the script was named profiler.sh, and after 3.0 it's bin/asprof
* @return
*/
private String getProfilerScriptPath() {
Path defaultPath = Paths.get(asyncProfilerHome + "/bin/asprof");
return Files.exists(defaultPath)? defaultPath.toString() : asyncProfilerHome + "/profiler.sh";
}

I think dropping profile 3.x support is another thing to discuss, I'm fine with not doing it now

@Aggarwal-Raghav

Copy link
Copy Markdown
Contributor Author

fix the comment typo from -m to -e flag

thanks @Aggarwal-Raghav for investigating this: I was worried that maybe async profiler dropped the support of profiling methods, however, if it works with the -e, we can simply fix the typo definitely! the goal is to be very clear about the fact that in newer async profiler versions, method profiling is possible via the -e option feel free to rename Jira accordingly

i think older also it was -e since 2.0 https://github.com/async-profiler/async-profiler/blob/v2.0/README.md#java-method-profiling
So, now I'll rename this ticket to update the javadoc only and update the path to only support for 4.4 and drop support for 3.x?

/**
* Get the path of the profiler script to be executed.
* Before async-profiler 3.0, the script was named profiler.sh, and after 3.0 it's bin/asprof
* @return
*/
private String getProfilerScriptPath() {
Path defaultPath = Paths.get(asyncProfilerHome + "/bin/asprof");
return Files.exists(defaultPath)? defaultPath.toString() : asyncProfilerHome + "/profiler.sh";
}

I think dropping profile 3.x support is another thing to discuss, I'm fine with not doing it now

ACK

Method profiling is still supported in newer versions of async-profiler (including 3.x and 4.x),
but it is accessed via the '-e' flag instead of '-m' in the backend. Admins can continue to use
the '?method=' HTTP query parameter for method profiling. This commit simply updates the Javadoc
to reflect the correct async-profiler flag, while leaving the existing method profiling functionality
intact.
@Aggarwal-Raghav

Copy link
Copy Markdown
Contributor Author

So final behaviour:
http://localhost:50000/prof?event=cpu&duration=10

FOR CPU:
Screenshot 2026-07-20 at 4 20 10 PM
Screenshot 2026-07-20 at 4 20 46 PM

FOR METHOD:
Screenshot 2026-07-20 at 4 24 03 PM
Screenshot 2026-07-20 at 4 24 14 PM

@Aggarwal-Raghav Aggarwal-Raghav changed the title TEZ-4740: Remove method parameter from ProfileServlet TEZ-4740: Fix typo in ProfileServlet javadoc for method profiling Jul 20, 2026
@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 7s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 3m 6s master passed
+1 💚 compile 2m 29s master passed
+1 💚 checkstyle 0m 21s master passed
+1 💚 javadoc 0m 19s master passed
+0 🆗 spotbugs 0m 35s tez-common in master has 13 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚 mvninstall 2m 10s the patch passed
+1 💚 codespell 0m 50s No new issues.
+1 💚 compile 2m 29s the patch passed
+1 💚 javac 2m 29s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 17s the patch passed
+1 💚 javadoc 0m 19s the patch passed
+1 💚 spotbugs 0m 40s the patch passed
_ Other Tests _
+1 💚 unit 60m 23s root in the patch passed.
+1 💚 asflicense 0m 22s The patch does not generate ASF License warnings.
75m 28s
Subsystem Report/Notes
Docker ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-525/2/artifact/out/Dockerfile
Optional Tests dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
uname Linux b09575a92cc4 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality tez-personality.sh
git revision master / b9d8c0f
Default Java Eclipse Adoptium-21.0.11+10-LTS
Test Results https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-525/2/testReport/
Max. process+thread count 1497 (vs. ulimit of 5500)
modules C: tez-common U: tez-common
Console output https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-525/2/console
versions git=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered by Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@abstractdog
abstractdog self-requested a review July 20, 2026 15:28
@abstractdog
abstractdog merged commit 0f389d2 into apache:master Jul 20, 2026
6 checks passed
@abstractdog

Copy link
Copy Markdown
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants