[#3976] improvement(core): Improve the exception behavior in entity s…

…tore (#3979)

### What changes were proposed in this pull request?

Improve the throwing behavior of some SQL exceptions in EntityStore.

### Why are the changes needed?

Fix: #3976 

### How was this patch tested?

Through the existing tests.

Co-authored-by: xiaojiebao <[email protected]>

core/src/main/java/com/datastrato/gravitino/EntityAlreadyExistsException.java

@@ -4,28 +4,37 @@
  */
 package com.datastrato.gravitino;
 
+import com.datastrato.gravitino.exceptions.GravitinoRuntimeException;
+import com.google.errorprone.annotations.FormatMethod;
+import com.google.errorprone.annotations.FormatString;
+
 /**
  * Exception class indicating that an entity already exists. This exception is thrown when an
  * attempt is made to create an entity that already exists within the Gravitino framework.
  */
-public class EntityAlreadyExistsException extends RuntimeException {
+public class EntityAlreadyExistsException extends GravitinoRuntimeException {
 
   /**
    * Constructs an EntityAlreadyExistsException.
    *
-   * @param message The detail message explaining the exception.
+   * @param message the detail message.
+   * @param args the arguments to the message.
    */
-  public EntityAlreadyExistsException(String message) {
-    super(message);
+  @FormatMethod
+  public EntityAlreadyExistsException(@FormatString String message, Object... args) {
+    super(message, args);
   }
 
   /**
    * Constructs an EntityAlreadyExistsException.
    *
-   * @param message The detail message explaining the exception.
-   * @param cause The cause of the exception.
+   * @param cause the cause.
+   * @param message the detail message.
+   * @param args the arguments to the message.
    */
-  public EntityAlreadyExistsException(String message, Throwable cause) {
-    super(message, cause);
+  @FormatMethod
+  public EntityAlreadyExistsException(
+      Throwable cause, @FormatString String message, Object... args) {
+    super(cause, message, args);
   }
 }

core/src/main/java/com/datastrato/gravitino/EntityStore.java

@@ -5,7 +5,6 @@
 package com.datastrato.gravitino;
 
 import com.datastrato.gravitino.Entity.EntityType;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
 import com.datastrato.gravitino.exceptions.NoSuchEntityException;
 import com.datastrato.gravitino.utils.Executable;
 import java.io.Closeable;
@@ -110,11 +109,11 @@ <E extends Entity & HasIdentifier> void put(E e, boolean overwritten)
    * @return E the updated entity
    * @throws IOException if the store operation fails
    * @throws NoSuchEntityException if the entity does not exist
-   * @throws AlreadyExistsException if the updated entity already existed.
+   * @throws EntityAlreadyExistsException if the updated entity already existed.
    */
   <E extends Entity & HasIdentifier> E update(
       NameIdentifier ident, Class<E> type, EntityType entityType, Function<E, E> updater)
-      throws IOException, NoSuchEntityException, AlreadyExistsException;
+      throws IOException, NoSuchEntityException, EntityAlreadyExistsException;
 
   /**
    * Get the entity from the underlying storage.

core/src/main/java/com/datastrato/gravitino/storage/kv/KvBackend.java

@@ -6,7 +6,7 @@
 package com.datastrato.gravitino.storage.kv;
 
 import com.datastrato.gravitino.Config;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
+import com.datastrato.gravitino.EntityAlreadyExistsException;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.List;
@@ -29,9 +29,10 @@ public interface KvBackend extends Closeable {
    * @param value The value of the pair.
    * @param overwrite If true, overwrites the existing value.
    * @throws IOException If an I/O exception occurs during the operation.
-   * @throws AlreadyExistsException If the key already exists and overwrite is false.
+   * @throws EntityAlreadyExistsException If the key already exists and overwrite is false.
    */
-  void put(byte[] key, byte[] value, boolean overwrite) throws IOException, AlreadyExistsException;
+  void put(byte[] key, byte[] value, boolean overwrite)
+      throws IOException, EntityAlreadyExistsException;
 
   /**
    * Retrieves the value associated with a given key.

core/src/main/java/com/datastrato/gravitino/storage/kv/KvEntityStore.java

@@ -24,7 +24,6 @@
 import com.datastrato.gravitino.HasIdentifier;
 import com.datastrato.gravitino.NameIdentifier;
 import com.datastrato.gravitino.Namespace;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
 import com.datastrato.gravitino.exceptions.NoSuchEntityException;
 import com.datastrato.gravitino.exceptions.NonEmptyEntityException;
 import com.datastrato.gravitino.storage.EntityKeyEncoder;
@@ -168,7 +167,7 @@ public <E extends Entity & HasIdentifier> void put(E e, boolean overwritten)
   @Override
   public <E extends Entity & HasIdentifier> E update(
       NameIdentifier ident, Class<E> type, EntityType entityType, Function<E, E> updater)
-      throws IOException, NoSuchEntityException, AlreadyExistsException {
+      throws IOException, NoSuchEntityException, EntityAlreadyExistsException {
     return executeInTransaction(
         () -> {
           byte[] key = entityKeyEncoder.encode(ident, entityType);
@@ -188,7 +187,7 @@ public <E extends Entity & HasIdentifier> E update(
           // Check whether the new entities already existed
           boolean newEntityExist = exists(updatedE.nameIdentifier(), entityType);
           if (newEntityExist) {
-            throw new AlreadyExistsException(
+            throw new EntityAlreadyExistsException(
                 "Entity %s already exist, please check again", updatedE.nameIdentifier());
           }
 

core/src/main/java/com/datastrato/gravitino/storage/kv/RocksDBKvBackend.java

@@ -7,7 +7,7 @@
 
 import com.datastrato.gravitino.Config;
 import com.datastrato.gravitino.Configs;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
+import com.datastrato.gravitino.EntityAlreadyExistsException;
 import com.datastrato.gravitino.utils.ByteUtils;
 import com.datastrato.gravitino.utils.Bytes;
 import com.google.common.annotations.VisibleForTesting;
@@ -95,7 +95,7 @@ public void initialize(Config config) throws IOException {
   public void put(byte[] key, byte[] value, boolean overwrite) throws IOException {
     try {
       handlePut(key, value, overwrite);
-    } catch (AlreadyExistsException e) {
+    } catch (EntityAlreadyExistsException e) {
       throw e;
     } catch (Exception e) {
       throw new IOException(e);
@@ -110,7 +110,7 @@ void handlePut(byte[] key, byte[] value, boolean overwrite) throws RocksDBExcept
     }
     byte[] existKey = db.get(key);
     if (existKey != null) {
-      throw new AlreadyExistsException(
+      throw new EntityAlreadyExistsException(
           "Key %s already exists in the database, please use overwrite option to overwrite it",
           ByteUtils.formatByteArray(key));
     }

core/src/main/java/com/datastrato/gravitino/storage/kv/TransactionalKvBackendImpl.java

@@ -142,7 +142,7 @@ public void put(byte[] key, byte[] value, boolean overwrite)
       throws IOException, EntityAlreadyExistsException {
     byte[] oldValue = get(key);
     if (oldValue != null && !overwrite) {
-      throw new EntityAlreadyExistsException("Key already exists: " + Bytes.wrap(key));
+      throw new EntityAlreadyExistsException("Key already exists: %s", Bytes.wrap(key));
     }
     putPairs
         .get()

core/src/main/java/com/datastrato/gravitino/storage/relational/JDBCBackend.java

@@ -15,7 +15,6 @@
 import com.datastrato.gravitino.NameIdentifier;
 import com.datastrato.gravitino.Namespace;
 import com.datastrato.gravitino.UnsupportedEntityTypeException;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
 import com.datastrato.gravitino.exceptions.NoSuchEntityException;
 import com.datastrato.gravitino.meta.BaseMetalake;
 import com.datastrato.gravitino.meta.CatalogEntity;
@@ -69,7 +68,7 @@ public void initialize(Config config) {
 
   @Override
   public <E extends Entity & HasIdentifier> List<E> list(
-      Namespace namespace, Entity.EntityType entityType) {
+      Namespace namespace, Entity.EntityType entityType) throws IOException {
     switch (entityType) {
       case METALAKE:
         return (List<E>) MetalakeMetaService.getInstance().listMetalakes();
@@ -90,7 +89,7 @@ public <E extends Entity & HasIdentifier> List<E> list(
   }
 
   @Override
-  public boolean exists(NameIdentifier ident, Entity.EntityType entityType) {
+  public boolean exists(NameIdentifier ident, Entity.EntityType entityType) throws IOException {
     try {
       Entity entity = get(ident, entityType);
       return entity != null;
@@ -101,7 +100,7 @@ public boolean exists(NameIdentifier ident, Entity.EntityType entityType) {
 
   @Override
   public <E extends Entity & HasIdentifier> void insert(E e, boolean overwritten)
-      throws EntityAlreadyExistsException {
+      throws EntityAlreadyExistsException, IOException {
     if (e instanceof BaseMetalake) {
       MetalakeMetaService.getInstance().insertMetalake((BaseMetalake) e, overwritten);
     } else if (e instanceof CatalogEntity) {
@@ -129,7 +128,7 @@ public <E extends Entity & HasIdentifier> void insert(E e, boolean overwritten)
   @Override
   public <E extends Entity & HasIdentifier> E update(
       NameIdentifier ident, Entity.EntityType entityType, Function<E, E> updater)
-      throws IOException, NoSuchEntityException, AlreadyExistsException {
+      throws IOException, NoSuchEntityException, EntityAlreadyExistsException {
     switch (entityType) {
       case METALAKE:
         return (E) MetalakeMetaService.getInstance().updateMetalake(ident, updater);
@@ -155,7 +154,8 @@ public <E extends Entity & HasIdentifier> E update(
 
   @Override
   public <E extends Entity & HasIdentifier> E get(
-      NameIdentifier ident, Entity.EntityType entityType) throws NoSuchEntityException {
+      NameIdentifier ident, Entity.EntityType entityType)
+      throws NoSuchEntityException, IOException {
     switch (entityType) {
       case METALAKE:
         return (E) MetalakeMetaService.getInstance().getMetalakeByIdentifier(ident);
@@ -182,7 +182,8 @@ public <E extends Entity & HasIdentifier> E get(
   }
 
   @Override
-  public boolean delete(NameIdentifier ident, Entity.EntityType entityType, boolean cascade) {
+  public boolean delete(NameIdentifier ident, Entity.EntityType entityType, boolean cascade)
+      throws IOException {
     switch (entityType) {
       case METALAKE:
         return MetalakeMetaService.getInstance().deleteMetalake(ident, cascade);
@@ -209,7 +210,8 @@ public boolean delete(NameIdentifier ident, Entity.EntityType entityType, boolea
   }
 
   @Override
-  public int hardDeleteLegacyData(Entity.EntityType entityType, long legacyTimeline) {
+  public int hardDeleteLegacyData(Entity.EntityType entityType, long legacyTimeline)
+      throws IOException {
     switch (entityType) {
       case METALAKE:
         return MetalakeMetaService.getInstance()
@@ -260,7 +262,8 @@ public int hardDeleteLegacyData(Entity.EntityType entityType, long legacyTimelin
   }
 
   @Override
-  public int deleteOldVersionData(Entity.EntityType entityType, long versionRetentionCount) {
+  public int deleteOldVersionData(Entity.EntityType entityType, long versionRetentionCount)
+      throws IOException {
     switch (entityType) {
       case METALAKE:
       case CATALOG:

core/src/main/java/com/datastrato/gravitino/storage/relational/RelationalBackend.java

@@ -35,28 +35,31 @@ public interface RelationalBackend extends Closeable {
    *     if the entities does not exist.
    * @throws NoSuchEntityException If the corresponding parent entity of these list entities cannot
    *     be found.
+   * @throws IOException If the store operation fails
    */
   <E extends Entity & HasIdentifier> List<E> list(Namespace namespace, Entity.EntityType entityType)
-      throws NoSuchEntityException;
+      throws NoSuchEntityException, IOException;
 
   /**
    * Checks the entity associated with the given identifier and entityType whether exists.
    *
    * @param ident The identifier of the entity.
    * @param entityType The type of the entity.
    * @return True, if the entity can be found, else return false.
+   * @throws IOException If the store operation fails
    */
-  boolean exists(NameIdentifier ident, Entity.EntityType entityType);
+  boolean exists(NameIdentifier ident, Entity.EntityType entityType) throws IOException;
 
   /**
    * Stores the entity, possibly overwriting an existing entity if specified.
    *
    * @param e The entity which need be stored.
    * @param overwritten If true, overwrites the existing value.
    * @throws EntityAlreadyExistsException If the entity already exists and overwrite is false.
+   * @throws IOException If the store operation fails
    */
   <E extends Entity & HasIdentifier> void insert(E e, boolean overwritten)
-      throws EntityAlreadyExistsException;
+      throws EntityAlreadyExistsException, IOException;
 
   /**
    * Updates the entity.
@@ -90,8 +93,10 @@ <E extends Entity & HasIdentifier> E get(NameIdentifier ident, Entity.EntityType
    * @param entityType The type of the entity.
    * @param cascade True, If you need to cascade delete entities, else false.
    * @return True, if the entity was successfully deleted, else false.
+   * @throws IOException If the store operation fails
    */
-  boolean delete(NameIdentifier ident, Entity.EntityType entityType, boolean cascade);
+  boolean delete(NameIdentifier ident, Entity.EntityType entityType, boolean cascade)
+      throws IOException;
 
   /**
    * Permanently deletes the legacy data that has been marked as deleted before the given legacy
@@ -100,8 +105,9 @@ <E extends Entity & HasIdentifier> E get(NameIdentifier ident, Entity.EntityType
    * @param entityType The type of the entity.
    * @param legacyTimeline The time before which the data has been marked as deleted.
    * @return The count of the deleted data.
+   * @throws IOException If the store operation fails
    */
-  int hardDeleteLegacyData(Entity.EntityType entityType, long legacyTimeline);
+  int hardDeleteLegacyData(Entity.EntityType entityType, long legacyTimeline) throws IOException;
 
   /**
    * Soft deletes the old version data that is older than or equal to the given version retention
@@ -110,6 +116,8 @@ <E extends Entity & HasIdentifier> E get(NameIdentifier ident, Entity.EntityType
    * @param entityType The type of the entity.
    * @param versionRetentionCount The count of versions to retain.
    * @return The count of the deleted data.
+   * @throws IOException If the store operation fails
    */
-  int deleteOldVersionData(Entity.EntityType entityType, long versionRetentionCount);
+  int deleteOldVersionData(Entity.EntityType entityType, long versionRetentionCount)
+      throws IOException;
 }

core/src/main/java/com/datastrato/gravitino/storage/relational/RelationalEntityStore.java

@@ -15,7 +15,6 @@
 import com.datastrato.gravitino.HasIdentifier;
 import com.datastrato.gravitino.NameIdentifier;
 import com.datastrato.gravitino.Namespace;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
 import com.datastrato.gravitino.exceptions.NoSuchEntityException;
 import com.datastrato.gravitino.utils.Executable;
 import com.google.common.collect.ImmutableMap;
@@ -88,7 +87,7 @@ public <E extends Entity & HasIdentifier> void put(E e, boolean overwritten)
   @Override
   public <E extends Entity & HasIdentifier> E update(
       NameIdentifier ident, Class<E> type, Entity.EntityType entityType, Function<E, E> updater)
-      throws IOException, NoSuchEntityException, AlreadyExistsException {
+      throws IOException, NoSuchEntityException, EntityAlreadyExistsException {
     return backend.update(ident, entityType, updater);
   }
 

core/src/main/java/com/datastrato/gravitino/storage/relational/converters/H2ExceptionConverter.java

@@ -5,8 +5,8 @@
 package com.datastrato.gravitino.storage.relational.converters;
 
 import com.datastrato.gravitino.Entity;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
-import com.datastrato.gravitino.exceptions.GravitinoRuntimeException;
+import com.datastrato.gravitino.EntityAlreadyExistsException;
+import java.io.IOException;
 import java.sql.SQLException;
 
 /**
@@ -19,13 +19,13 @@ public class H2ExceptionConverter implements SQLExceptionConverter {
 
   @SuppressWarnings("FormatStringAnnotation")
   @Override
-  public GravitinoRuntimeException toGravitinoException(
-      SQLException se, Entity.EntityType type, String name) {
+  public void toGravitinoException(SQLException se, Entity.EntityType type, String name)
+      throws IOException {
     switch (se.getErrorCode()) {
       case DUPLICATED_ENTRY_ERROR_CODE:
-        return new AlreadyExistsException(se, se.getMessage());
+        throw new EntityAlreadyExistsException(se, se.getMessage());
       default:
-        return new GravitinoRuntimeException(se, se.getMessage());
+        throw new IOException(se);
     }
   }
 }

core/src/main/java/com/datastrato/gravitino/storage/relational/converters/MySQLExceptionConverter.java

@@ -5,8 +5,8 @@
 package com.datastrato.gravitino.storage.relational.converters;
 
 import com.datastrato.gravitino.Entity;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
-import com.datastrato.gravitino.exceptions.GravitinoRuntimeException;
+import com.datastrato.gravitino.EntityAlreadyExistsException;
+import java.io.IOException;
 import java.sql.SQLException;
 
 /**
@@ -20,13 +20,13 @@ public class MySQLExceptionConverter implements SQLExceptionConverter {
 
   @SuppressWarnings("FormatStringAnnotation")
   @Override
-  public GravitinoRuntimeException toGravitinoException(
-      SQLException se, Entity.EntityType type, String name) {
+  public void toGravitinoException(SQLException se, Entity.EntityType type, String name)
+      throws IOException {
     switch (se.getErrorCode()) {
       case DUPLICATED_ENTRY_ERROR_CODE:
-        return new AlreadyExistsException(se, se.getMessage());
+        throw new EntityAlreadyExistsException(se, se.getMessage());
       default:
-        return new GravitinoRuntimeException(se, se.getMessage());
+        throw new IOException(se);
     }
   }
 }

core/src/main/java/com/datastrato/gravitino/storage/relational/converters/SQLExceptionConverter.java

@@ -5,7 +5,7 @@
 package com.datastrato.gravitino.storage.relational.converters;
 
 import com.datastrato.gravitino.Entity;
-import com.datastrato.gravitino.exceptions.GravitinoRuntimeException;
+import java.io.IOException;
 import java.sql.SQLException;
 
 /** Interface for converter JDBC SQL exceptions to Gravitino exceptions. */
@@ -16,9 +16,7 @@ public interface SQLExceptionConverter {
    * @param sqlException The sql exception to map
    * @param type The type of the entity
    * @param name The name of the entity
-   * @return A best attempt at a corresponding jdbc connector exception or generic with the
-   *     SQLException as the cause
    */
-  GravitinoRuntimeException toGravitinoException(
-      SQLException sqlException, Entity.EntityType type, String name);
+  void toGravitinoException(SQLException sqlException, Entity.EntityType type, String name)
+      throws IOException;
 }