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 <[email protected]>
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
