You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "mnpoonia (via GitHub)" <gi...@apache.org> on 2026/01/25 19:54:02 UTC

[PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

mnpoonia opened a new pull request, #7679:
URL: https://github.com/apache/hbase/pull/7679

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-4794797110

   Merging the PR


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-3797243242

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 26s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 21s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 24s |  |  master passed  |
   | +1 :green_heart: |  compile  |   8m 43s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   2m 21s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |   8m 14s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   0m 52s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 15s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   8m 47s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   8m 47s |  |  root generated 0 new + 1886 unchanged - 2 fixed = 1886 total (was 1888)  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   2m 16s | [/results-checkstyle-root.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/1/artifact/yetus-general-check/output/results-checkstyle-root.txt) |  root: The patch generated 1 new + 0 unchanged - 1 fixed = 1 total (was 1)  |
   | +1 :green_heart: |  xmllint  |   0m  0s |  |  No new issues.  |
   | +1 :green_heart: |  spotbugs  |   8m 25s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  12m 21s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  spotless  |   0m 47s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 29s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  69m  6s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7679 |
   | Optional Tests | dupname asflicense javac codespell detsecrets xmllint hadoopcheck spotless compile spotbugs checkstyle hbaseanti |
   | uname | Linux 0b15cae4fdd6 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 91ff53461061406d02b085c92b13749f39769594 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 189 (vs. ulimit of 30000) |
   | modules | C: hbase-resource-bundle hbase-http . U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/1/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on code in PR #7679:
URL: https://github.com/apache/hbase/pull/7679#discussion_r3394255957


##########
hbase-http/src/main/java/org/apache/hadoop/hbase/http/ProfileServlet.java:
##########
@@ -189,109 +265,149 @@ protected void doGet(final HttpServletRequest req, final HttpServletResponse res
     final Integer height = getInteger(req, "height", null);
     final Double minwidth = getMinWidth(req);
     final boolean reverse = req.getParameterMap().containsKey("reverse");
+    int refreshDelay = getInteger(req, "refreshDelay", 0);
+
+    return new ProfileRequest(duration, output, event, interval, jstackDepth, bufsize, thread,
+      simple, width, height, minwidth, reverse, refreshDelay, requestedPid);
+  }
+
+  protected String executeStart(ProfileRequest request, File outputFile) throws IOException {
+    return backend.executeStart(request, outputFile);
+  }
+
+  protected String executeStop(ProfileRequest request, File outputFile) throws IOException {
+    return backend.executeStop(request, outputFile);
+  }
+
+  @Override
+  protected void doGet(final HttpServletRequest req, final HttpServletResponse resp)
+    throws IOException {
+    if (!checkInstrumentationAccess(req, resp)) {
+      return;
+    }
+
+    final ProfileRequest request = parseProfileRequest(req);
+
+    // We keep the pid parameter for backward compatibility but only support profiling this JVM.
+    if (request.getPid() != null && request.getPid().longValue() != currentPid) {
+      writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+        "The 'pid' parameter is only supported for the current process when using the "
+          + "embedded async-profiler library.");
+      return;
+    }
+
+    if (profiling) {
+      writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+        "Another instance of profiler is already running.");
+      return;
+    }
+
+    int lockTimeoutSecs = 3;
+    boolean locked = false;
+    try {
+      locked = profilerLock.tryLock(lockTimeoutSecs, TimeUnit.SECONDS);
+      if (!locked) {
+        writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+          "Unable to acquire lock. Another instance of profiler might be running.");
+        LOG.warn("Unable to acquire lock in " + lockTimeoutSecs
+          + " seconds. Another instance of profiler might be running.");
+        return;
+      }
+
+      File outputFile = createOutputFile(request);
+      // Ensure the file exists so ProfileOutputServlet can poll until it is complete.
+      Files.write(outputFile.toPath(), new byte[0]);
 
-    if (process == null || !process.isAlive()) {
+      executeStart(request, outputFile);
+      profiling = true;
+
+      startStopperThread(request.getDuration(), request, outputFile);
+
+      writeAcceptedResponse(resp, request, outputFile);
+    } catch (InterruptedException e) {

Review Comment:
   ```
   } catch (InterruptedException e) {
       Thread.currentThread().interrupt();     // handled first, specifically
       ...
   } catch (IOException | Error | RuntimeException e) {
       // Catches:
       // - IOException: AsyncProfiler.execute() throws IOException for invalid agent commands
       // - UnsatisfiedLinkError / other Error: native lib absent or incompatible OS/kernel
       // - IllegalStateException / IllegalArgumentException (RuntimeException): double-start,
       //   unsupported event, rejected format from the profiler API
       LOG.warn("Profiler failed to start or execute", e);
       writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
           "Profiler error: " + e.getMessage()
             + ". Check that the async-profiler native library is compatible with this OS/kernel.");
   ```
   This should cover every scenario. 
   
   ```
   - UnsatisfiedLinkError → caught by Error
   - glibc missing versioned symbols → UnsatisfiedLinkError or Error → caught
   - kernel disabled perf_event → throws at first execute() call → caught by IOException | RuntimeException
   - seccomp policy → same
   - InterruptedException is handled first and explicitly re-interrupts the thread — it never reaches the broader catch
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-4776484605

   This test(org.apache.hadoop.hbase.util.TestProcDispatcher) runs fine locally and is unrelated to my change. 
   @virajjasani FYI


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "apurtell (via GitHub)" <gi...@apache.org>.
apurtell commented on code in PR #7679:
URL: https://github.com/apache/hbase/pull/7679#discussion_r3277718928


##########
hbase-http/src/main/java/org/apache/hadoop/hbase/http/ProfileServlet.java:
##########
@@ -189,109 +265,149 @@ protected void doGet(final HttpServletRequest req, final HttpServletResponse res
     final Integer height = getInteger(req, "height", null);
     final Double minwidth = getMinWidth(req);
     final boolean reverse = req.getParameterMap().containsKey("reverse");
+    int refreshDelay = getInteger(req, "refreshDelay", 0);
+
+    return new ProfileRequest(duration, output, event, interval, jstackDepth, bufsize, thread,
+      simple, width, height, minwidth, reverse, refreshDelay, requestedPid);
+  }
+
+  protected String executeStart(ProfileRequest request, File outputFile) throws IOException {
+    return backend.executeStart(request, outputFile);
+  }
+
+  protected String executeStop(ProfileRequest request, File outputFile) throws IOException {
+    return backend.executeStop(request, outputFile);
+  }
+
+  @Override
+  protected void doGet(final HttpServletRequest req, final HttpServletResponse resp)
+    throws IOException {
+    if (!checkInstrumentationAccess(req, resp)) {
+      return;
+    }
+
+    final ProfileRequest request = parseProfileRequest(req);
+
+    // We keep the pid parameter for backward compatibility but only support profiling this JVM.
+    if (request.getPid() != null && request.getPid().longValue() != currentPid) {
+      writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+        "The 'pid' parameter is only supported for the current process when using the "
+          + "embedded async-profiler library.");
+      return;
+    }
+
+    if (profiling) {
+      writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+        "Another instance of profiler is already running.");
+      return;
+    }
+
+    int lockTimeoutSecs = 3;
+    boolean locked = false;
+    try {
+      locked = profilerLock.tryLock(lockTimeoutSecs, TimeUnit.SECONDS);
+      if (!locked) {
+        writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+          "Unable to acquire lock. Another instance of profiler might be running.");
+        LOG.warn("Unable to acquire lock in " + lockTimeoutSecs
+          + " seconds. Another instance of profiler might be running.");
+        return;
+      }
+
+      File outputFile = createOutputFile(request);
+      // Ensure the file exists so ProfileOutputServlet can poll until it is complete.
+      Files.write(outputFile.toPath(), new byte[0]);
 
-    if (process == null || !process.isAlive()) {
+      executeStart(request, outputFile);
+      profiling = true;
+
+      startStopperThread(request.getDuration(), request, outputFile);
+
+      writeAcceptedResponse(resp, request, outputFile);
+    } catch (InterruptedException e) {

Review Comment:
   The native library won't actually be loaded until first use, which will happen in `doGet`, and there could be other native layer issues such as a glibc missing some versioned symbols, kernel disabled perf_event support, a seccomp policy, etc. Catching `Error` here, or even `Throwable` and then testing for  `instanceof InterruptedException` to deal with that specific case would handle all of this. Otherwise `UnsatisfiedLinkError ` (most likely) will bubble up to the servlet container which will emit a generic 500 error response. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-4376915288

   @apurtell FYI. Hopefully i have addressed the concerns.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-4417763536

   @virajjasani Please review


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani merged PR #7679:
URL: https://github.com/apache/hbase/pull/7679


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-4366010225

   > Async profiler is an optional dependency. The availability of a profiler on the server side can be a security risk and so some (or many) may opt not to place one there. This change does not gracefully handle the absence of the profiler nor make its deployment optional.
   
    Good point. This PR addresses both concerns: the dependency is now marked <optional>true</optional> so it is never transitively pulled into downstream modules. Users who want to bundle it can activate
      the async-profiler Maven profile (-Pasync-profiler). Absence is handled gracefully — HttpServer checks ProfileServlet.isAvailable() at startup and falls back to DisabledServlet (returning HTTP 500   
     with a clear message) when neither the JAR nor ASYNC_PROFILER_HOME is present.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-3801017354

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 39s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  |  Patch does not have any anti-patterns.  |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 25s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 21s |  |  master passed  |
   | +1 :green_heart: |  compile  |  11m 54s |  |  master passed  |
   | +1 :green_heart: |  checkstyle  |   2m 29s |  |  master passed  |
   | +1 :green_heart: |  spotbugs  |  11m 26s |  |  master passed  |
   | +1 :green_heart: |  spotless  |   1m 12s |  |  branch has no errors when running spotless:check.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 19s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 48s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  13m 25s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |  13m 25s |  |  root generated 0 new + 1886 unchanged - 2 fixed = 1886 total (was 1888)  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   3m  0s |  |  root: The patch generated 0 new + 0 unchanged - 1 fixed = 0 total (was 1)  |
   | +1 :green_heart: |  xmllint  |   0m  0s |  |  No new issues.  |
   | +1 :green_heart: |  spotbugs  |  11m 30s |  |  the patch passed  |
   | +1 :green_heart: |  hadoopcheck  |  18m 11s |  |  Patch does not cause any errors with Hadoop 3.3.6 3.4.1.  |
   | +1 :green_heart: |  spotless  |   1m  3s |  |  patch has no errors when running spotless:check.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 39s |  |  The patch does not generate ASF License warnings.  |
   |  |   |  94m 23s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7679 |
   | Optional Tests | dupname asflicense javac codespell detsecrets xmllint hadoopcheck spotless compile spotbugs checkstyle hbaseanti |
   | uname | Linux 93f23ab627a1 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 8a7bcaa583652374293cf40eee0f3be28298db40 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   | Max. process+thread count | 194 (vs. ulimit of 30000) |
   | modules | C: hbase-resource-bundle hbase-http . U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/2/console |
   | versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 xmllint=20913 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "apurtell (via GitHub)" <gi...@apache.org>.
apurtell commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-4289781792

   Async profiler is an optional dependency. The availability of a profiler on the server side can be a security risk and so some (or many) may opt not to place one there. This change does not gracefully handle the absence of the profiler nor make its deployment optional. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-3801935406

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 37s |  |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck  |
   |||| _ Prechecks _ |
   |||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 23s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 42s |  |  master passed  |
   | +1 :green_heart: |  compile  |   3m 35s |  |  master passed  |
   | +1 :green_heart: |  javadoc  |   3m 12s |  |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m 58s |  |  branch has no errors when building our shaded downstream artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 30s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 32s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   3m 26s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   3m 26s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   3m 49s |  |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 49s |  |  patch has no errors when building our shaded downstream artifacts.  |
   |||| _ Other Tests _ |
   | -1 :x: |  unit  | 263m 57s | [/patch-unit-root.txt](https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/2/artifact/yetus-jdk17-hadoop3-check/output/patch-unit-root.txt) |  root in the patch failed.  |
   |  |   | 310m 49s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/7679 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 9f19bff6798d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 8a7bcaa583652374293cf40eee0f3be28298db40 |
   | Default Java | Eclipse Adoptium-17.0.11+9 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/2/testReport/ |
   | Max. process+thread count | 4615 (vs. ulimit of 30000) |
   | modules | C: hbase-resource-bundle hbase-http . U: . |
   | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7679/2/console |
   | versions | git=2.34.1 maven=3.9.8 |
   | Powered by | Apache Yetus 0.15.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "apurtell (via GitHub)" <gi...@apache.org>.
apurtell commented on code in PR #7679:
URL: https://github.com/apache/hbase/pull/7679#discussion_r3277718928


##########
hbase-http/src/main/java/org/apache/hadoop/hbase/http/ProfileServlet.java:
##########
@@ -189,109 +265,149 @@ protected void doGet(final HttpServletRequest req, final HttpServletResponse res
     final Integer height = getInteger(req, "height", null);
     final Double minwidth = getMinWidth(req);
     final boolean reverse = req.getParameterMap().containsKey("reverse");
+    int refreshDelay = getInteger(req, "refreshDelay", 0);
+
+    return new ProfileRequest(duration, output, event, interval, jstackDepth, bufsize, thread,
+      simple, width, height, minwidth, reverse, refreshDelay, requestedPid);
+  }
+
+  protected String executeStart(ProfileRequest request, File outputFile) throws IOException {
+    return backend.executeStart(request, outputFile);
+  }
+
+  protected String executeStop(ProfileRequest request, File outputFile) throws IOException {
+    return backend.executeStop(request, outputFile);
+  }
+
+  @Override
+  protected void doGet(final HttpServletRequest req, final HttpServletResponse resp)
+    throws IOException {
+    if (!checkInstrumentationAccess(req, resp)) {
+      return;
+    }
+
+    final ProfileRequest request = parseProfileRequest(req);
+
+    // We keep the pid parameter for backward compatibility but only support profiling this JVM.
+    if (request.getPid() != null && request.getPid().longValue() != currentPid) {
+      writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+        "The 'pid' parameter is only supported for the current process when using the "
+          + "embedded async-profiler library.");
+      return;
+    }
+
+    if (profiling) {
+      writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+        "Another instance of profiler is already running.");
+      return;
+    }
+
+    int lockTimeoutSecs = 3;
+    boolean locked = false;
+    try {
+      locked = profilerLock.tryLock(lockTimeoutSecs, TimeUnit.SECONDS);
+      if (!locked) {
+        writeError(resp, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+          "Unable to acquire lock. Another instance of profiler might be running.");
+        LOG.warn("Unable to acquire lock in " + lockTimeoutSecs
+          + " seconds. Another instance of profiler might be running.");
+        return;
+      }
+
+      File outputFile = createOutputFile(request);
+      // Ensure the file exists so ProfileOutputServlet can poll until it is complete.
+      Files.write(outputFile.toPath(), new byte[0]);
 
-    if (process == null || !process.isAlive()) {
+      executeStart(request, outputFile);
+      profiling = true;
+
+      startStopperThread(request.getDuration(), request, outputFile);
+
+      writeAcceptedResponse(resp, request, outputFile);
+    } catch (InterruptedException e) {

Review Comment:
   The native library won't actually be loaded until first use, and there could be other native layer issues such as a glibc missing some versioned symbols, kernel disabled perf_event support, a seccomp policy, etc. Catching `Error` here, or even `Throwable` and then testing for  `instanceof InterruptedException` to deal with that specific case would handle all of this. Otherwise `UnsatisfiedLinkError ` (most likely) will bubble up to the servlet container which will emit a generic 500 error response. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] HBASE-29853 Move from async profiler binary to a maven dependency [hbase]

Posted by "mnpoonia (via GitHub)" <gi...@apache.org>.
mnpoonia commented on PR #7679:
URL: https://github.com/apache/hbase/pull/7679#issuecomment-4678469597

   @virajjasani @apurtell A gentle reminder.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org