From c2b98e66927f3383e146285a51bd1e489fa9468f Mon Sep 17 00:00:00 2001 From: shobham Date: Wed, 28 Oct 2026 20:47:12 +0530 Subject: [PATCH 1/9] feat(nv-boot): add ServiceUnavailableException and stop echoing unmapped messages Add a shared ServiceUnavailableException (503, Retry-After: 10s by default, withoutRetryAfter when a retry is unsafe) for dependencies that are temporarily unavailable, so services stop defining their own. The MVC and reactive catch-all handlers now return a fixed "Internal server error" detail instead of the exception message, which can carry internals such as CQL text. Relates to #2893 Signed-off-by: shobham --- .../ServiceUnavailableException.java | 56 +++++++++++++++++++ .../handlers/BootMvcExceptionHandler.java | 7 ++- .../BootReactiveExceptionHandler.java | 6 +- .../exceptions/BootResponseExceptionTest.java | 30 ++++++++++ .../handlers/BootMvcExceptionHandlerTest.java | 4 +- .../BootReactiveExceptionHandlerTest.java | 4 +- 6 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/ServiceUnavailableException.java diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/ServiceUnavailableException.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/ServiceUnavailableException.java new file mode 100644 index 0000000000..469128ae22 --- /dev/null +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/ServiceUnavailableException.java @@ -0,0 +1,56 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.nvidia.boot.exceptions; + +import java.time.Duration; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; + +/** + * A dependency such as the database is temporarily unavailable. Sets {@code Retry-After} so + * clients back off, unless created with {@link #withoutRetryAfter}; the default covers a typical + * node failover. + */ +public class ServiceUnavailableException extends BootResponseException { + + public static final Duration DEFAULT_RETRY_AFTER = Duration.ofSeconds(10); + + public ServiceUnavailableException(String message) { + this(message, null, DEFAULT_RETRY_AFTER); + } + + public ServiceUnavailableException(String message, Throwable cause) { + this(message, cause, DEFAULT_RETRY_AFTER); + } + + public ServiceUnavailableException(String message, Throwable cause, Duration retryAfter) { + super(HttpStatus.SERVICE_UNAVAILABLE, message, cause, ServiceUnavailableException.class); + if (retryAfter != null) { + getHeaders().set(HttpHeaders.RETRY_AFTER, + Long.toString(Math.max(1, retryAfter.toSeconds()))); + } + } + + /** + * Omits {@code Retry-After}, for failures where a blind retry is unsafe, such as a create + * whose outcome is unknown: retrying it can create a second resource. + */ + public static ServiceUnavailableException withoutRetryAfter(String message, Throwable cause) { + return new ServiceUnavailableException(message, cause, null); + } +} diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/handlers/BootMvcExceptionHandler.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/handlers/BootMvcExceptionHandler.java index 3be510c514..f5c3428daf 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/handlers/BootMvcExceptionHandler.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/handlers/BootMvcExceptionHandler.java @@ -75,6 +75,10 @@ protected ResponseEntity handleAuthentication( return super.handleException(new UnauthorizedException(message, ex), request); } + /** + * The detail is fixed: an unmapped exception's message can carry internals such as a CQL + * statement. The exception itself is still recorded. + */ @ExceptionHandler(Exception.class) protected ResponseEntity handleUnmapped( Exception ex, @@ -84,8 +88,7 @@ protected ResponseEntity handleUnmapped( new ErrorResponseException(INTERNAL_SERVER_ERROR, ProblemDetail.forStatusAndDetail( INTERNAL_SERVER_ERROR, - ex.getMessage() != null ? ex.getMessage() : - "Internal server error"), + "Internal server error"), ex); return super.handleException(wrapped, request); } diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/handlers/BootReactiveExceptionHandler.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/handlers/BootReactiveExceptionHandler.java index ae9b728886..8ad9850e52 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/handlers/BootReactiveExceptionHandler.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/main/java/com/nvidia/boot/exceptions/handlers/BootReactiveExceptionHandler.java @@ -71,6 +71,10 @@ protected Mono> handleException( new UnauthorizedException(ex.getMessage(), ex.getCause()), exchange); } + /** + * The detail is fixed: an unmapped exception's message can carry internals such as a CQL + * statement. The exception itself is still recorded. + */ @ExceptionHandler(Exception.class) protected Mono> handleExceptionCatchAll( Exception ex, ServerWebExchange exchange) { @@ -78,6 +82,6 @@ protected Mono> handleExceptionCatchAll( new ErrorResponseException(HttpStatus.INTERNAL_SERVER_ERROR, ProblemDetail.forStatusAndDetail( HttpStatus.INTERNAL_SERVER_ERROR, - ex.getMessage()), ex), exchange); + "Internal server error"), ex), exchange); } } diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/BootResponseExceptionTest.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/BootResponseExceptionTest.java index 84f13eff3e..8dccbe32a0 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/BootResponseExceptionTest.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/BootResponseExceptionTest.java @@ -19,7 +19,9 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.time.Duration; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; class BootResponseExceptionTest { @@ -36,6 +38,34 @@ void badRequestExceptionSetsStatusDetailAndUrn() { assertThat(body.getTitle()).isEqualTo(HttpStatus.BAD_REQUEST.getReasonPhrase()); } + @Test + void serviceUnavailableExceptionSetsStatusUrnAndRetryAfter() { + var cause = new RuntimeException("root"); + var ex = new ServiceUnavailableException(MESSAGE, cause); + assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + assertThat(ex.getCause()).isSameAs(cause); + assertThat(ex.getBody().getDetail()).isEqualTo(MESSAGE); + assertThat(ex.getBody().getType()) + .hasToString("urn:nv-boot:problem-details:service-unavailable"); + assertThat(ex.getHeaders().getFirst(HttpHeaders.RETRY_AFTER)).isEqualTo("10"); + } + + @Test + void serviceUnavailableExceptionRoundsRetryAfterUpToOneSecond() { + var ex = new ServiceUnavailableException(MESSAGE, null, Duration.ofMillis(200)); + assertThat(ex.getHeaders().getFirst(HttpHeaders.RETRY_AFTER)).isEqualTo("1"); + } + + @Test + void serviceUnavailableExceptionWithoutRetryAfterOmitsHeader() { + var cause = new RuntimeException("root"); + var ex = ServiceUnavailableException.withoutRetryAfter(MESSAGE, cause); + assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + assertThat(ex.getCause()).isSameAs(cause); + assertThat(ex.getBody().getDetail()).isEqualTo(MESSAGE); + assertThat(ex.getHeaders().containsHeader(HttpHeaders.RETRY_AFTER)).isFalse(); + } + @Test void badRequestExceptionWithCausePreservesCause() { var cause = new RuntimeException("root"); diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/handlers/BootMvcExceptionHandlerTest.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/handlers/BootMvcExceptionHandlerTest.java index 69e9942301..1fef5874ee 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/handlers/BootMvcExceptionHandlerTest.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/handlers/BootMvcExceptionHandlerTest.java @@ -81,12 +81,12 @@ void handleAuthenticationNullMessageUsesDefault() throws Exception { @Test void handleUnmappedReturnsInternalServerErrorAndRecordsException() throws Exception { - var ex = new IllegalStateException("unexpected"); + var ex = new IllegalStateException("Query; CQL [SELECT * FROM t]; timeout"); var response = handler.handleUnmapped(ex, webRequest); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); assertThat(response.getBody()).isInstanceOf(ProblemDetail.class); var body = (ProblemDetail) response.getBody(); - assertThat(body.getDetail()).isEqualTo("unexpected"); + assertThat(body.getDetail()).isEqualTo("Internal server error"); // recordException is invoked from handleExceptionInternal with the wrapped exception assertThat(handler.recordedException).isInstanceOf(org.springframework.web.ErrorResponseException.class); assertThat(handler.recordedException.getCause()).isSameAs(ex); diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/handlers/BootReactiveExceptionHandlerTest.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/handlers/BootReactiveExceptionHandlerTest.java index a0e6adcaff..5bbeedf24a 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/handlers/BootReactiveExceptionHandlerTest.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-exceptions/src/test/java/com/nvidia/boot/exceptions/handlers/BootReactiveExceptionHandlerTest.java @@ -67,13 +67,13 @@ void handleAuthenticationReturnsUnauthorizedWithProblemDetail() { @Test void handleExceptionCatchAllReturnsInternalServerError() { - var ex = new IllegalStateException("unexpected"); + var ex = new IllegalStateException("Query; CQL [SELECT * FROM t]; timeout"); var response = handler.handleExceptionCatchAll(ex, exchange).block(); assertThat(response).isNotNull(); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); assertThat(response.getBody()).isInstanceOf(ProblemDetail.class); var body = (ProblemDetail) response.getBody(); - assertThat(body.getDetail()).isEqualTo("unexpected"); + assertThat(body.getDetail()).isEqualTo("Internal server error"); } @Test From a4238329109f86abd2da121fd07d03f6bacb62d0 Mon Sep 17 00:00:00 2001 From: shobham Date: Wed, 28 Oct 2026 20:47:12 +0530 Subject: [PATCH 2/9] refactor(api-keys): use the shared ServiceUnavailableException Replace CassandraUnavailableException with the nv-boot exception. Status and messages are unchanged; responses now also carry Retry-After, except when the outcome of a key write is unknown. Relates to #2893 Signed-off-by: shobham --- .../CassandraUnavailableException.java | 28 ------------------- .../exceptions/MvcExceptionHandler.java | 3 +- .../apikeys/persistance/dao/KeysDao.java | 8 ++++-- .../exceptions/MvcExceptionHandlerTest.java | 9 ++++-- .../apikeys/persistance/dao/KeysDaoTest.java | 13 +++++---- 5 files changed, 22 insertions(+), 39 deletions(-) delete mode 100644 src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/config/exceptions/CassandraUnavailableException.java diff --git a/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/config/exceptions/CassandraUnavailableException.java b/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/config/exceptions/CassandraUnavailableException.java deleted file mode 100644 index a92e3c97d0..0000000000 --- a/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/config/exceptions/CassandraUnavailableException.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.nvidia.apikeys.config.exceptions; - -import com.nvidia.boot.exceptions.BootResponseException; -import org.springframework.http.HttpStatus; - -public class CassandraUnavailableException extends BootResponseException { - - public CassandraUnavailableException(String message, Throwable cause) { - super(HttpStatus.SERVICE_UNAVAILABLE, message, cause, CassandraUnavailableException.class); - } -} diff --git a/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/config/exceptions/MvcExceptionHandler.java b/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/config/exceptions/MvcExceptionHandler.java index 9a13827af2..318b96d41e 100644 --- a/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/config/exceptions/MvcExceptionHandler.java +++ b/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/config/exceptions/MvcExceptionHandler.java @@ -18,6 +18,7 @@ package com.nvidia.apikeys.config.exceptions; import com.nvidia.boot.exceptions.BootResponseException; +import com.nvidia.boot.exceptions.ServiceUnavailableException; import com.nvidia.boot.exceptions.handlers.BootMvcExceptionHandler; import io.opentelemetry.api.trace.Span; import jakarta.annotation.Nonnull; @@ -63,7 +64,7 @@ protected ResponseEntity handleCassandraUnavailable( DataAccessException ex, WebRequest request) throws Exception { return super.handleException( - new CassandraUnavailableException("Key storage is temporarily unavailable", ex), + new ServiceUnavailableException("Key storage is temporarily unavailable", ex), request); } diff --git a/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/persistance/dao/KeysDao.java b/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/persistance/dao/KeysDao.java index af0a6c30df..14b9e34393 100644 --- a/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/persistance/dao/KeysDao.java +++ b/src/control-plane-services/api-keys/src/main/java/com/nvidia/apikeys/persistance/dao/KeysDao.java @@ -21,7 +21,6 @@ import com.datastax.oss.driver.api.core.cql.ResultSet; import com.nvidia.apikeys.converters.KeyOwnerVoBuilder; import com.nvidia.apikeys.config.exceptions.CassandraException; -import com.nvidia.apikeys.config.exceptions.CassandraUnavailableException; import com.nvidia.apikeys.persistance.models.KeyByOwnerAndServiceModel; import com.nvidia.apikeys.persistance.models.KeyModel; import com.nvidia.apikeys.persistance.repositories.KeyByOwnerAndServiceRepository; @@ -31,6 +30,7 @@ import com.nvidia.apikeys.vo.KeyOwnerVo; import com.nvidia.apikeys.vo.KeyVo; import com.nvidia.apikeys.vo.SavedKeyVo; +import com.nvidia.boot.exceptions.ServiceUnavailableException; import com.nvidia.boot.exceptions.UnprocessableEntityException; import java.time.Clock; import java.util.List; @@ -187,13 +187,15 @@ public void deleteKey(KeyByOwnerAndServiceVo key) { /** * Writes the rows of a logged batch one by one after the batch failed. The batch outcome is * unknown: a timed-out batch may still be replayed from the batchlog. The rows use the batch - * timestamp, so writing them again leaves the same state as the batch. + * timestamp, so writing them again leaves the same state as the batch. A failure here does not + * advertise Retry-After: a client that retries a create generates a different key, while the + * first one may already be stored. */ private void writeDirectly(Runnable write) { try { write.run(); } catch (TransientDataAccessException | DataAccessResourceFailureException e) { - throw new CassandraUnavailableException( + throw ServiceUnavailableException.withoutRetryAfter( "Key storage is temporarily unavailable, the outcome of the write is unknown", e); } } diff --git a/src/control-plane-services/api-keys/src/test/java/com/nvidia/apikeys/config/exceptions/MvcExceptionHandlerTest.java b/src/control-plane-services/api-keys/src/test/java/com/nvidia/apikeys/config/exceptions/MvcExceptionHandlerTest.java index 36c43a106a..608e43fe52 100644 --- a/src/control-plane-services/api-keys/src/test/java/com/nvidia/apikeys/config/exceptions/MvcExceptionHandlerTest.java +++ b/src/control-plane-services/api-keys/src/test/java/com/nvidia/apikeys/config/exceptions/MvcExceptionHandlerTest.java @@ -25,6 +25,7 @@ import org.springframework.dao.DataAccessException; import org.springframework.data.cassandra.CassandraConnectionFailureException; import org.springframework.data.cassandra.CassandraReadTimeoutException; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ProblemDetail; import org.springframework.http.ResponseEntity; @@ -52,7 +53,11 @@ private void assertServiceUnavailable(DataAccessException ex) throws Exception { assertThat(response).isNotNull(); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); - assertThat(response.getBody()).isInstanceOfSatisfying(ProblemDetail.class, body -> - assertThat(body.getDetail()).isEqualTo("Key storage is temporarily unavailable")); + assertThat(response.getHeaders().getFirst(HttpHeaders.RETRY_AFTER)).isEqualTo("10"); + assertThat(response.getBody()).isInstanceOfSatisfying(ProblemDetail.class, body -> { + assertThat(body.getDetail()).isEqualTo("Key storage is temporarily unavailable"); + assertThat(body.getType()) + .hasToString("urn:nv-boot:problem-details:service-unavailable"); + }); } } diff --git a/src/control-plane-services/api-keys/src/test/java/com/nvidia/apikeys/persistance/dao/KeysDaoTest.java b/src/control-plane-services/api-keys/src/test/java/com/nvidia/apikeys/persistance/dao/KeysDaoTest.java index b9563422e9..dbf93e4ef3 100644 --- a/src/control-plane-services/api-keys/src/test/java/com/nvidia/apikeys/persistance/dao/KeysDaoTest.java +++ b/src/control-plane-services/api-keys/src/test/java/com/nvidia/apikeys/persistance/dao/KeysDaoTest.java @@ -39,15 +39,16 @@ import com.datastax.oss.driver.api.core.CqlSession; import com.nvidia.apikeys.config.exceptions.CassandraException; -import com.nvidia.apikeys.config.exceptions.CassandraUnavailableException; import com.nvidia.apikeys.persistance.models.KeyModel; import com.nvidia.apikeys.persistance.repositories.KeyByOwnerAndServiceRepository; import com.nvidia.apikeys.persistance.repositories.KeyRepository; import com.nvidia.apikeys.vo.SavedKeyVo; +import com.nvidia.boot.exceptions.ServiceUnavailableException; import java.time.Clock; import java.util.List; import java.util.Optional; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Answers; import org.mockito.InOrder; @@ -156,9 +157,10 @@ void save_throwsUnavailableIfDirectWriteFails() { any(InsertOptions.class))) .thenThrow(writeTimeout("SIMPLE")); - assertThrowsExceptionWithDetails( - CassandraUnavailableException.class, () -> dao.save(KEY_VO_1, KEY_OWNER_VO_1), + var ex = assertThrowsExceptionWithDetails( + ServiceUnavailableException.class, () -> dao.save(KEY_VO_1, KEY_OWNER_VO_1), "Key storage is temporarily unavailable, the outcome of the write is unknown"); + assertThat(ex.getHeaders().containsHeader(HttpHeaders.RETRY_AFTER)).isFalse(); verify(cassandraTemplateMock, never()).insert(eq(modelMock), any(InsertOptions.class)); } @@ -216,10 +218,11 @@ void deleteKey_throwsUnavailableIfDirectDeleteFails() { when(cassandraTemplateMock.delete(eq(expectedDeletedKeyModel), any(DeleteOptions.class))) .thenThrow(writeTimeout("SIMPLE")); - assertThrowsExceptionWithDetails( - CassandraUnavailableException.class, + var ex = assertThrowsExceptionWithDetails( + ServiceUnavailableException.class, () -> dao.deleteKey(KEY_BY_OWNER_AND_SERVICE_VO_1), "Key storage is temporarily unavailable, the outcome of the write is unknown"); + assertThat(ex.getHeaders().containsHeader(HttpHeaders.RETRY_AFTER)).isFalse(); } private void mockBatch() { From 8fda4ba7941e9721acb9eb608b79f012fc228e4c Mon Sep 17 00:00:00 2001 From: shobham Date: Wed, 28 Oct 2026 20:47:12 +0530 Subject: [PATCH 3/9] fix(nvcf-api): return 503 for retriable Cassandra failures Map read timeouts, unavailable replicas, overloaded or bootstrapping coordinators and connection failures to 503 with Retry-After over REST, and UNAVAILABLE over gRPC. Write timeouts, driver timeouts, aborted requests and read/write failures stay 500, because the request may have reached Cassandra. Neither response includes the driver message. Relates to #2893 Signed-off-by: shobham --- .../cloud-functions/nvcf-core/BUILD.bazel | 1 + .../nvcf/configuration/GrpcConfiguration.java | 17 +++ .../ExceptionTracingExceptionHandler.java | 18 +++ .../exceptions/StorageExceptions.java | 40 +++++++ .../configuration/GrpcConfigurationTest.java | 70 +++++++++++ .../ExceptionTracingExceptionHandlerTest.java | 113 ++++++++++++++++++ 6 files changed, 259 insertions(+) create mode 100644 src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/StorageExceptions.java create mode 100644 src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/GrpcConfigurationTest.java create mode 100644 src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandlerTest.java diff --git a/src/control-plane-services/cloud-functions/nvcf-core/BUILD.bazel b/src/control-plane-services/cloud-functions/nvcf-core/BUILD.bazel index 35941f90ee..fb444be5ce 100644 --- a/src/control-plane-services/cloud-functions/nvcf-core/BUILD.bazel +++ b/src/control-plane-services/cloud-functions/nvcf-core/BUILD.bazel @@ -235,6 +235,7 @@ NVCF_CORE_TEST_DEPS = [ "@nv_third_party_deps//:org_springframework_spring_context", "@nv_third_party_deps//:org_springframework_spring_core", "@nv_third_party_deps//:org_springframework_spring_test", + "@nv_third_party_deps//:org_springframework_spring_tx", "@nv_third_party_deps//:org_springframework_spring_web", "@nv_third_party_deps//:org_springframework_spring_webflux", "@nv_third_party_deps//:org_springframework_spring_webmvc", diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/GrpcConfiguration.java b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/GrpcConfiguration.java index 01f36ebbf8..af34a2033f 100644 --- a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/GrpcConfiguration.java +++ b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/GrpcConfiguration.java @@ -21,11 +21,17 @@ import com.nvidia.boot.exceptions.ForbiddenException; import com.nvidia.boot.exceptions.UnauthorizedException; import com.nvidia.nvcf.configuration.exceptions.InvalidInvocationException; +import com.nvidia.nvcf.configuration.exceptions.StorageExceptions; import io.grpc.Metadata; import io.grpc.Status; import io.grpc.Status.Code; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.dao.DataAccessException; +import org.springframework.dao.TransientDataAccessResourceException; +import org.springframework.data.cassandra.CassandraConnectionFailureException; +import org.springframework.data.cassandra.CassandraInsufficientReplicasAvailableException; +import org.springframework.data.cassandra.CassandraReadTimeoutException; import org.springframework.grpc.server.GlobalServerInterceptor; import org.springframework.grpc.server.advice.GrpcAdvice; import org.springframework.grpc.server.advice.GrpcExceptionHandler; @@ -81,6 +87,17 @@ public io.grpc.StatusRuntimeException handleInvalidInvocationException( return status.asRuntimeException(metadata); } + // Keep in sync with ExceptionTracingExceptionHandler; StorageExceptions explains the choice. + @GrpcExceptionHandler({ + CassandraReadTimeoutException.class, + CassandraInsufficientReplicasAvailableException.class, + TransientDataAccessResourceException.class, + CassandraConnectionFailureException.class + }) + public Status handleStorageUnavailable(DataAccessException e) { + return toGrpcStatus(StorageExceptions.storageUnavailable(e)); + } + @GrpcExceptionHandler public Status handleException(AccessDeniedException e) { return handleErrorResponseException(new ForbiddenException(e.getMessage(), e.getCause())); diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandler.java b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandler.java index 5b3e7b5ed3..8c7d7c803e 100644 --- a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandler.java +++ b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandler.java @@ -27,6 +27,11 @@ import io.micrometer.tracing.Tracer; import jakarta.annotation.Nonnull; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.DataAccessException; +import org.springframework.dao.TransientDataAccessResourceException; +import org.springframework.data.cassandra.CassandraConnectionFailureException; +import org.springframework.data.cassandra.CassandraInsufficientReplicasAvailableException; +import org.springframework.data.cassandra.CassandraReadTimeoutException; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.context.request.WebRequest; @@ -131,4 +136,17 @@ protected ResponseEntity handleException( new BadRequestException(ex.getMessage(), ex.getCause()), request); } + // Keep in sync with GrpcConfiguration; StorageExceptions explains the choice. + @ExceptionHandler({ + CassandraReadTimeoutException.class, + CassandraInsufficientReplicasAvailableException.class, + TransientDataAccessResourceException.class, + CassandraConnectionFailureException.class + }) + protected ResponseEntity handleStorageUnavailable( + DataAccessException ex, + WebRequest request) throws Exception { + return super.handleException(StorageExceptions.storageUnavailable(ex), request); + } + } diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/StorageExceptions.java b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/StorageExceptions.java new file mode 100644 index 0000000000..67e6ea182a --- /dev/null +++ b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/StorageExceptions.java @@ -0,0 +1,40 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.nvidia.nvcf.configuration.exceptions; + +import com.nvidia.boot.exceptions.ServiceUnavailableException; + +/** + * Cassandra failures that are safe to report as retriable. + * + *

Only failures where Cassandra did no work are: a coordinator read timeout, too few replicas + * alive, an overloaded or bootstrapping coordinator, or no node reachable. Write timeouts and + * driver timeouts are not, because the write may have been applied and a retry of a create would + * duplicate it. Read and write failures are not transient. + */ +public final class StorageExceptions { + + public static final String STORAGE_UNAVAILABLE = "Storage is temporarily unavailable"; + + private StorageExceptions() { + } + + public static ServiceUnavailableException storageUnavailable(Throwable cause) { + return new ServiceUnavailableException(STORAGE_UNAVAILABLE, cause); + } +} diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/GrpcConfigurationTest.java b/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/GrpcConfigurationTest.java new file mode 100644 index 0000000000..f796effe46 --- /dev/null +++ b/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/GrpcConfigurationTest.java @@ -0,0 +1,70 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.nvidia.nvcf.configuration; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.nvidia.nvcf.configuration.exceptions.ExceptionTracingExceptionHandler; +import com.nvidia.nvcf.configuration.exceptions.StorageExceptions; +import io.grpc.Status; +import org.junit.jupiter.api.Test; +import org.springframework.dao.DataAccessException; +import org.springframework.data.cassandra.CassandraDriverTimeoutException; +import org.springframework.data.cassandra.CassandraReadTimeoutException; +import org.springframework.grpc.server.advice.GrpcExceptionHandler; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.context.request.WebRequest; + +class GrpcConfigurationTest { + + private static final String CQL = "Query; CQL [SELECT * FROM functions_v3]; read timeout"; + + private final GrpcConfiguration grpcConfiguration = new GrpcConfiguration(); + + @Test + void retriableStorageFailureIsUnavailable() { + var status = grpcConfiguration.handleStorageUnavailable( + new CassandraReadTimeoutException(false, CQL, null)); + + assertThat(status.getCode()).isEqualTo(Status.Code.UNAVAILABLE); + assertThat(status.getDescription()).isEqualTo(StorageExceptions.STORAGE_UNAVAILABLE); + } + + @Test + void indexReadTimeoutIsUnavailable() { + var status = grpcConfiguration.handleErrorResponseException( + StorageExceptions.storageUnavailable( + new CassandraDriverTimeoutException(CQL, null))); + + assertThat(status.getCode()).isEqualTo(Status.Code.UNAVAILABLE); + assertThat(status.getDescription()).isEqualTo(StorageExceptions.STORAGE_UNAVAILABLE); + } + + @Test + void grpcAndMvcTreatTheSameStorageFailuresAsRetriable() throws Exception { + var grpcTypes = GrpcConfiguration.class + .getMethod("handleStorageUnavailable", DataAccessException.class) + .getAnnotation(GrpcExceptionHandler.class).value(); + var mvcTypes = ExceptionTracingExceptionHandler.class + .getDeclaredMethod("handleStorageUnavailable", DataAccessException.class, + WebRequest.class) + .getAnnotation(ExceptionHandler.class).value(); + + assertThat(grpcTypes).isNotEmpty().containsExactlyInAnyOrder(mvcTypes); + } +} diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandlerTest.java b/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandlerTest.java new file mode 100644 index 0000000000..56c5b8a672 --- /dev/null +++ b/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandlerTest.java @@ -0,0 +1,113 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.nvidia.nvcf.configuration.exceptions; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; + +import io.micrometer.tracing.Tracer; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.dao.DataAccessResourceFailureException; +import org.springframework.dao.TransientDataAccessResourceException; +import org.springframework.data.cassandra.CassandraConnectionFailureException; +import org.springframework.data.cassandra.CassandraDriverTimeoutException; +import org.springframework.data.cassandra.CassandraInsufficientReplicasAvailableException; +import org.springframework.data.cassandra.CassandraReadTimeoutException; +import org.springframework.data.cassandra.CassandraWriteTimeoutException; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RestController; + +class ExceptionTracingExceptionHandlerTest { + + private static final String CQL = "Query; CQL [SELECT * FROM functions_v3]; "; + + private MockMvc mockMvc; + + @BeforeEach + void setUp() { + var advice = new ValidationAwareExceptionHandler(); + ReflectionTestUtils.setField(advice, "tracer", mock(Tracer.class)); + mockMvc = MockMvcBuilders.standaloneSetup(new FailingStorageController()) + .setControllerAdvice(advice) + .build(); + } + + @ParameterizedTest + @ValueSource(strings = {"read-timeout", "unavailable", "overloaded", "connection-failure"}) + void retriableStorageFailureIsServiceUnavailable(String failure) throws Exception { + var response = perform(failure); + + assertThat(response.getStatus()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE.value()); + assertThat(response.getHeader(HttpHeaders.RETRY_AFTER)).isEqualTo("10"); + assertThat(response.getContentAsString()) + .contains("Storage is temporarily unavailable") + .contains("urn:nv-boot:problem-details:service-unavailable") + .doesNotContain("CQL"); + } + + // A write that timed out may have been applied, and a read or write failure does not clear on + // retry, so none of these may tell the client to retry. + @ParameterizedTest + @ValueSource(strings = {"write-timeout", "driver-timeout", "read-failure", "wrapped"}) + void otherStorageFailureIsInternalErrorWithoutCql(String failure) throws Exception { + var response = perform(failure); + + assertThat(response.getStatus()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR.value()); + assertThat(response.getHeader(HttpHeaders.RETRY_AFTER)).isNull(); + assertThat(response.getContentAsString()).doesNotContain("CQL"); + } + + private MockHttpServletResponse perform(String failure) throws Exception { + return mockMvc.perform(get("/fail/" + failure)).andReturn().getResponse(); + } + + @RestController + static class FailingStorageController { + + @GetMapping("/fail/{failure}") + String fail(@PathVariable String failure) { + throw switch (failure) { + case "read-timeout" -> + new CassandraReadTimeoutException(true, CQL + "read timeout", null); + case "unavailable" -> + new CassandraInsufficientReplicasAvailableException(2, 1, CQL, null); + case "overloaded" -> new TransientDataAccessResourceException(CQL + "overloaded"); + case "connection-failure" -> + new CassandraConnectionFailureException(Map.of(), CQL + "no node", null); + case "write-timeout" -> + new CassandraWriteTimeoutException("SIMPLE", CQL + "write timeout", null); + case "driver-timeout" -> new CassandraDriverTimeoutException(CQL + "timed out", null); + case "read-failure" -> new DataAccessResourceFailureException(CQL + "read failure"); + case "wrapped" -> new IllegalStateException( + "Failed to update deployment: " + CQL + "write timeout"); + default -> new IllegalArgumentException(failure); + }; + } + } +} From dcc9da8710a0e3ef7489002ed40d1bf2240de26d Mon Sep 17 00:00:00 2001 From: shobham Date: Wed, 28 Oct 2026 20:47:12 +0530 Subject: [PATCH 4/9] fix(nv-boot): retry range read timeouts on the next host A secondary-index read is a range read, and the coordinator's own replica always answers, so a timeout caused by a dead replica reports data present with fewer responses than required. The policy rethrew those. Retry them once on the next host, like a read timeout without data. Relates to #2893 Signed-off-by: shobham --- .../nvidia/boot/cassandra/retry/NextHostRetryPolicy.java | 6 ++++-- .../boot/cassandra/retry/NextHostRetryPolicyTest.java | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicy.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicy.java index 9b42707401..c5711d5dd2 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicy.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicy.java @@ -33,7 +33,7 @@ /** * Retry policy that retries on the next host for transient failures (connection/heartbeat, - * read timeout without data, write timeout, unavailable). Rethrows for ReadFailure, WriteFailure, + * read timeout without data or with too few responses, write timeout, unavailable). Rethrows for ReadFailure, WriteFailure, * and after one retry attempt. */ @Slf4j @@ -68,7 +68,9 @@ public RetryVerdict onReadTimeoutVerdict( int received, boolean dataPresent, int retryCount) { - var verdict = retryCount == 0 && !dataPresent ? + // A range read (secondary index) always has data present, from the coordinator's own + // replica, so a timeout there only shows as fewer responses than required. + var verdict = retryCount == 0 && (!dataPresent || received < blockFor) ? RetryVerdict.RETRY_NEXT : RetryVerdict.RETHROW; log.info(VERDICT_ON_READ_TIMEOUT, logPrefix, cl, blockFor, received, dataPresent, retryCount, verdict); diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicyTest.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicyTest.java index b8637c0ca0..92bfa92875 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicyTest.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicyTest.java @@ -55,7 +55,9 @@ class NextHostRetryPolicyTest extends RetryPolicyTestBase { void shouldProcessReadTimeouts() { assertOnReadTimeout(QUORUM, 2, 2, false, 1).hasDecision(RETHROW); assertOnReadTimeout(QUORUM, 2, 1, false, 0).hasDecision(RETRY_NEXT); - assertOnReadTimeout(QUORUM, 2, 1, true, 0).hasDecision(RETHROW); + assertOnReadTimeout(QUORUM, 2, 1, true, 0).hasDecision(RETRY_NEXT); + assertOnReadTimeout(QUORUM, 2, 1, true, 1).hasDecision(RETHROW); + assertOnReadTimeout(QUORUM, 2, 2, true, 0).hasDecision(RETHROW); } @Test From a54980ae22db10d00d84cbb2139a033e045ce001 Mon Sep 17 00:00:00 2001 From: shobham Date: Wed, 28 Oct 2026 20:47:12 +0530 Subject: [PATCH 5/9] fix(cassandra): lower range_request_timeout to 5s Secondary-index reads are range reads, which the coordinator never retries on another replica, so a read that picked a crashed replica waits for the whole range_request_timeout. The 10s default equals the services' 10s driver timeout, so the driver never gets to retry it. 5s gives healthy range reads room to finish (normal function lookups take up to about 2s on a small cluster) and leaves about 5s for the driver retry on another coordinator. In testing, 7s left the retry too little time and some requests still failed. Relates to #2893 Signed-off-by: shobham --- deploy/helm/cassandra/helm/values.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/deploy/helm/cassandra/helm/values.yaml b/deploy/helm/cassandra/helm/values.yaml index 3dae7e3522..b766b25b90 100644 --- a/deploy/helm/cassandra/helm/values.yaml +++ b/deploy/helm/cassandra/helm/values.yaml @@ -74,6 +74,13 @@ cassandra: # streaming connection is rejected as version-incompatible even though # gossip and schema exchange succeed. storage_compatibility_mode: NONE + # Secondary-index (SAI) reads are range reads, which the coordinator never + # retries on another replica, so a crashed replica stalls them for the whole + # timeout. 5s gives healthy range reads room to finish (normal function + # lookups take up to about 2s on a small cluster) and leaves about 5s of the + # services' 10s driver request timeout for the driver retry policy to retry + # on another coordinator. Larger values leave the retry too little time. + range_request_timeout: 5000ms # Automatic repair (Cassandra 5.0). Values supplied by the Cassandra SME. reject_repair_compaction_threshold: 1024 repair_disk_headroom_reject_ratio: 0.2 From 80ecf3299c80893aa00e935538b6939613c80fce Mon Sep 17 00:00:00 2001 From: shobham Date: Wed, 28 Oct 2026 21:23:35 +0530 Subject: [PATCH 6/9] refactor(nvcf-api): drop StorageExceptions in favour of ServiceUnavailableException The handlers now throw nv-boot's ServiceUnavailableException directly. The note on which Cassandra failures are retriable moves to the MVC handler. Signed-off-by: shobham --- .../nvcf/configuration/GrpcConfiguration.java | 7 ++-- .../ExceptionTracingExceptionHandler.java | 16 +++++++- .../exceptions/StorageExceptions.java | 40 ------------------- .../configuration/GrpcConfigurationTest.java | 12 +++--- 4 files changed, 23 insertions(+), 52 deletions(-) delete mode 100644 src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/StorageExceptions.java diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/GrpcConfiguration.java b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/GrpcConfiguration.java index af34a2033f..7628bc9e1b 100644 --- a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/GrpcConfiguration.java +++ b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/GrpcConfiguration.java @@ -16,12 +16,13 @@ */ package com.nvidia.nvcf.configuration; +import static com.nvidia.nvcf.configuration.exceptions.ExceptionTracingExceptionHandler.STORAGE_UNAVAILABLE; import static com.nvidia.nvcf.util.NvcfConstants.TAG_NCA_ID; import com.nvidia.boot.exceptions.ForbiddenException; +import com.nvidia.boot.exceptions.ServiceUnavailableException; import com.nvidia.boot.exceptions.UnauthorizedException; import com.nvidia.nvcf.configuration.exceptions.InvalidInvocationException; -import com.nvidia.nvcf.configuration.exceptions.StorageExceptions; import io.grpc.Metadata; import io.grpc.Status; import io.grpc.Status.Code; @@ -87,7 +88,7 @@ public io.grpc.StatusRuntimeException handleInvalidInvocationException( return status.asRuntimeException(metadata); } - // Keep in sync with ExceptionTracingExceptionHandler; StorageExceptions explains the choice. + // Keep in sync with ExceptionTracingExceptionHandler, which explains the choice. @GrpcExceptionHandler({ CassandraReadTimeoutException.class, CassandraInsufficientReplicasAvailableException.class, @@ -95,7 +96,7 @@ public io.grpc.StatusRuntimeException handleInvalidInvocationException( CassandraConnectionFailureException.class }) public Status handleStorageUnavailable(DataAccessException e) { - return toGrpcStatus(StorageExceptions.storageUnavailable(e)); + return toGrpcStatus(new ServiceUnavailableException(STORAGE_UNAVAILABLE, e)); } @GrpcExceptionHandler diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandler.java b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandler.java index 8c7d7c803e..eb5818cbbe 100644 --- a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandler.java +++ b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/ExceptionTracingExceptionHandler.java @@ -20,6 +20,7 @@ import com.nvidia.boot.exceptions.ConflictException; import com.nvidia.boot.exceptions.ForbiddenException; import com.nvidia.boot.exceptions.NotFoundException; +import com.nvidia.boot.exceptions.ServiceUnavailableException; import com.nvidia.boot.exceptions.TooManyRequestsException; import com.nvidia.boot.exceptions.UnauthorizedException; import com.nvidia.boot.exceptions.handlers.BootMvcExceptionHandler; @@ -39,6 +40,8 @@ public class ExceptionTracingExceptionHandler extends BootMvcExceptionHandler { + public static final String STORAGE_UNAVAILABLE = "Storage is temporarily unavailable"; + @Autowired private Tracer tracer; @@ -136,7 +139,15 @@ protected ResponseEntity handleException( new BadRequestException(ex.getMessage(), ex.getCause()), request); } - // Keep in sync with GrpcConfiguration; StorageExceptions explains the choice. + /** + * Cassandra failures that are safe to report as retriable. + * + *

Only failures where Cassandra did no work are: a coordinator read timeout, too few replicas + * alive, an overloaded or bootstrapping coordinator, or no node reachable. Write timeouts and + * driver timeouts are not, because the write may have been applied and a retry of a create would + * duplicate it. Read and write failures are not transient. Keep in sync with + * {@code GrpcConfiguration}. + */ @ExceptionHandler({ CassandraReadTimeoutException.class, CassandraInsufficientReplicasAvailableException.class, @@ -146,7 +157,8 @@ protected ResponseEntity handleException( protected ResponseEntity handleStorageUnavailable( DataAccessException ex, WebRequest request) throws Exception { - return super.handleException(StorageExceptions.storageUnavailable(ex), request); + return super.handleException( + new ServiceUnavailableException(STORAGE_UNAVAILABLE, ex), request); } } diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/StorageExceptions.java b/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/StorageExceptions.java deleted file mode 100644 index 67e6ea182a..0000000000 --- a/src/control-plane-services/cloud-functions/nvcf-core/src/main/java/com/nvidia/nvcf/configuration/exceptions/StorageExceptions.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.nvidia.nvcf.configuration.exceptions; - -import com.nvidia.boot.exceptions.ServiceUnavailableException; - -/** - * Cassandra failures that are safe to report as retriable. - * - *

Only failures where Cassandra did no work are: a coordinator read timeout, too few replicas - * alive, an overloaded or bootstrapping coordinator, or no node reachable. Write timeouts and - * driver timeouts are not, because the write may have been applied and a retry of a create would - * duplicate it. Read and write failures are not transient. - */ -public final class StorageExceptions { - - public static final String STORAGE_UNAVAILABLE = "Storage is temporarily unavailable"; - - private StorageExceptions() { - } - - public static ServiceUnavailableException storageUnavailable(Throwable cause) { - return new ServiceUnavailableException(STORAGE_UNAVAILABLE, cause); - } -} diff --git a/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/GrpcConfigurationTest.java b/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/GrpcConfigurationTest.java index f796effe46..e785ac04a8 100644 --- a/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/GrpcConfigurationTest.java +++ b/src/control-plane-services/cloud-functions/nvcf-core/src/test/java/com/nvidia/nvcf/configuration/GrpcConfigurationTest.java @@ -19,12 +19,11 @@ import static org.assertj.core.api.Assertions.assertThat; +import com.nvidia.boot.exceptions.ServiceUnavailableException; import com.nvidia.nvcf.configuration.exceptions.ExceptionTracingExceptionHandler; -import com.nvidia.nvcf.configuration.exceptions.StorageExceptions; import io.grpc.Status; import org.junit.jupiter.api.Test; import org.springframework.dao.DataAccessException; -import org.springframework.data.cassandra.CassandraDriverTimeoutException; import org.springframework.data.cassandra.CassandraReadTimeoutException; import org.springframework.grpc.server.advice.GrpcExceptionHandler; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -42,17 +41,16 @@ void retriableStorageFailureIsUnavailable() { new CassandraReadTimeoutException(false, CQL, null)); assertThat(status.getCode()).isEqualTo(Status.Code.UNAVAILABLE); - assertThat(status.getDescription()).isEqualTo(StorageExceptions.STORAGE_UNAVAILABLE); + assertThat(status.getDescription()).isEqualTo("Storage is temporarily unavailable"); } @Test - void indexReadTimeoutIsUnavailable() { + void serviceUnavailableIsUnavailable() { var status = grpcConfiguration.handleErrorResponseException( - StorageExceptions.storageUnavailable( - new CassandraDriverTimeoutException(CQL, null))); + new ServiceUnavailableException("Storage is temporarily unavailable")); assertThat(status.getCode()).isEqualTo(Status.Code.UNAVAILABLE); - assertThat(status.getDescription()).isEqualTo(StorageExceptions.STORAGE_UNAVAILABLE); + assertThat(status.getDescription()).isEqualTo("Storage is temporarily unavailable"); } @Test From c0cc24326cb63240bf603b235a20b03ec7498f9f Mon Sep 17 00:00:00 2001 From: Anand Parthasarathi Date: Thu, 29 Oct 2026 08:22:58 +0530 Subject: [PATCH 7/9] fix(nv-boot): make the range-read retry policy opt-in Move the "retry a read timeout that has data but too few responses" branch out of NextHostRetryPolicy into RangeReadRetryPolicy, which extends it. The base policy goes back to retrying only read timeouts without data, so services that select it no longer also retry single-partition read timeouts. NextHostRetryPolicy gets a protected shouldRetryReadTimeout hook for this. nvcf-service and api-keys select RangeReadRetryPolicy in their ncp profile configs; every other service keeps NextHostRetryPolicy. Signed-off-by: Anand Parthasarathi --- .../src/main/resources/application-ncp.yaml | 5 ++ .../src/main/resources/application-ncp.yaml | 5 ++ .../cassandra/retry/NextHostRetryPolicy.java | 21 +++++-- .../cassandra/retry/RangeReadRetryPolicy.java | 48 ++++++++++++++ .../retry/NextHostRetryPolicyTest.java | 4 +- .../retry/RangeReadRetryPolicyTest.java | 63 +++++++++++++++++++ 6 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/RangeReadRetryPolicy.java create mode 100644 src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/RangeReadRetryPolicyTest.java diff --git a/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml b/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml index 161955a496..1290a31fae 100644 --- a/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml +++ b/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml @@ -18,6 +18,11 @@ spring: contact-points: ${CASSANDRA_CONTACT_POINTS:cassandra.cassandra-system.svc.cluster.local} keyspace-name: api_keys_api local-datacenter: ${CASSANDRA_DATACENTER:ncp} + advanced: + retry-policy: + # Opt-in: also retry range-read (index lookup) timeouts on the next host, which the + # default NextHostRetryPolicy does not do. + class: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy ssl: enabled: false bundle: "" diff --git a/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml b/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml index dfdb58eb4d..49f6415108 100644 --- a/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml +++ b/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml @@ -17,6 +17,11 @@ spring: contact-points: ${CASSANDRA_CONTACT_POINTS:cassandra.cassandra-system.svc.cluster.local} keyspace-name: nvcf_api local-datacenter: ${CASSANDRA_DATACENTER:ncp} + advanced: + retry-policy: + # Opt-in: also retry range-read (index lookup) timeouts on the next host, which the + # default NextHostRetryPolicy does not do. + class: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy username: cassandra password: cassandra security: diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicy.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicy.java index c5711d5dd2..b3b672eb31 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicy.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicy.java @@ -33,7 +33,7 @@ /** * Retry policy that retries on the next host for transient failures (connection/heartbeat, - * read timeout without data or with too few responses, write timeout, unavailable). Rethrows for ReadFailure, WriteFailure, + * read timeout without data, write timeout, unavailable). Rethrows for ReadFailure, WriteFailure, * and after one retry attempt. */ @Slf4j @@ -68,15 +68,26 @@ public RetryVerdict onReadTimeoutVerdict( int received, boolean dataPresent, int retryCount) { - // A range read (secondary index) always has data present, from the coordinator's own - // replica, so a timeout there only shows as fewer responses than required. - var verdict = retryCount == 0 && (!dataPresent || received < blockFor) ? - RetryVerdict.RETRY_NEXT : RetryVerdict.RETHROW; + var verdict = shouldRetryReadTimeout(blockFor, received, dataPresent, retryCount) + ? RetryVerdict.RETRY_NEXT : RetryVerdict.RETHROW; log.info(VERDICT_ON_READ_TIMEOUT, logPrefix, cl, blockFor, received, dataPresent, retryCount, verdict); return verdict; } + /** + * Decides whether a read timeout is retried on the next host. By default only a first + * attempt that timed out before the coordinator had any data is retried. Subclasses can + * widen this for other kinds of reads; see {@link RangeReadRetryPolicy}. + */ + protected boolean shouldRetryReadTimeout( + int blockFor, + int received, + boolean dataPresent, + int retryCount) { + return retryCount == 0 && !dataPresent; + } + @Override public RetryVerdict onWriteTimeoutVerdict( Request request, diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/RangeReadRetryPolicy.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/RangeReadRetryPolicy.java new file mode 100644 index 0000000000..8b7e1b1608 --- /dev/null +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/main/java/com/nvidia/boot/cassandra/retry/RangeReadRetryPolicy.java @@ -0,0 +1,48 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.nvidia.boot.cassandra.retry; + +import com.datastax.oss.driver.api.core.context.DriverContext; + +/** + * {@link NextHostRetryPolicy} that also retries a first read timeout when the coordinator had + * data but fewer responses than required. + * + *

A range read (a secondary or SAI index lookup) always has data present from the + * coordinator's own replica, so when a replica is down the timeout only shows as fewer + * responses than required, never as missing data. Cassandra does not retry range reads on + * another replica, so this policy retries them on the next host from the client. + * + *

This is opt-in. {@link NextHostRetryPolicy}, which most services use, does not retry + * these timeouts, because that would also retry single-partition read timeouts. Select this + * policy per service with {@code spring.cassandra.advanced.retry-policy.class}. + */ +public class RangeReadRetryPolicy extends NextHostRetryPolicy { + + public RangeReadRetryPolicy(DriverContext context, String profileName) { + super(context, profileName); + } + + @Override + protected boolean shouldRetryReadTimeout( + int blockFor, + int received, + boolean dataPresent, + int retryCount) { + return retryCount == 0 && (!dataPresent || received < blockFor); + } +} diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicyTest.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicyTest.java index 92bfa92875..b8637c0ca0 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicyTest.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/NextHostRetryPolicyTest.java @@ -55,9 +55,7 @@ class NextHostRetryPolicyTest extends RetryPolicyTestBase { void shouldProcessReadTimeouts() { assertOnReadTimeout(QUORUM, 2, 2, false, 1).hasDecision(RETHROW); assertOnReadTimeout(QUORUM, 2, 1, false, 0).hasDecision(RETRY_NEXT); - assertOnReadTimeout(QUORUM, 2, 1, true, 0).hasDecision(RETRY_NEXT); - assertOnReadTimeout(QUORUM, 2, 1, true, 1).hasDecision(RETHROW); - assertOnReadTimeout(QUORUM, 2, 2, true, 0).hasDecision(RETHROW); + assertOnReadTimeout(QUORUM, 2, 1, true, 0).hasDecision(RETHROW); } @Test diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/RangeReadRetryPolicyTest.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/RangeReadRetryPolicyTest.java new file mode 100644 index 0000000000..a06b4a0bcd --- /dev/null +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/retry/RangeReadRetryPolicyTest.java @@ -0,0 +1,63 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.nvidia.boot.cassandra.retry; + +import static com.datastax.oss.driver.api.core.ConsistencyLevel.QUORUM; +import static com.datastax.oss.driver.api.core.retry.RetryDecision.RETHROW; +import static com.datastax.oss.driver.api.core.retry.RetryDecision.RETRY_NEXT; +import static com.datastax.oss.driver.api.core.servererrors.WriteType.UNLOGGED_BATCH; + +import com.datastax.oss.driver.api.core.connection.ClosedConnectionException; +import com.datastax.oss.driver.api.core.servererrors.ReadFailureException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class RangeReadRetryPolicyTest extends RetryPolicyTestBase { + + RangeReadRetryPolicyTest() { + super(new RangeReadRetryPolicy(null, "testSession1")); + } + + @Test + void shouldRetryReadTimeoutWithDataButTooFewResponses() { + // A range read has data from the coordinator's own replica, but a replica is down. + assertOnReadTimeout(QUORUM, 2, 1, true, 0).hasDecision(RETRY_NEXT); + assertOnReadTimeout(QUORUM, 2, 1, true, 1).hasDecision(RETHROW); + } + + @Test + void shouldNotRetryReadTimeoutWhenEnoughResponsesArrived() { + assertOnReadTimeout(QUORUM, 2, 2, true, 0).hasDecision(RETHROW); + } + + @Test + void shouldKeepBaseBehaviourForReadTimeoutsWithoutData() { + assertOnReadTimeout(QUORUM, 2, 2, false, 1).hasDecision(RETHROW); + assertOnReadTimeout(QUORUM, 2, 1, false, 0).hasDecision(RETRY_NEXT); + } + + @Test + void shouldInheritTheOtherVerdictsFromNextHostRetryPolicy() { + assertOnWriteTimeout(QUORUM, UNLOGGED_BATCH, 2, 1, 0).hasDecision(RETRY_NEXT); + assertOnWriteTimeout(QUORUM, UNLOGGED_BATCH, 2, 1, 1).hasDecision(RETHROW); + assertOnUnavailable(QUORUM, 2, 1, 0).hasDecision(RETRY_NEXT); + assertOnRequestAborted(ClosedConnectionException.class, 0).hasDecision(RETRY_NEXT); + assertOnErrorResponse(ReadFailureException.class, 0).hasDecision(RETHROW); + } +} From f0f79802bb536dde90d730d832e209d9782c255f Mon Sep 17 00:00:00 2001 From: Anand Parthasarathi Date: Thu, 29 Oct 2026 09:21:12 +0530 Subject: [PATCH 8/9] docs(nv-boot): document RangeReadRetryPolicy as a separate opt-in knob Keep NextHostRetryPolicy as the default everywhere (drop the ncp profile selection) and document how to enable RangeReadRetryPolicy through SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS, independent of highAvailability.mode. Add a test that the env var resolves to the driver retry-policy option. Signed-off-by: Anand Parthasarathi --- docs/self-managed/high-availability.md | 26 +++++++++++++++++++ .../src/main/resources/application-ncp.yaml | 5 ---- .../src/main/resources/application-ncp.yaml | 5 ---- .../nv-boot-starter-cassandra/README.md | 14 ++++++++++ .../CassandraAdvancedConfigurationTest.java | 21 +++++++++++++++ 5 files changed, 61 insertions(+), 10 deletions(-) diff --git a/docs/self-managed/high-availability.md b/docs/self-managed/high-availability.md index e24f3c3eda..4e1fd1402b 100644 --- a/docs/self-managed/high-availability.md +++ b/docs/self-managed/high-availability.md @@ -317,6 +317,32 @@ The control-plane services default to `LOCAL_QUORUM` consistency, and some [Zone spread for the quorum services](#zone-spread-for-the-quorum-services)) and does not hold above three pods. +#### Optional: extra Cassandra read resilience (`RangeReadRetryPolicy`) + +When one Cassandra pod dies, index-backed reads (for example listing function +versions) can fail for a few seconds until the other replicas stop treating +the dead node as alive. By default `nvcf-api` and `api-keys` use +`NextHostRetryPolicy`, which retries a read timeout on the next replica only +when no data came back. + +If you need to tune additional Cassandra resilience, enable +`RangeReadRetryPolicy`. It also retries a read timeout that has data but too +few responses, once, on the next replica. It is a separate knob: it is **off by +default** and is **not** set by `highAvailability.mode`. Enable it per service +with the driver retry-policy class, through the operator env hooks in your +environment file: + +```yaml +api: + env: + SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy +apikeys: + env: + SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy +``` + +Remove the entries to return to the default. + ## Tuning scheduling policy (`global.affinity` / `global.topologySpreadConstraints`) Affinity and zone topology spread are shared scheduling policy, so you can diff --git a/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml b/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml index 1290a31fae..161955a496 100644 --- a/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml +++ b/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml @@ -18,11 +18,6 @@ spring: contact-points: ${CASSANDRA_CONTACT_POINTS:cassandra.cassandra-system.svc.cluster.local} keyspace-name: api_keys_api local-datacenter: ${CASSANDRA_DATACENTER:ncp} - advanced: - retry-policy: - # Opt-in: also retry range-read (index lookup) timeouts on the next host, which the - # default NextHostRetryPolicy does not do. - class: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy ssl: enabled: false bundle: "" diff --git a/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml b/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml index 49f6415108..dfdb58eb4d 100644 --- a/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml +++ b/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml @@ -17,11 +17,6 @@ spring: contact-points: ${CASSANDRA_CONTACT_POINTS:cassandra.cassandra-system.svc.cluster.local} keyspace-name: nvcf_api local-datacenter: ${CASSANDRA_DATACENTER:ncp} - advanced: - retry-policy: - # Opt-in: also retry range-read (index lookup) timeouts on the next host, which the - # default NextHostRetryPolicy does not do. - class: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy username: cassandra password: cassandra security: diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/README.md b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/README.md index ebbe216656..4ae6119933 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/README.md +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/README.md @@ -239,6 +239,20 @@ This requires: |------|------|-------------| | `advancedProperties` | `DriverConfigLoaderBuilderCustomizer` (`@Primary`) | Binds `spring.cassandra.advanced.*` properties to the DataStax Java Driver. | +#### Retry policies + +Select a retry policy with `spring.cassandra.advanced.retry-policy.class` +(env var `SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS`). + +| Class | Behavior | +|-------|----------| +| `com.nvidia.boot.cassandra.retry.NextHostRetryPolicy` (default in the services) | Retries once on the next host: read timeout without data, write timeout, unavailable, aborted request, error response. | +| `com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy` (opt-in) | Extends `NextHostRetryPolicy` and also retries once on the next host a read timeout that has data but fewer responses than required (range reads / index lookups). | + +`RangeReadRetryPolicy` is not enabled anywhere by default. Enable it when you +need additional Cassandra resilience, for example to ride out the loss of a +replica. + ### CassandraHealthConfiguration | Bean | Type | Description | diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/configuration/CassandraAdvancedConfigurationTest.java b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/configuration/CassandraAdvancedConfigurationTest.java index fa82e0f393..6c519ea2b5 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/configuration/CassandraAdvancedConfigurationTest.java +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/src/test/java/com/nvidia/boot/cassandra/configuration/CassandraAdvancedConfigurationTest.java @@ -20,7 +20,10 @@ import static org.assertj.core.api.Assertions.assertThat; import com.datastax.oss.driver.api.core.config.DriverConfigLoader; +import java.util.Map; import org.junit.jupiter.api.Test; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.core.env.SystemEnvironmentPropertySource; import org.springframework.mock.env.MockEnvironment; class CassandraAdvancedConfigurationTest { @@ -80,4 +83,22 @@ void advancedPropertiesIgnoresBlankProperties() { customizer.customize(builder); assertThat(builder.build()).isNotNull(); } + + @Test + void retryPolicyClassCanBeSelectedFromEnvironmentVariable() { + var env = new StandardEnvironment(); + env.getPropertySources() + .addFirst(new SystemEnvironmentPropertySource( + "test-env", + Map.of( + "SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS", + "com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy"))); + + var builder = DriverConfigLoader.programmaticBuilder(); + config.advancedProperties(env).customize(builder); + var profile = builder.build().getInitialConfig().getDefaultProfile(); + + assertThat(profile.getString(com.datastax.oss.driver.api.core.config.DefaultDriverOption.RETRY_POLICY_CLASS)) + .isEqualTo("com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy"); + } } From 3df298a5617f48d8b53ee9ab4d501d94c8867e96 Mon Sep 17 00:00:00 2001 From: Anand Parthasarathi Date: Thu, 29 Oct 2026 09:28:30 +0530 Subject: [PATCH 9/9] feat(nvcf-api,api-keys): select RangeReadRetryPolicy in the ncp profile Per review: the widened range-read retry applies only where the ncp profile selects it. Operators can still override it with SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS. Signed-off-by: Anand Parthasarathi --- docs/self-managed/high-availability.md | 24 ++++++++----------- .../src/main/resources/application-ncp.yaml | 5 ++++ .../src/main/resources/application-ncp.yaml | 5 ++++ .../nv-boot-starter-cassandra/README.md | 8 +++---- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/docs/self-managed/high-availability.md b/docs/self-managed/high-availability.md index 4e1fd1402b..c93cb98382 100644 --- a/docs/self-managed/high-availability.md +++ b/docs/self-managed/high-availability.md @@ -317,32 +317,28 @@ The control-plane services default to `LOCAL_QUORUM` consistency, and some [Zone spread for the quorum services](#zone-spread-for-the-quorum-services)) and does not hold above three pods. -#### Optional: extra Cassandra read resilience (`RangeReadRetryPolicy`) +#### Cassandra read retry policy (`RangeReadRetryPolicy`) When one Cassandra pod dies, index-backed reads (for example listing function versions) can fail for a few seconds until the other replicas stop treating -the dead node as alive. By default `nvcf-api` and `api-keys` use -`NextHostRetryPolicy`, which retries a read timeout on the next replica only -when no data came back. +the dead node as alive. The default `NextHostRetryPolicy` retries a read +timeout on the next replica only when no data came back. -If you need to tune additional Cassandra resilience, enable -`RangeReadRetryPolicy`. It also retries a read timeout that has data but too -few responses, once, on the next replica. It is a separate knob: it is **off by -default** and is **not** set by `highAvailability.mode`. Enable it per service -with the driver retry-policy class, through the operator env hooks in your -environment file: +The self-managed `nvcf-api` and `api-keys` services (ncp profile) select +`RangeReadRetryPolicy`, which also retries once, on the next replica, a read +timeout that has data but too few responses. This is independent of +`highAvailability.mode`. To return to the default, or to tune resilience +further, override the driver retry-policy class through the operator env hooks: ```yaml api: env: - SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy + SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS: com.nvidia.boot.cassandra.retry.NextHostRetryPolicy apikeys: env: - SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy + SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS: com.nvidia.boot.cassandra.retry.NextHostRetryPolicy ``` -Remove the entries to return to the default. - ## Tuning scheduling policy (`global.affinity` / `global.topologySpreadConstraints`) Affinity and zone topology spread are shared scheduling policy, so you can diff --git a/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml b/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml index 161955a496..3a99885400 100644 --- a/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml +++ b/src/control-plane-services/api-keys/src/main/resources/application-ncp.yaml @@ -18,6 +18,11 @@ spring: contact-points: ${CASSANDRA_CONTACT_POINTS:cassandra.cassandra-system.svc.cluster.local} keyspace-name: api_keys_api local-datacenter: ${CASSANDRA_DATACENTER:ncp} + advanced: + retry-policy: + # Also retry range-read (index lookup) timeouts on the next host. Override with + # SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS to go back to NextHostRetryPolicy. + class: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy ssl: enabled: false bundle: "" diff --git a/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml b/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml index dfdb58eb4d..da34f148d8 100644 --- a/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml +++ b/src/control-plane-services/cloud-functions/nvcf-service/src/main/resources/application-ncp.yaml @@ -17,6 +17,11 @@ spring: contact-points: ${CASSANDRA_CONTACT_POINTS:cassandra.cassandra-system.svc.cluster.local} keyspace-name: nvcf_api local-datacenter: ${CASSANDRA_DATACENTER:ncp} + advanced: + retry-policy: + # Also retry range-read (index lookup) timeouts on the next host. Override with + # SPRING_CASSANDRA_ADVANCED_RETRY_POLICY_CLASS to go back to NextHostRetryPolicy. + class: com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy username: cassandra password: cassandra security: diff --git a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/README.md b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/README.md index 4ae6119933..9c85cc5ece 100644 --- a/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/README.md +++ b/src/libraries/java/nv-boot-parent/nv-boot-starter-cassandra/README.md @@ -247,11 +247,11 @@ Select a retry policy with `spring.cassandra.advanced.retry-policy.class` | Class | Behavior | |-------|----------| | `com.nvidia.boot.cassandra.retry.NextHostRetryPolicy` (default in the services) | Retries once on the next host: read timeout without data, write timeout, unavailable, aborted request, error response. | -| `com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy` (opt-in) | Extends `NextHostRetryPolicy` and also retries once on the next host a read timeout that has data but fewer responses than required (range reads / index lookups). | +| `com.nvidia.boot.cassandra.retry.RangeReadRetryPolicy` (selected by the nvcf-service and api-keys ncp profiles) | Extends `NextHostRetryPolicy` and also retries once on the next host a read timeout that has data but fewer responses than required (range reads / index lookups). | -`RangeReadRetryPolicy` is not enabled anywhere by default. Enable it when you -need additional Cassandra resilience, for example to ride out the loss of a -replica. +Other services keep `NextHostRetryPolicy`. Select `RangeReadRetryPolicy` in a +service when you need additional Cassandra resilience, for example to ride out +the loss of a replica. ### CassandraHealthConfiguration