This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 424cc10401 [#12341] improvement(server): define optimistic-lock
conflict response (#12349)
424cc10401 is described below
commit 424cc10401c1aadfc12974e304ab569e78a8d976
Author: Qi Yu <[email protected]>
AuthorDate: Tue Aug 4 16:05:23 2026 +0800
[#12341] improvement(server): define optimistic-lock conflict response
(#12349)
### What changes were proposed in this pull request?
- Add `OptimisticLockException` as the shared optimistic-concurrency
conflict contract.
- Add error code 1012 and an optimistic-lock error response.
- Map optimistic-lock conflicts to HTTP 409.
- Add response, server utility, and REST exception-handler tests.
This PR only defines the shared contract. Entity-specific CAS behavior
will be implemented by the follow-up subtasks of #12166.
### Why are the changes needed?
Entity write paths need a typed conflict signal instead of relying on
generic exceptions or matching error-message prefixes. A shared
exception and REST mapping allow later entity PRs to report lost CAS
operations consistently.
Fix: #12341
### Does this PR introduce _any_ user-facing change?
Yes. It introduces the public `OptimisticLockException` type and error
code 1012. REST operations that throw this exception are returned as
HTTP 409 Conflict.
### How was this patch tested?
- `./gradlew spotlessApply`
- `./gradlew :common:test --tests
'org.apache.gravitino.dto.responses.TestResponses' :server-common:test
--tests 'org.apache.gravitino.server.web.TestUtils' :server:test --tests
'org.apache.gravitino.server.web.rest.TestExceptionHandlers' -PskipITs
-PskipDockerTests=true`
---
.../exceptions/OptimisticLockException.java | 53 ++++++++++++++++++++++
.../gravitino/dto/responses/ErrorConstants.java | 3 ++
.../gravitino/dto/responses/ErrorResponse.java | 14 ++++++
.../gravitino/dto/responses/TestResponses.java | 10 ++++
.../org/apache/gravitino/server/web/Utils.java | 16 +++++++
.../org/apache/gravitino/server/web/TestUtils.java | 16 +++++++
.../server/web/rest/ExceptionHandlers.java | 6 +++
.../server/web/rest/TestExceptionHandlers.java | 19 ++++++++
8 files changed, 137 insertions(+)
diff --git
a/api/src/main/java/org/apache/gravitino/exceptions/OptimisticLockException.java
b/api/src/main/java/org/apache/gravitino/exceptions/OptimisticLockException.java
new file mode 100644
index 0000000000..34451063e6
--- /dev/null
+++
b/api/src/main/java/org/apache/gravitino/exceptions/OptimisticLockException.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.gravitino.exceptions;
+
+import com.google.errorprone.annotations.FormatMethod;
+import com.google.errorprone.annotations.FormatString;
+
+/**
+ * Exception thrown when an optimistic-lock update fails because the entity
changed after it was
+ * read.
+ */
+public class OptimisticLockException extends GravitinoRuntimeException {
+ private static final long serialVersionUID = 1L;
+
+ /**
+ * Constructs a new exception with the specified detail message.
+ *
+ * @param message the detail message.
+ * @param args the arguments to the message.
+ */
+ @FormatMethod
+ public OptimisticLockException(@FormatString String message, Object... args)
{
+ super(message, args);
+ }
+
+ /**
+ * Constructs a new exception with the specified detail message and cause.
+ *
+ * @param cause the cause.
+ * @param message the detail message.
+ * @param args the arguments to the message.
+ */
+ @FormatMethod
+ public OptimisticLockException(Throwable cause, @FormatString String
message, Object... args) {
+ super(cause, message, args);
+ }
+}
diff --git
a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorConstants.java
b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorConstants.java
index 870addfba9..8b94b0df3e 100644
---
a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorConstants.java
+++
b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorConstants.java
@@ -57,6 +57,9 @@ public class ErrorConstants {
/** Error codes for unauthorized access. */
public static final int UNAUTHORIZED_CODE = 1011;
+ /** Error codes for optimistic-lock conflicts. */
+ public static final int OPTIMISTIC_LOCK_CONFLICT_CODE = 1012;
+
/** Error codes for invalid state. */
public static final int UNKNOWN_ERROR_CODE = 1100;
diff --git
a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
index a47d4cbb80..df5afc81a4 100644
--- a/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
+++ b/common/src/main/java/org/apache/gravitino/dto/responses/ErrorResponse.java
@@ -234,6 +234,20 @@ public class ErrorResponse extends BaseResponse {
ErrorConstants.ALREADY_EXISTS_CODE, type, message,
getStackTrace(throwable));
}
+ /**
+ * Create a new optimistic-lock conflict error instance of {@link
ErrorResponse}.
+ *
+ * @param type The type of the error.
+ * @param message The message of the error.
+ * @param throwable The throwable that caused the error.
+ * @return The new instance.
+ */
+ public static ErrorResponse optimisticLockConflict(
+ String type, String message, Throwable throwable) {
+ return new ErrorResponse(
+ ErrorConstants.OPTIMISTIC_LOCK_CONFLICT_CODE, type, message,
getStackTrace(throwable));
+ }
+
/**
* Create a new not in use error instance of {@link ErrorResponse}.
*
diff --git
a/common/src/test/java/org/apache/gravitino/dto/responses/TestResponses.java
b/common/src/test/java/org/apache/gravitino/dto/responses/TestResponses.java
index 89269ab966..67f2c43659 100644
--- a/common/src/test/java/org/apache/gravitino/dto/responses/TestResponses.java
+++ b/common/src/test/java/org/apache/gravitino/dto/responses/TestResponses.java
@@ -244,6 +244,16 @@ public class TestResponses {
error.validate(); // No exception thrown
}
+ @Test
+ void testOptimisticLockConflictErrorResponse() throws
IllegalArgumentException {
+ ErrorResponse error =
+ ErrorResponse.optimisticLockConflict(
+ "OptimisticLockException", "optimistic lock conflict", null);
+ error.validate(); // No exception thrown
+ assertEquals(ErrorConstants.OPTIMISTIC_LOCK_CONFLICT_CODE,
error.getCode());
+ assertEquals("OptimisticLockException", error.getType());
+ }
+
@Test
void testNonEmptyErrorResponse() throws IllegalArgumentException {
ErrorResponse error = ErrorResponse.nonEmpty("error type", "non empty
error");
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/web/Utils.java
b/server-common/src/main/java/org/apache/gravitino/server/web/Utils.java
index 4ef45b6a50..21338047d6 100644
--- a/server-common/src/main/java/org/apache/gravitino/server/web/Utils.java
+++ b/server-common/src/main/java/org/apache/gravitino/server/web/Utils.java
@@ -136,6 +136,22 @@ public class Utils {
.build();
}
+ /**
+ * Creates an HTTP conflict response for a failed optimistic-lock update.
+ *
+ * @param message the conflict message
+ * @param throwable the conflict exception
+ * @return the HTTP conflict response
+ */
+ public static Response optimisticLockConflict(String message, Throwable
throwable) {
+ return Response.status(Response.Status.CONFLICT)
+ .entity(
+ ErrorResponse.optimisticLockConflict(
+ throwable.getClass().getSimpleName(), message, throwable))
+ .type(MediaType.APPLICATION_JSON)
+ .build();
+ }
+
public static Response notInUse(String message, Throwable throwable) {
return notInUse(throwable.getClass().getSimpleName(), message, throwable);
}
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/web/TestUtils.java
b/server-common/src/test/java/org/apache/gravitino/server/web/TestUtils.java
index c495a6275b..3f276c8316 100644
--- a/server-common/src/test/java/org/apache/gravitino/server/web/TestUtils.java
+++ b/server-common/src/test/java/org/apache/gravitino/server/web/TestUtils.java
@@ -30,7 +30,9 @@ import javax.ws.rs.core.Response;
import org.apache.gravitino.audit.FilesetAuditConstants;
import org.apache.gravitino.audit.FilesetDataOperation;
import org.apache.gravitino.audit.InternalClientType;
+import org.apache.gravitino.dto.responses.ErrorConstants;
import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.apache.gravitino.exceptions.OptimisticLockException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
@@ -164,6 +166,20 @@ public class TestUtils {
assertEquals("New message", errorResponse.getMessage());
}
+ @Test
+ public void testOptimisticLockConflict() {
+ OptimisticLockException exception = new
OptimisticLockException("Conflict");
+ Response response = Utils.optimisticLockConflict("Conflict", exception);
+
+ assertNotNull(response);
+ assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ assertEquals(MediaType.APPLICATION_JSON,
response.getMediaType().toString());
+ ErrorResponse errorResponse = (ErrorResponse) response.getEntity();
+ assertEquals(ErrorConstants.OPTIMISTIC_LOCK_CONFLICT_CODE,
errorResponse.getCode());
+ assertEquals(OptimisticLockException.class.getSimpleName(),
errorResponse.getType());
+ assertEquals("Conflict", errorResponse.getMessage());
+ }
+
@Test
public void testUnsupportedOperation() {
Response response = Utils.unsupportedOperation("Unsupported operation");
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
index 6b550fd869..a3fb6e0a99 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
@@ -43,6 +43,7 @@ import
org.apache.gravitino.exceptions.NonEmptyMetalakeException;
import org.apache.gravitino.exceptions.NonEmptySchemaException;
import org.apache.gravitino.exceptions.NotFoundException;
import org.apache.gravitino.exceptions.NotInUseException;
+import org.apache.gravitino.exceptions.OptimisticLockException;
import org.apache.gravitino.exceptions.PartitionAlreadyExistsException;
import org.apache.gravitino.exceptions.PolicyAlreadyAssociatedException;
import org.apache.gravitino.exceptions.PolicyAlreadyExistsException;
@@ -1105,6 +1106,11 @@ public class ExceptionHandlers {
return Utils.connectionFailed(errorMsg, e);
}
+ if (e instanceof OptimisticLockException) {
+ LOG.warn(errorMsg, e);
+ return Utils.optimisticLockConflict(errorMsg, e);
+ }
+
LOG.error(errorMsg, e);
return Utils.internalError(errorMsg, e);
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
index bdc43c4fc2..ae5401af36 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestExceptionHandlers.java
@@ -18,6 +18,10 @@
*/
package org.apache.gravitino.server.web.rest;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.dto.responses.ErrorConstants;
+import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.apache.gravitino.exceptions.OptimisticLockException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -50,4 +54,19 @@ public class TestExceptionHandlers {
String msg6 = ExceptionHandlers.BaseExceptionHandler.getErrorMsg(e6);
Assertions.assertEquals("", msg6);
}
+
+ @Test
+ public void testOptimisticLockConflictReturnsConflict() {
+ Response response =
+ ExceptionHandlers.handleTableException(
+ OperationType.ALTER,
+ "table",
+ "schema",
+ new OptimisticLockException("The table was modified
concurrently"));
+
+ Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ ErrorResponse errorResponse = (ErrorResponse) response.getEntity();
+ Assertions.assertEquals(ErrorConstants.OPTIMISTIC_LOCK_CONFLICT_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(OptimisticLockException.class.getSimpleName(),
errorResponse.getType());
+ }
}
