CASSSIDECAR-360: Sidecar API Endpoint for Nodetool Compaction Stop #272

shalnisundram · 2025-10-30T17:19:53Z

Add bespoke API endpoint for nodetool compaction stop.

This implementation carries out functionality currently performed when execution command nodetool stop, which is a short-running operation that requests ongoing compactions of a specified type or id to be stopped.

See CASSANDRA-20021 JIRA ticket - more details can be found on ongoing effort to build APIs for nodetool C* cluster management operations.

patch by Shalni Sundram

…detool compaction stop API implementation

bbotella · 2025-10-30T17:38:33Z

...in/java/org/apache/cassandra/sidecar/adapters/base/CassandraCompactionManagerOperations.java

+        else
+        {
+            // Handler guarantees compactionType is non-null/non-empty
+            proxy.stopCompaction(compactionType);


Maybe we should add a @NotNull annotation to compactionType?

See change on CassandraCompactionManagerOperations line 77 . Not exactly an annotation but a requireNonNull check. Let me know if that's acceptable.

client-common/src/main/java/org/apache/cassandra/sidecar/common/ApiEndpointsV1.java

...-common/src/main/java/org/apache/cassandra/sidecar/common/request/CompactionStopRequest.java

bbotella · 2025-10-30T17:41:30Z

...main/java/org/apache/cassandra/sidecar/common/request/data/CompactionStopRequestPayload.java

+@JsonInclude(JsonInclude.Include.NON_NULL)
+public class CompactionStopRequestPayload
+{
+    private final String compactionType;


Should this be an enum instead of a free string?

Chose deliberately to use String, as I figured this would encourage forward compatibility with future C* versions if more compactionTypes are added later. i.e., would want to avoid sidecar forcing all C* versions into a single enum definition, and would rather leave this responsibility to cassandra itself. Would appreciate @arjunashok's input for this though.

You are already defining the compaction types as a set in AbstractHandler.SUPPORTED_COMPACTION_TYPES. Better to make it an enum instead of a set.

...ommon/src/main/java/org/apache/cassandra/sidecar/common/response/CompactionStopResponse.java

server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java

bbotella · 2025-10-30T17:46:19Z

server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java

+    @APIResponse(description = "Compaction stop operation completed",
+            responseCode = "200",
+            content = @Content(mediaType = "application/json",
+                    schema = @Schema(implementation = CompactionStopResponse.class)))


Please add the @apiresponse for the error codes as well

See lines 149-189 of CompactionStopHandler.java - I do some custom error handling here, as particular api response codes will have different messages thrown depending on the input. Let me know if I should still edit this section in CassandraOperationsModule.

sklaha

Thank you for the PR. Looks good at a high level. Please run ./gradlew clean; ./gradlew build which will show you check style issues. I am leaving my other comments inline.

server/src/main/java/org/apache/cassandra/sidecar/handlers/CompactionStopHandler.java

...in/java/org/apache/cassandra/sidecar/adapters/base/CassandraCompactionManagerOperations.java

sklaha · 2025-11-05T05:33:05Z

...s/src/integrationTest/org/apache/cassandra/sidecar/routes/CompactionStopIntegrationTest.java

+    }
+
+    @Test
+    void testStopCompactionByIdSuccess()


In the current implementation, a request with a non-existent compaction ID silently succeeds. The JMX call simply succeeds without any action being taken. It would be more informative to update the Cassandra JMX implementation to return a boolean value or some other indicator. I will seek input from others on this.

https://github.pie.apple.com/slaha/aci-cassandra/blob/38ab7d02e0979074f3de1c7f90ce70668d2492b9/src/java/org/apache/cassandra/db/compaction/CompactionManager.java#L2306

...s/src/integrationTest/org/apache/cassandra/sidecar/routes/CompactionStopIntegrationTest.java

client-common/src/main/java/org/apache/cassandra/sidecar/common/ApiEndpointsV1.java

...-common/src/main/java/org/apache/cassandra/sidecar/common/request/CompactionStopRequest.java

...ommon/src/main/java/org/apache/cassandra/sidecar/common/response/CompactionStopResponse.java

arjunashok · 2025-11-18T21:34:52Z

...ommon/src/main/java/org/apache/cassandra/sidecar/common/response/CompactionStopResponse.java

+    public static final String STATUS_KEY = "status";
+    public static final String ERROR_CODE_KEY = "error_code";


Let's evaluate the intended usage of the error_code field in determining the values. Based on the current name, it seems to suggest this is a status code (currently returning 200 OK)

The 200 status code would be redundant in this case, as it is implied with the HTTP response code.

The other option is to do away with this field since we have a reason field to denote an arbitrary string reason when there is an error (or a non-200 response).

Also, can status be an enum here? Seems like it can be one of 3 or 4 values - SUBMITTED, PENDING or FAILED

Original status options for the command were just two - "PENDING" and "FAILED". For this API it's "SUBMITTED" (more clear I think) and "FAILED".
Status changed to enum - see CompactionStopStatus.java for new enum class

server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java

…ts, fix bug in handler

arjunashok

Looking good for the most part. Added some minor comments.

client/src/main/java/org/apache/cassandra/sidecar/client/SidecarClient.java

arjunashok · 2025-12-02T01:14:44Z

...s/src/integrationTest/org/apache/cassandra/sidecar/routes/CompactionStopIntegrationTest.java

+        assertThat(stopResponse).isNotNull();
+        assertThat(stopResponse.status()).isEqualTo(CompactionStopStatus.SUBMITTED);
+        assertThat(stopResponse.compactionType()).isEqualTo("COMPACTION");
+        assertThat(stopResponse.reason()).isEqualTo("Operation Succeeded");


No longer applicable, correct?

Omitted the reason() field - the status , type, and id fields still stand in the response so left those in

...s/src/integrationTest/org/apache/cassandra/sidecar/routes/CompactionStopIntegrationTest.java

…gration test

...in/java/org/apache/cassandra/sidecar/adapters/base/CassandraCompactionManagerOperations.java

bbotella · 2025-12-03T22:45:12Z

client-common/src/main/java/org/apache/cassandra/sidecar/common/ApiEndpointsV1.java

    public static final String STREAM_STATS_ROUTE = API_V1 + CASSANDRA + "/stats/streams";
    public static final String TABLE_STATS_ROUTE = API_V1 + CASSANDRA + PER_KEYSPACE + PER_TABLE + "/stats";
-
+    public static final String COMPACTION_STOP_ROUTE = API_V1 + CASSANDRA + "/operations/compaction/stop";


nit: I see the operations part of the path in several static variables here. Let's extract it to its own private static

Done. Added the private static, but only used it for the node stop route rather than node drain and node decommission routes which also uses this path, as those changes would not be directly related to this PR.

...main/java/org/apache/cassandra/sidecar/common/request/data/CompactionStopRequestPayload.java

bbotella · 2025-12-03T22:58:53Z

server/src/main/java/org/apache/cassandra/sidecar/handlers/CompactionStopHandler.java

+
+        // Attempt to stop the compaction
+        // If compactionId is provided, use it (takes precedence over type)
+        if (compactionId != null && !compactionId.trim().isEmpty()) {


I see this pattern of "if compactionID then do otherwise do by type" in several different places. What do you think about abstracting that inside operations, with a generic method that does stopCompaction(@Nullable String compactionId, @Nullable String compactionType), and performs that if internally?

Sorry if this has already been discussed.

Yeah in the latter case we would have a single method that accepts two params but only uses one, which others expressed was not preferable. Thus, preserved the stopByID() and stopCompaction() structure already used by node stop

bbotella · 2025-12-03T23:00:26Z

server/src/main/java/org/apache/cassandra/sidecar/handlers/CompactionStopHandler.java

+
+        // Validate that at least one field is provided
+        if (payload.compactionType() == null &&
+            (payload.compactionId() == null || payload.compactionId().trim().isEmpty()))


I also see this pattern of == null or == null or empty across different places. What about adding boolean public methods on payload that return if they are valid? Something like isCompactionTypeValid() and isCompactionIdValid?

Done - added those functions in CompactionStopRequestPayload.java, although didn't use those functions for the pattern present in CassandraCompactionManagerOperations.java, as those methods don't work with the payload object.

cassandra-sidecar/client-common/src/main/java/org/apache/cassandra/sidecar/common/request/data/CompactionStopRequestPayload.java

Line 77 in 0a9b3f4

public boolean hasValidCompactionId() {

…ions for integration tests

…on stop

…est.java

...ava/org/apache/cassandra/sidecar/adapters/base/CassandraCompactionManagerOperationsTest.java

.../java/org/apache/cassandra/sidecar/common/request/data/CompactionStopRequestPayloadTest.java

server/src/main/java/org/apache/cassandra/sidecar/handlers/CompactionStopHandler.java

...in/java/org/apache/cassandra/sidecar/adapters/base/CassandraCompactionManagerOperations.java

jyothsnakonisa · 2025-12-16T22:49:21Z

Looks good overall, few minor comments

sarankk

+1 Thanks Shalni, overall looks great to me. Left some comments, I am +1 once the comments are addressed.

...in/java/org/apache/cassandra/sidecar/adapters/base/CassandraCompactionManagerOperations.java

...main/java/org/apache/cassandra/sidecar/adapters/base/jmx/CompactionManagerJmxOperations.java

client-common/src/main/java/org/apache/cassandra/sidecar/common/ApiEndpointsV1.java

...on/src/main/java/org/apache/cassandra/sidecar/common/server/CompactionManagerOperations.java

server/src/main/java/org/apache/cassandra/sidecar/modules/CassandraOperationsModule.java

sarankk · 2025-12-21T23:04:02Z

...s/adapters-base/src/main/java/org/apache/cassandra/sidecar/adapters/base/CompactionType.java

+/**
+ * Supported compaction types based on Cassandra's OperationType enum
+ */
+public enum CompactionType


I do not see MAJOR_COMPACTION type here from OperationType class, is that intentional?

Thanks for the catch - I may have been looking at another C* version. Just changed it to reflect this list: https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/compaction/OperationType.java

client-common/src/main/java/org/apache/cassandra/sidecar/common/data/CompactionType.java

sarankk · 2025-12-22T20:45:55Z

client-common/src/main/java/org/apache/cassandra/sidecar/common/data/CompactionStopStatus.java

@@ -0,0 +1,17 @@
+package org.apache.cassandra.sidecar.common.data;


Missing license for file

…ported compaction type per cassandra version

…rs to check for supported compaction type

Shalni Sundram added 2 commits October 29, 2025 18:38

implement API endpoint for nodetool compaction stop

e59a7ad

include CassandraCompactionManagerOperations.java modification for no…

1621d35

…detool compaction stop API implementation

bbotella reviewed Oct 30, 2025

View reviewed changes

sklaha reviewed Nov 5, 2025

View reviewed changes

arjunashok reviewed Nov 18, 2025

View reviewed changes

client-common/src/main/java/org/apache/cassandra/sidecar/common/ApiEndpointsV1.java Outdated Show resolved Hide resolved

arjunashok reviewed Nov 18, 2025

View reviewed changes

Shalni Sundram added 2 commits November 19, 2025 09:45

clean imports, add annotations, improve naming for constants

ff07a40

add compaction type enum support, add full compaction integration tes…

fc26e34

…ts, fix bug in handler

shalnisundram force-pushed the CASSSIDECAR-360 branch 2 times, most recently from 4b0287f to c97daba Compare November 19, 2025 18:07

remove redundant JSON response fields for compaction stop endpoint

faa9f7c

shalnisundram force-pushed the CASSSIDECAR-360 branch from c97daba to faa9f7c Compare November 19, 2025 18:19

shalnisundram and others added 2 commits November 19, 2025 10:52

Merge branch 'trunk' into CASSSIDECAR-360

3e60ced

include compaction stop import

a6b5497

shalnisundram force-pushed the CASSSIDECAR-360 branch from fd5e624 to a6b5497 Compare November 19, 2025 19:06

arjunashok reviewed Dec 2, 2025

View reviewed changes

shalnisundram force-pushed the CASSSIDECAR-360 branch 4 times, most recently from 5013920 to def6e6f Compare December 3, 2025 21:03

add SidecarClientTest test for compaction stop, assumeTrue() for inte…

f404939

…gration test

shalnisundram force-pushed the CASSSIDECAR-360 branch from def6e6f to f404939 Compare December 3, 2025 21:05

bbotella reviewed Dec 3, 2025

View reviewed changes

Shalni Sundram added 2 commits December 4, 2025 11:48

make minor structural changes, adjust tests

0a9b3f4

throttle compaction via nodetool compactiontrhoughput to slow compact…

39c9242

…ions for integration tests

shalnisundram force-pushed the CASSSIDECAR-360 branch from b7d1bae to 39c9242 Compare December 8, 2025 22:04

Shalni Sundram and others added 3 commits December 11, 2025 09:43

fix enum errors in integration tests

a439508

shorten integration test duration for triggering and testing compacti…

08fce25

…on stop

eliminiate redundant tests and settings in CompactionStopIntegrationT…

6a023d6

…est.java

jyothsnakonisa approved these changes Dec 16, 2025

View reviewed changes

return trimmed compaction ID, throw errors, add new lines

b4bb71e

sarankk reviewed Dec 21, 2025

View reviewed changes

shalnisundram added 3 commits December 22, 2025 12:24

change post to put request, clean up, edit compaction types

dcf5c94

resolve checksytle errors

4d9101d

apply formatting changes

f83d1bb

sarankk reviewed Dec 22, 2025

View reviewed changes

shalnisundram added 3 commits December 22, 2025 17:00

edit enums in tests, make bug fix for post to put request change

1ed12a2

limit CompactionType functionality to adapter-specific checks for sup…

9767b72

…ported compaction type per cassandra version

limit CompactionType enum to use in cassandra version-specific adapte…

bddf663

…rs to check for supported compaction type

		public static final String STATUS_KEY = "status";
		public static final String ERROR_CODE_KEY = "error_code";

		@@ -0,0 +1,17 @@
		package org.apache.cassandra.sidecar.common.data;

CASSSIDECAR-360: Sidecar API Endpoint for Nodetool Compaction Stop #272

Are you sure you want to change the base?

CASSSIDECAR-360: Sidecar API Endpoint for Nodetool Compaction Stop #272

Uh oh!

Conversation

shalnisundram commented Oct 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shalnisundram Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklaha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shalnisundram Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arjunashok left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyothsnakonisa commented Dec 16, 2025

shalnisundram Oct 31, 2025 •

edited

Loading

shalnisundram Nov 19, 2025 •

edited

Loading