HBASE-29853 Move from async profiler binary to a maven dependency#7679
HBASE-29853 Move from async profiler binary to a maven dependency#7679mnpoonia wants to merge 3 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
mnpoonia
force-pushed
the
hbase_async_profiler
branch
from
91ff534 to
8a7bcaa
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
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. |
mnpoonia
force-pushed
the
hbase_async_profiler
branch
from
8a7bcaa to
268b567
Compare
mnpoonia
force-pushed
the
hbase_async_profiler
branch
from
268b567 to
05919a4
Compare
Good point. Updated PR addresses both concerns: the dependency is now marked true so it is never transitively pulled into downstream modules. Users who want to bundle it can activate |
|
@apurtell FYI. Hopefully i have addressed the concerns. |
|
@virajjasani Please review |
There was a problem hiding this comment.
Almost there. There's a first use case where we could be more graceful in explaining the error.
Also consider a hbase.profiler.enabled = < true | false > configuration setting that allows operators to selectively disable the profiler even if the binaries are present in the deployment. Just a thought.
| startStopperThread(request.getDuration(), request, outputFile); | ||
|
|
||
| writeAcceptedResponse(resp, request, outputFile); | ||
| } catch (InterruptedException e) { |
There was a problem hiding this 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.
…toggle, and cleanup - Catch Error (UnsatisfiedLinkError, glibc mismatch, perf_event disabled, seccomp) in ProfileServlet.doGet so native load failures return a descriptive 500 instead of bubbling to the servlet container as a generic error - Add hbase.profiler.enabled config (default true) to HttpServer so operators can disable the /prof endpoint via config even when the async-profiler binary is present - Cache ProfilerBackend.detect() result in a static field so isAvailable() and the default constructor share one detection call instead of each triggering reflective class-loading at startup - Reduce getAsyncProfilerHome() from public to package-private - Add TestHttpServer#testProfilerDisabledByConfig to verify the new config flag Co-authored-by: Claude <noreply@anthropic.com>
mnpoonia
force-pushed
the
hbase_async_profiler
branch
from
cbf1ae2 to
e55e963
Compare
3 participants
No description provided.