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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
