Skip to content

Commit ecdae51

Browse files
arjunashokarjunashok
committed
Update to only perform retries until a valid repair status is received. Address PR comments.
1 parent 7288d9f commit ecdae51

File tree

12 files changed

+396
-193
lines changed

12 files changed

+396
-193
lines changed

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

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818

1919
package org.apache.cassandra.sidecar.common.request.data;
2020

21-
import java.util.Arrays;
2221
import java.util.List;
23-
import java.util.Optional;
2422

2523
import com.fasterxml.jackson.annotation.JsonCreator;
2624
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
@@ -29,6 +27,7 @@
2927
import com.fasterxml.jackson.annotation.JsonValue;
3028
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
3129
import org.apache.cassandra.sidecar.common.DataObjectBuilder;
30+
import org.jetbrains.annotations.NotNull;
3231
import org.jetbrains.annotations.Nullable;
3332

3433
/**
@@ -148,35 +147,32 @@ public Boolean shouldValidate()
148147
*/
149148
public enum RepairType
150149
{
151-
FULL("full"),
152-
INCREMENTAL("incremental");
153-
154-
private final String value;
155-
156-
RepairType(String value)
157-
{
158-
this.value = value;
159-
}
150+
FULL,
151+
INCREMENTAL;
160152

161153
@JsonValue
162154
public String getValue()
163155
{
164-
return value;
156+
return name().toLowerCase();
165157
}
166158

167159
@JsonCreator
168-
public static RepairType fromValue(String text)
160+
public static RepairType fromValue(@NotNull String text)
169161
{
170-
String normalized = Optional.ofNullable(text)
171-
.map(String::trim)
172-
.filter(s -> !s.isEmpty())
173-
.map(String::toLowerCase)
174-
.orElse(null);
175-
176-
return Arrays.stream(RepairType.values())
177-
.filter(type -> type.getValue().equals(normalized))
178-
.findFirst()
179-
.orElseThrow(() -> new IllegalArgumentException("Unexpected value: " + text));
162+
String trimmed = text.trim();
163+
try
164+
{
165+
if (trimmed.isEmpty())
166+
{
167+
throw new IllegalArgumentException("Unexpected value: " + text);
168+
}
169+
170+
return valueOf(trimmed.toUpperCase());
171+
}
172+
catch (IllegalArgumentException e)
173+
{
174+
throw new IllegalArgumentException("Unexpected value: " + text);
175+
}
180176
}
181177
}
182178

‎integration-tests/src/integrationTest/org/apache/cassandra/sidecar/routes/RepairIntegrationTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ void repairTest(VertxTestContext context)
104104
OperationalJobResponse response = getBlocking(trustedClient().put(serverWrapper.serverPort, "localhost", testRoute)
105105
.as(BodyCodec.json(OperationalJobResponse.class))
106106
.sendJson(JsonObject.mapFrom(payload))
107-
.expecting(HttpResponseExpectation.SC_OK))
107+
.expecting(HttpResponseExpectation.SC_ACCEPTED))
108108
.body();
109109

110110
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);

‎server-common/src/main/java/org/apache/cassandra/sidecar/common/server/StorageOperations.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ default void outOfRangeDataCleanup(@NotNull String keyspace, @NotNull String tab
133133
/**
134134
* Triggers a repair operation for the given keyspace and options
135135
*
136-
* @param keyspace keyspace for the repair ioeration
136+
* @param keyspace keyspace for the repair operation
137137
* @param options repair options
138138
* @return an integer value representing the status of the repair operation
139139
* which can be used as a reference to check for the status of the repair session via {@link #getParentRepairStatus(int)}.

‎server/src/main/java/org/apache/cassandra/sidecar/config/RepairJobsConfiguration.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@
2626
public interface RepairJobsConfiguration
2727
{
2828
/**
29-
* @return the max runtime (in milliseconds) of a repair job tracked by the Sidecar
29+
* @return the max retry attempts for the repair job status to be valid
3030
*/
31-
MillisecondBoundConfiguration maxRepairJobRuntime();
31+
int validRepairStatusAttempts();
3232

3333
/**
34-
* @return the polling interval (in milliseconds) that checks for the completion of the repair job
34+
* @return the polling interval (in milliseconds) that checks for the status of the repair job
3535
*/
3636
MillisecondBoundConfiguration repairPollInterval();
3737
}

‎server/src/main/java/org/apache/cassandra/sidecar/config/yaml/RepairJobsConfigurationImpl.java‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,48 +31,50 @@
3131
public class RepairJobsConfigurationImpl implements RepairJobsConfiguration
3232
{
3333
// 1 day in milliseconds
34-
public static final long DEFAULT_MAX_REPAIR_RUNTIME_MILLIS = 24 * 60 * 60 * 1000L;
35-
public static final long DEFAULT_REPAIR_POLLING_INTERVAL_MILLIS = 2_000L;
34+
public static final int DEFAULT_VALID_REPAIR_STATUS_ATTEMPTS = 5;
35+
public static final long DEFAULT_REPAIR_STATUS_POLLING_INTERVAL_MILLIS = 2_000L;
3636

37-
@JsonProperty(value = "max_repair_runtime", defaultValue = DEFAULT_MAX_REPAIR_RUNTIME_MILLIS + "")
38-
protected final long maxRepairRuntimeMillis;
37+
@JsonProperty(value = "repair_status_attempts", defaultValue = DEFAULT_VALID_REPAIR_STATUS_ATTEMPTS + "")
38+
protected final int validRepairStatusAttempts;
3939

40-
@JsonProperty(value = "repair_polling_interval", defaultValue = DEFAULT_REPAIR_POLLING_INTERVAL_MILLIS + "")
41-
protected final long repairPollIntervalMillis;
40+
@JsonProperty(value = "repair_status_polling_interval", defaultValue = DEFAULT_REPAIR_STATUS_POLLING_INTERVAL_MILLIS + "")
41+
protected final long repairStatusPollIntervalMillis;
4242

4343
/**
4444
* Default constructor that sets default values
4545
*/
4646
public RepairJobsConfigurationImpl()
4747
{
48-
this(DEFAULT_MAX_REPAIR_RUNTIME_MILLIS, DEFAULT_REPAIR_POLLING_INTERVAL_MILLIS);
48+
this(DEFAULT_VALID_REPAIR_STATUS_ATTEMPTS, DEFAULT_REPAIR_STATUS_POLLING_INTERVAL_MILLIS);
4949
}
5050

5151
/**
5252
* Constructor with parameters for JSON deserialization
5353
*
54-
* @param maxRepairRuntimeMillis the maximum runtime for repair jobs in milliseconds
55-
* @param repairPollIntervalMillis the polling interval for repair jobs in milliseconds
54+
* @param validRepairStatusAttempts the max retry attempts for the repair job status to be valid
55+
* @param repairStatusPollIntervalMillis the polling interval for repair job status in milliseconds
5656
*/
5757
@JsonCreator
5858
public RepairJobsConfigurationImpl(
59-
@JsonProperty(value = "max_repair_runtime", defaultValue = DEFAULT_MAX_REPAIR_RUNTIME_MILLIS + "") long maxRepairRuntimeMillis,
60-
@JsonProperty(value = "repair_polling_interval", defaultValue = DEFAULT_REPAIR_POLLING_INTERVAL_MILLIS + "") long repairPollIntervalMillis)
59+
@JsonProperty(value = "repair_status_attempts", defaultValue = DEFAULT_VALID_REPAIR_STATUS_ATTEMPTS + "")
60+
int validRepairStatusAttempts,
61+
@JsonProperty(value = "repair_status_polling_interval", defaultValue = DEFAULT_REPAIR_STATUS_POLLING_INTERVAL_MILLIS + "")
62+
long repairStatusPollIntervalMillis)
6163
{
62-
this.maxRepairRuntimeMillis = maxRepairRuntimeMillis;
63-
this.repairPollIntervalMillis = repairPollIntervalMillis;
64+
this.validRepairStatusAttempts = validRepairStatusAttempts;
65+
this.repairStatusPollIntervalMillis = repairStatusPollIntervalMillis;
6466
}
6567

6668
@Override
67-
public MillisecondBoundConfiguration maxRepairJobRuntime()
69+
public int validRepairStatusAttempts()
6870
{
69-
return new MillisecondBoundConfiguration(maxRepairRuntimeMillis, TimeUnit.MILLISECONDS);
71+
return validRepairStatusAttempts;
7072
}
7173

7274
@Override
7375
public MillisecondBoundConfiguration repairPollInterval()
7476
{
75-
return new MillisecondBoundConfiguration(repairPollIntervalMillis, TimeUnit.MILLISECONDS);
77+
return new MillisecondBoundConfiguration(repairStatusPollIntervalMillis, TimeUnit.MILLISECONDS);
7678
}
7779

7880

‎server/src/main/java/org/apache/cassandra/sidecar/handlers/RepairHandler.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ protected void handleInternal(RoutingContext context,
135135
return;
136136
}
137137

138-
// Get the result, waiting for the specified wait time for result
139138
job.asyncResult(executorPools.service(), config.operationalJobExecutionMaxWaitTime())
140139
.onComplete(v -> OperationalJobUtils.sendStatusBasedResponse(context, job));
141140
}

‎server/src/main/java/org/apache/cassandra/sidecar/handlers/validations/ValidateKeyspaceExistenceHandler.java‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.cassandra.sidecar.common.server.data.Name;
2929
import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
3030
import org.apache.cassandra.sidecar.handlers.AbstractHandler;
31+
import org.apache.cassandra.sidecar.routes.RoutingContextUtils;
3132
import org.apache.cassandra.sidecar.utils.CassandraInputValidator;
3233
import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
3334
import org.jetbrains.annotations.NotNull;
@@ -70,13 +71,27 @@ protected void handleInternal(RoutingContext context,
7071
return;
7172
}
7273

73-
ValidationUtils.validateKeyspaceExists(context, metadataFetcher, executorPools, host, keyspace.name())
74+
ValidationUtils.validateKeyspaceExists(metadataFetcher, executorPools, host, keyspace.name())
7475
.onComplete(ar -> {
75-
if (ar.succeeded() && !context.failed())
76+
if (ar.succeeded())
7677
{
78+
// Store metadata in context
79+
KeyspaceMetadata metadata = ar.result();
80+
RoutingContextUtils.put(context, RoutingContextUtils.SC_KEYSPACE_METADATA, metadata);
7781
context.next();
7882
}
79-
// Context has already been failed by the utility method when validation fails
83+
else
84+
{
85+
// Handle failure
86+
if (ar.cause().getMessage().contains("not found"))
87+
{
88+
context.fail(wrapHttpException(HttpResponseStatus.NOT_FOUND, ar.cause().getMessage()));
89+
}
90+
else
91+
{
92+
context.fail(ar.cause());
93+
}
94+
}
8095
});
8196
}
8297
}

‎server/src/main/java/org/apache/cassandra/sidecar/handlers/validations/ValidateTableExistenceHandler.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,25 @@ protected void handleInternal(RoutingContext context,
7575
return;
7676
}
7777

78-
ValidationUtils.validateKeyspaceExists(context, metadataFetcher, executorPools, host, input.keyspace())
78+
ValidationUtils.validateKeyspaceExists(metadataFetcher, executorPools, host, input.keyspace())
7979
.onComplete(ar -> {
80-
// If validation failed, the context has already been failed by the utility method
81-
if (ar.failed() || context.failed())
80+
if (ar.failed())
8281
{
82+
// Handle failure
83+
if (ar.cause().getMessage().contains("not found"))
84+
{
85+
context.fail(wrapHttpException(HttpResponseStatus.NOT_FOUND, ar.cause().getMessage()));
86+
}
87+
else
88+
{
89+
context.fail(ar.cause());
90+
}
8391
return;
8492
}
93+
94+
// Store metadata in context
95+
KeyspaceMetadata keyspaceMetadata = ar.result();
96+
RoutingContextUtils.put(context, RoutingContextUtils.SC_KEYSPACE_METADATA, keyspaceMetadata);
8597

8698
String table = input.tableName();
8799
if (table == null)
@@ -92,7 +104,6 @@ protected void handleInternal(RoutingContext context,
92104

93105
try
94106
{
95-
KeyspaceMetadata keyspaceMetadata = RoutingContextUtils.get(context, RoutingContextUtils.SC_KEYSPACE_METADATA);
96107
TableMetadata tableMetadata = keyspaceMetadata.getTable(table);
97108
if (tableMetadata == null)
98109
{

‎server/src/main/java/org/apache/cassandra/sidecar/handlers/validations/ValidationUtils.java‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,10 @@
1919
package org.apache.cassandra.sidecar.handlers.validations;
2020

2121
import com.datastax.driver.core.KeyspaceMetadata;
22-
import io.netty.handler.codec.http.HttpResponseStatus;
2322
import io.vertx.core.Future;
24-
import io.vertx.ext.web.RoutingContext;
2523
import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
26-
import org.apache.cassandra.sidecar.routes.RoutingContextUtils;
2724
import org.apache.cassandra.sidecar.utils.InstanceMetadataFetcher;
2825

29-
import static org.apache.cassandra.sidecar.utils.HttpExceptions.wrapHttpException;
30-
3126
/**
3227
* Utility class for validation handlers that check the existence of Cassandra schema elements.
3328
*/
@@ -59,17 +54,16 @@ public static Future<KeyspaceMetadata> getKeyspaceMetadata(InstanceMetadataFetch
5954
}
6055

6156
/**
62-
* Validates that a keyspace exists and stores its metadata in the routing context.
57+
* Validates that a keyspace exists.
6358
*
64-
* @param context the routing context
6559
* @param metadataFetcher the metadata fetcher
6660
* @param executorPools the executor pools
6761
* @param host the host to validate against
6862
* @param keyspace the keyspace name to validate
69-
* @return a Future that completes when validation is done
63+
* @return a Future that completes with the KeyspaceMetadata if the keyspace exists,
64+
* or fails with an error if the keyspace doesn't exist or an error occurs
7065
*/
71-
public static Future<KeyspaceMetadata> validateKeyspaceExists(RoutingContext context,
72-
InstanceMetadataFetcher metadataFetcher,
66+
public static Future<KeyspaceMetadata> validateKeyspaceExists(InstanceMetadataFetcher metadataFetcher,
7367
ExecutorPools executorPools,
7468
String host,
7569
String keyspace)
@@ -78,18 +72,12 @@ public static Future<KeyspaceMetadata> validateKeyspaceExists(RoutingContext con
7872
.compose(keyspaceMetadata -> {
7973
if (keyspaceMetadata == null)
8074
{
81-
context.fail(wrapHttpException(HttpResponseStatus.NOT_FOUND,
82-
"Keyspace " + keyspace + " was not found"));
8375
return Future.failedFuture("Keyspace " + keyspace + " was not found");
8476
}
8577
else
8678
{
87-
RoutingContextUtils.put(context, RoutingContextUtils.SC_KEYSPACE_METADATA, keyspaceMetadata);
8879
return Future.succeededFuture(keyspaceMetadata);
8980
}
90-
}, throwable -> {
91-
context.fail(throwable);
92-
return Future.failedFuture(throwable);
9381
});
9482
}
9583
}

0 commit comments

Comments
 (0)