You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ib...@apache.org on 2024/04/16 12:16:08 UTC

(ignite-3) branch main updated: IGNITE-22047 Ignore index destruction attempt if table storage is closed/destroyed (#3609)

This is an automated email from the ASF dual-hosted git repository.

ibessonov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git


The following commit(s) were added to refs/heads/main by this push:
     new 7306b533c0 IGNITE-22047 Ignore index destruction attempt if table storage is closed/destroyed (#3609)
7306b533c0 is described below

commit 7306b533c013cae1b5af517424e6a2cccef68d8d
Author: Roman Puchkovskiy <ro...@gmail.com>
AuthorDate: Tue Apr 16 16:16:03 2024 +0400

    IGNITE-22047 Ignore index destruction attempt if table storage is closed/destroyed (#3609)
---
 .../internal/storage/engine/MvTableStorage.java    |  2 +-
 .../storage/AbstractMvTableStorageTest.java        | 27 ++++++++++++++++++++++
 .../pagememory/AbstractPageMemoryTableStorage.java | 10 ++++++--
 .../storage/rocksdb/RocksDbTableStorage.java       | 26 ++++++++++++---------
 4 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java b/modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java
index 3601a6fe3e..bcb4f58b3a 100644
--- a/modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java
+++ b/modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java
@@ -118,7 +118,7 @@ public interface MvTableStorage extends ManuallyCloseable {
     /**
      * Destroys the index with the given ID and all data in it.
      *
-     * <p>This method is a no-op if the index with the given ID does not exist.
+     * <p>This method is a no-op if the index with the given ID does not exist or if the table storage is closed or under destruction.
      *
      * @param indexId Index ID.
      */
diff --git a/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/AbstractMvTableStorageTest.java b/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/AbstractMvTableStorageTest.java
index a443ba8740..93eb0fd0b6 100644
--- a/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/AbstractMvTableStorageTest.java
+++ b/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/AbstractMvTableStorageTest.java
@@ -324,6 +324,33 @@ public abstract class AbstractMvTableStorageTest extends BaseMvStoragesTest {
         assertThat(tableStorage.getIndex(PARTITION_ID, hashIdx.id()), is(nullValue()));
     }
 
+    /**
+     * Tests that an attempt to destroy an index in a table storage that is already destroyed does not
+     * cause an exception.
+     */
+    @ParameterizedTest
+    @ValueSource(booleans = {false, true})
+    public void indexDestructionDoesNotFailIfTableStorageIsDestroyed(boolean waitForDestroyFuture) throws Exception {
+        MvPartitionStorage partitionStorage = getOrCreateMvPartition(PARTITION_ID);
+
+        SortedIndexStorage sortedIndexStorage = tableStorage.getOrCreateSortedIndex(PARTITION_ID, sortedIdx);
+        assertThat(sortedIndexStorage, is(notNullValue()));
+
+        HashIndexStorage hashIndexStorage = tableStorage.getOrCreateHashIndex(PARTITION_ID, hashIdx);
+        assertThat(hashIndexStorage, is(notNullValue()));
+
+        assertThat(partitionStorage.flush(), willCompleteSuccessfully());
+
+        CompletableFuture<Void> destroyTableStorageFuture = tableStorage.destroy();
+
+        if (waitForDestroyFuture) {
+            assertThat(destroyTableStorageFuture, willCompleteSuccessfully());
+        }
+
+        assertDoesNotThrow(() -> tableStorage.destroyIndex(sortedIdx.id()).get(10, SECONDS));
+        assertDoesNotThrow(() -> tableStorage.destroyIndex(hashIdx.id()).get(10, SECONDS));
+    }
+
     /**
      * Tests that removing one Sorted Index does not affect the data in the other.
      */
diff --git a/modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryTableStorage.java b/modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryTableStorage.java
index 8260a927d3..6a492da3a9 100644
--- a/modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryTableStorage.java
+++ b/modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryTableStorage.java
@@ -180,7 +180,11 @@ public abstract class AbstractPageMemoryTableStorage implements MvTableStorage {
 
     @Override
     public CompletableFuture<Void> destroyIndex(int indexId) {
-        return busy(() -> {
+        if (!busyLock.enterBusy()) {
+            return nullCompletedFuture();
+        }
+
+        try {
             List<AbstractPageMemoryMvPartitionStorage> storages = mvPartitionStorages.getAll();
 
             var destroyFutures = new CompletableFuture[storages.size()];
@@ -192,7 +196,9 @@ public abstract class AbstractPageMemoryTableStorage implements MvTableStorage {
             }
 
             return allOf(destroyFutures);
-        });
+        } finally {
+            busyLock.leaveBusy();
+        }
     }
 
     @Override
diff --git a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
index 3f7cd6ac93..ea4ec855bf 100644
--- a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
+++ b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/RocksDbTableStorage.java
@@ -118,21 +118,21 @@ public class RocksDbTableStorage implements MvTableStorage {
     /**
      * Returns a column family handle for partitions column family.
      */
-    public ColumnFamilyHandle partitionCfHandle() {
+    ColumnFamilyHandle partitionCfHandle() {
         return rocksDb.partitionCf.handle();
     }
 
     /**
      * Returns a column family handle for meta column family.
      */
-    public ColumnFamilyHandle metaCfHandle() {
+    ColumnFamilyHandle metaCfHandle() {
         return rocksDb.meta.columnFamily().handle();
     }
 
     /**
      * Returns a column family handle for GC queue.
      */
-    public ColumnFamilyHandle gcQueueHandle() {
+    ColumnFamilyHandle gcQueueHandle() {
         return rocksDb.gcQueueCf.handle();
     }
 
@@ -258,15 +258,19 @@ public class RocksDbTableStorage implements MvTableStorage {
 
     @Override
     public CompletableFuture<Void> destroyIndex(int indexId) {
-        return inBusyLock(busyLock, () -> {
-            try {
-                indexes.destroyIndex(indexId);
+        if (!busyLock.enterBusy()) {
+            return nullCompletedFuture();
+        }
 
-                return nullCompletedFuture();
-            } catch (RocksDBException e) {
-                throw new StorageException("Error when destroying index: {}", e, indexId);
-            }
-        });
+        try {
+            indexes.destroyIndex(indexId);
+
+            return nullCompletedFuture();
+        } catch (RocksDBException e) {
+            throw new StorageException("Error when destroying index: {}", e, indexId);
+        } finally {
+            busyLock.leaveBusy();
+        }
     }
 
     @Override