yuqi1129 commented on code in PR #13792: URL: https://github.com/apache/gravitino/pull/13792#discussion_r4270037830
########## core/src/test/java/org/apache/gravitino/storage/relational/TestRelationalEntityStoreBackendLockWaitTimeout.java: ########## @@ -0,0 +1,206 @@ +/* + * 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.storage.relational; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.List; +import org.apache.commons.dbcp2.BasicDataSource; +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.gravitino.Entity; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.cache.EntityCache; +import org.apache.gravitino.exceptions.OptimisticLockException; +import org.apache.gravitino.meta.SchemaEntity; +import org.apache.gravitino.meta.TableEntity; +import org.apache.gravitino.storage.RandomIdGenerator; +import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; +import org.apache.gravitino.storage.relational.session.SqlSessions; +import org.apache.ibatis.mapping.Environment; +import org.apache.ibatis.session.Configuration; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.TestTemplate; +import org.junit.jupiter.api.function.Executable; +import org.mockito.Mockito; + +/** + * Verifies real backend lock timeouts, transaction cleanup, and retries through the entity store. + */ +@Tag("gravitino-docker-test") +public class TestRelationalEntityStoreBackendLockWaitTimeout extends TestJDBCBackend { + + @AfterEach + void closeBackend() throws Exception { + String databasePath = null; + try { + if ("h2".equals(backendType)) { + BasicDataSource source = + (BasicDataSource) + SqlSessionFactoryHelper.getInstance() + .getSqlSessionFactory() + .getConfiguration() + .getEnvironment() + .getDataSource(); + databasePath = source.getUrl().substring("jdbc:h2:file:".length()).split(";")[0]; + // The shared fixture uses DB_CLOSE_DELAY=-1, so close the engine before deleting its files. + try (Connection connection = source.getConnection(); + Statement statement = connection.createStatement()) { + statement.execute("SHUTDOWN"); + } + } + } finally { + try { + backend.close(); + } finally { + if (databasePath != null) { + FileUtils.deleteDirectory(Path.of(databasePath).toFile()); + Files.deleteIfExists(Path.of(databasePath + ".mv.db")); + Files.deleteIfExists(Path.of(databasePath + ".trace.db")); + } + } + } Review Comment: The shared H2 fixture currently creates a real directory at the database base path in BackendTestExtension.buildBackend(): it calls mkdirs() on jdbcPath before using that same path in the JDBC URL. The teardown deletes that fixture-owned directory and separately deletes the adjacent .mv.db and .trace.db files after SHUTDOWN and backend.close(). Deleting the parent would delete /tmp, so it would be unsafe here. No code change is needed for the current fixture. I reran both real-backend timeout cases on H2, MySQL and PostgreSQL: all six passed, including teardown. ########## core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java: ########## @@ -524,13 +580,42 @@ public <E extends Entity & HasIdentifier> List<E> updateEntityRelations(Relation public int batchDelete( List<Pair<NameIdentifier, Entity.EntityType>> entitiesToDelete, boolean cascade) throws IOException { - return backend.batchDelete(entitiesToDelete, cascade); + try { + return backend.batchDelete(entitiesToDelete, cascade); + } catch (RuntimeException re) { + throw ExceptionUtils.lockWaitTimeoutAsConflict( + re, + describeBatch( + entitiesToDelete.size(), + entitiesToDelete.isEmpty() ? null : entitiesToDelete.get(0).getRight(), + entitiesToDelete.isEmpty() ? null : entitiesToDelete.get(0).getLeft())); + } } @Override public <E extends Entity & HasIdentifier> void batchPut(List<E> entities, boolean overwritten) throws IOException, EntityAlreadyExistsException { - backend.batchPut(entities, overwritten); + try { + backend.batchPut(entities, overwritten); + } catch (RuntimeException re) { + throw ExceptionUtils.lockWaitTimeoutAsConflict( + re, + describeBatch( + entities.size(), + entities.isEmpty() ? null : entities.get(0).type(), + entities.isEmpty() ? null : entities.get(0).nameIdentifier())); + } + } + + /** Describes a batch write by its size and its first entity, for a conflict message. */ + private static String describeBatch( + int size, @Nullable Entity.EntityType firstType, @Nullable NameIdentifier first) { + if (firstType == null) { + return String.format("The batch of %d entities", size); + } + return String.format( + "The batch of %d entities starting with %s %s", + size, firstType.name().toLowerCase(Locale.ROOT), first); Review Comment: javax.annotation.Nullable is already imported in RelationalEntityStore (line 36) and used by the existing fields and close helper. The new parameters use that same annotation. Compilation and core Spotless passed, along with 22 focused timeout/classification tests, including six real H2/MySQL/PostgreSQL cases. No import change is needed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
