CASSANDRA-20854: Support low-overhead async profiling #4487

bbotella · 2025-11-20T18:40:10Z

This is a follow up PR on top of
#4255

conf/jvm-server.options

src/java/org/apache/cassandra/profiler/AsyncProfilerMBean.java

src/java/org/apache/cassandra/tools/profiler/AsyncProfilerService.java

test/unit/org/apache/cassandra/tools/AsyncProfilerServiceTest.java

test/unit/org/apache/cassandra/tools/AsyncProfilerTest.java

src/java/org/apache/cassandra/tools/nodetool/Profile.java

src/java/org/apache/cassandra/config/CassandraRelevantProperties.java

src/java/org/apache/cassandra/service/CassandraDaemon.java

src/java/org/apache/cassandra/profiler/AsyncProfilerMBean.java

src/java/org/apache/cassandra/tools/nodetool/Profile.java

src/java/org/apache/cassandra/tools/profiler/AsyncProfilerService.java

src/java/org/apache/cassandra/service/AsyncProfilerService.java

src/java/org/apache/cassandra/tools/profiler/AsyncProfilerService.java

src/java/org/apache/cassandra/profiler/AsyncProfilerUnsafe.java

src/java/org/apache/cassandra/service/AsyncProfilerService.java

netudima · 2025-12-03T17:50:17Z

src/java/org/apache/cassandra/service/AsyncProfilerService.java

+        try
+        {
+            createLogDir();
+            return Files.readAllBytes(new File(logDir, resultFile).toPath());


we need a validate a file size here against a configured limit, if it is too large we can cause OOM or affect GC on a server size or OOM on a client side (nodetool), I suppose the default value should be around 25MiB...

@netudima should not we also limit duration of profiling? I think that something "sensible" should be used there as well but at the same time something not too short. Like 12h? That has to be enough for everybody I guess?

Also we can not limit size of a file before it is created. Can we? This can be limited in async-profiler itself? We can basically just react on this. So what if async-profiler creates a file bigger than your limit? How do we actually get that information? As this is all asynchronous - if we wait for duration to expire (if we do not stop it ourselves).
What do you want to do with such a big file? Remove it?
We were also thinking about compressing it before sending.

should not we also limit duration of profiling?

yes, I think it makes sense to set an upper limit, maybe 12h is even too big.., I think 1h is more realistic one (I do not remember if I actually used more than 15 minutes in reality). If if we speak about hours it is more like continuous profiling use case and should be implemented in a different way

Also we can not limit size of a file before it is created

I've not seen such logic in async-profiler.., only in FlightRecorder. I am not sure if we can handle this use case easily enough to implement..

I guess time limit would only apply for the safe mode right?

That of course mitigates the problem, but does not quite make it bullet proof. I guess such a guardrail could depend on a monitor process server side that automatically stops the profiling if the process is going to be causing GC problems? That worries me as I think we may be getting in the area of over complicating things... Using nodetool against a live database is complex. And you can do worse things than profiling for an hour with it. Do those commands have guardrails?

src/java/org/apache/cassandra/service/AsyncProfilerService.java

src/java/org/apache/cassandra/profiler/AsyncProfiler.java

src/java/org/apache/cassandra/service/AsyncProfilerService.java

src/java/org/apache/cassandra/tools/nodetool/AsyncProfileCommandGroup.java

src/java/org/apache/cassandra/service/AsyncProfilerService.java

src/java/org/apache/cassandra/service/CassandraDaemon.java

src/java/org/apache/cassandra/tools/nodetool/AsyncProfileCommandGroup.java

Add enable/disable AsyncProfiler JVM flags Refactor Add output format option & initial tests Refactor & add tests Add tests for system properties & refactor Checkstyle fixes & switching airlift with picocli (CASSANDRA-17445) Add more tests Fix Picocli 'Profile' command integration Address feedback Changes.txt Remove not needed property Add missing licenses Apply feedback Fix help tests refactoring fix commands hardened validation and simplification of commands fix log dir fixed tests more fixes add purge command more hardening implement list and fetch Remove unused import more fixes added documentation fixed nodetool help output tests added status command introduced binary download of files in fetch command if necessary hardened code ability to specify duration in human format (e.g. 5m) improved error parsing ability to execute purge, list and fetch even with disabled profiler async-profiler is disabled by default added nodetool tests add startup check checking kernel parameters Updates documentation with different profiler info Applies some feedback Merge AsyncProfiler and AsyncProfilerService Improvements Improve refactor Fixed folder for profiles Fix help tests Use config log folder Applies feedback more fixes

fixes

smiklosovic · 2025-12-16T11:37:33Z

src/java/org/apache/cassandra/tools/nodetool/AsyncProfileCommandGroup.java

-                if (!profiler.start(event.stream().map(Enum::name).collect(joining(",")),
-                                    outputFormat.name(),
-                                    duration,
-                                    validateOutputFileName(filename)))


you need to move validateOutputFileName over to Map.of() for ASYNC_PROFILER_START_OUTPUT_FILE_NAME_PARAM entry value.

smiklosovic · 2025-12-16T11:39:55Z

src/java/org/apache/cassandra/service/AsyncProfilerService.java

        }
    }

+    private void validateStartParameters(Map<String, String> parameters)


can you please move this method down, before validateStopParameters? I do not like that we mix public method with private, just move private down so it is not distracting.

smiklosovic · 2025-12-16T12:02:50Z

src/java/org/apache/cassandra/service/AsyncProfilerService.java

+
+    private void validateStopParameters(Map<String, String> parameters)
+    {
+        // With current implementation, the only parameter for output filename is optional. There is nothing else to


well this is not entirely true, we most probably want to validate the the map does not contain anything which it should not if it is not empty.

smiklosovic · 2025-12-16T12:36:43Z

src/java/org/apache/cassandra/tools/nodetool/AsyncProfileCommandGroup.java

            doWithProfiler(probe, profiler -> {
                String file = filename != null ? validateOutputFileName(filename) : null;
-                if (!profiler.stop(file))
+                Map<String, String> stopParameters = Map.of(ASYNC_PROFILER_STOP_OUTPUT_FILE_NAME_PARAM, file);


this fails when file is not specified on command line, it fails to create a map where value of that entry is null.

fixes

jyothsnakonisa

Looks good overall, a few minor comments.

jyothsnakonisa · 2025-12-19T00:21:11Z

src/java/org/apache/cassandra/service/AsyncProfilerService.java

+    {
+        if (!ASYNC_PROFILER_START_PARAMS.equals(parameters.keySet()))
+        {
+            throw new IllegalArgumentException("Wrong parameters passed to start async profiler method. Passed parameters" +


May be "Invalid parameters. Allowed parameters are: ..." might be a better message?

jyothsnakonisa · 2025-12-19T00:21:41Z

src/java/org/apache/cassandra/service/AsyncProfilerService.java

+    {
+        if (!ASYNC_PROFILER_STOP_PARAMS.containsAll(parameters.keySet()))
+        {
+            throw new IllegalArgumentException("Wrong parameters passed to stop async profiler method. Passed parameters" +


Same here, may be "Invalid parameters. Allowed parameters are: ..." might be a better message?

jyothsnakonisa · 2025-12-19T00:33:03Z

src/java/org/apache/cassandra/service/AsyncProfilerService.java

+
+    private void validateStopParameters(Map<String, String> parameters)
+    {
+        if (!ASYNC_PROFILER_STOP_PARAMS.containsAll(parameters.keySet()))


Should this be instead?

Suggested change

if (!ASYNC_PROFILER_STOP_PARAMS.containsAll(parameters.keySet()))

if (!parameters.keySet().stream().allMatch(ASYNC_PROFILER_STOP_PARAMS::contains))

netudima reviewed Nov 20, 2025

View reviewed changes

conf/jvm-server.options Outdated Show resolved Hide resolved

netudima reviewed Nov 20, 2025

View reviewed changes

src/java/org/apache/cassandra/profiler/AsyncProfilerMBean.java Outdated Show resolved Hide resolved

netudima reviewed Nov 20, 2025

View reviewed changes

src/java/org/apache/cassandra/tools/profiler/AsyncProfilerService.java Outdated Show resolved Hide resolved

netudima reviewed Nov 20, 2025

View reviewed changes

test/unit/org/apache/cassandra/tools/AsyncProfilerServiceTest.java Outdated Show resolved Hide resolved

netudima reviewed Nov 20, 2025

View reviewed changes

test/unit/org/apache/cassandra/tools/AsyncProfilerServiceTest.java Show resolved Hide resolved

netudima reviewed Nov 20, 2025

View reviewed changes

test/unit/org/apache/cassandra/tools/AsyncProfilerTest.java Outdated Show resolved Hide resolved

netudima reviewed Nov 20, 2025

View reviewed changes

src/java/org/apache/cassandra/tools/nodetool/Profile.java Outdated Show resolved Hide resolved