You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by tk...@apache.org on 2023/11/17 13:40:12 UTC

(ignite-3) branch main updated: IGNITE-20886 Don't unregister indexes on CatalogEvent#INDEX_DROP (#2849)

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

tkalkirill 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 9462367a70 IGNITE-20886 Don't unregister indexes on CatalogEvent#INDEX_DROP (#2849)
9462367a70 is described below

commit 9462367a70a01b9086793dff0e9f13e1dde1e382
Author: Kirill Tkalenko <tk...@yandex.ru>
AuthorDate: Fri Nov 17 16:40:07 2023 +0300

    IGNITE-20886 Don't unregister indexes on CatalogEvent#INDEX_DROP (#2849)
---
 .../apache/ignite/internal/index/IndexManager.java | 10 +-------
 .../ignite/internal/index/IndexManagerTest.java    | 30 +++++++++++++++++++---
 .../internal/index/TestIndexManagementUtils.java   |  4 +++
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java b/modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java
index 3d132204e5..c5a10da1e1 100644
--- a/modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java
+++ b/modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java
@@ -20,7 +20,6 @@ package org.apache.ignite.internal.index;
 import static java.util.concurrent.CompletableFuture.allOf;
 import static java.util.concurrent.CompletableFuture.failedFuture;
 import static org.apache.ignite.internal.catalog.events.CatalogEvent.INDEX_CREATE;
-import static org.apache.ignite.internal.catalog.events.CatalogEvent.INDEX_DROP;
 import static org.apache.ignite.internal.util.IgniteUtils.inBusyLock;
 import static org.apache.ignite.internal.util.IgniteUtils.inBusyLockAsync;
 
@@ -135,14 +134,6 @@ public class IndexManager implements IgniteComponent {
             return onIndexCreate((CreateIndexEventParameters) parameters);
         });
 
-        catalogService.listen(INDEX_DROP, (parameters, exception) -> {
-            if (exception != null) {
-                return failedFuture(exception);
-            }
-
-            return onIndexDrop((DropIndexEventParameters) parameters);
-        });
-
         LOG.info("Index manager started");
     }
 
@@ -179,6 +170,7 @@ public class IndexManager implements IgniteComponent {
         return mvTableStoragesByIdVv.get(causalityToken).thenApply(mvTableStoragesById -> mvTableStoragesById.get(tableId));
     }
 
+    // TODO: IGNITE-20121 Unregister index only before we physically start deleting the index before truncate catalog
     private CompletableFuture<Boolean> onIndexDrop(DropIndexEventParameters parameters) {
         int indexId = parameters.indexId();
         int tableId = parameters.tableId();
diff --git a/modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java b/modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java
index 1b38573f37..d146da3172 100644
--- a/modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java
+++ b/modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java
@@ -22,9 +22,12 @@ import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_SCHEMA_N
 import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_ZONE_NAME;
 import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_DATA_REGION;
 import static org.apache.ignite.internal.index.TestIndexManagementUtils.COLUMN_NAME;
+import static org.apache.ignite.internal.index.TestIndexManagementUtils.INDEX_NAME;
 import static org.apache.ignite.internal.index.TestIndexManagementUtils.NODE_NAME;
 import static org.apache.ignite.internal.index.TestIndexManagementUtils.TABLE_NAME;
+import static org.apache.ignite.internal.index.TestIndexManagementUtils.createIndex;
 import static org.apache.ignite.internal.index.TestIndexManagementUtils.createTable;
+import static org.apache.ignite.internal.index.TestIndexManagementUtils.dropIndex;
 import static org.apache.ignite.internal.table.TableTestUtils.createHashIndex;
 import static org.apache.ignite.internal.table.TableTestUtils.dropTable;
 import static org.apache.ignite.internal.table.TableTestUtils.getTableIdStrict;
@@ -38,10 +41,15 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.LongFunction;
 import org.apache.ignite.internal.catalog.CatalogManager;
 import org.apache.ignite.internal.catalog.CatalogManagerImpl;
@@ -73,9 +81,7 @@ import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-/**
- * Test class to verify {@link IndexManager}.
- */
+/** Test class to verify {@link IndexManager}. */
 public class IndexManagerTest extends BaseIgniteAbstractTest {
     private final HybridClock clock = new HybridClockImpl();
 
@@ -89,6 +95,8 @@ public class IndexManagerTest extends BaseIgniteAbstractTest {
 
     private IndexManager indexManager;
 
+    private final Map<Integer, TableViewInternal> tableViewInternalByTableId = new ConcurrentHashMap<>();
+
     @BeforeEach
     public void setUp() {
         TableManager tableManagerMock = mock(TableManager.class);
@@ -190,7 +198,21 @@ public class IndexManagerTest extends BaseIgniteAbstractTest {
         assertThat(getMvTableStorageInCatalogListenerFuture, willBe(notNullValue()));
     }
 
+    @Test
+    void testDontUnregisterIndexOnCatalogEventIndexDrop() throws Exception {
+        createIndex(catalogManager, TABLE_NAME, INDEX_NAME, COLUMN_NAME);
+        dropIndex(catalogManager, INDEX_NAME);
+
+        TableViewInternal tableViewInternal = tableViewInternalByTableId.get(tableId());
+
+        verify(tableViewInternal, never()).unregisterIndex(anyInt());
+    }
+
     private TableViewInternal mockTable(int tableId) {
+        return tableViewInternalByTableId.computeIfAbsent(tableId, this::newMockTable);
+    }
+
+    private TableViewInternal newMockTable(int tableId) {
         CatalogZoneDescriptor zone = catalogManager.zone(DEFAULT_ZONE_NAME, clock.nowLong());
 
         assertNotNull(zone);
@@ -206,7 +228,7 @@ public class IndexManagerTest extends BaseIgniteAbstractTest {
         when(internalTable.tableId()).thenReturn(tableId);
         when(internalTable.storage()).thenReturn(mvTableStorage);
 
-        return new TableImpl(internalTable, new HeapLockManager(), new ConstantSchemaVersions(1));
+        return spy(new TableImpl(internalTable, new HeapLockManager(), new ConstantSchemaVersions(1)));
     }
 
     private CompletableFuture<MvTableStorage> getMvTableStorageLatestRevision(int tableId) {
diff --git a/modules/index/src/test/java/org/apache/ignite/internal/index/TestIndexManagementUtils.java b/modules/index/src/test/java/org/apache/ignite/internal/index/TestIndexManagementUtils.java
index 8b1339f288..a44f789c34 100644
--- a/modules/index/src/test/java/org/apache/ignite/internal/index/TestIndexManagementUtils.java
+++ b/modules/index/src/test/java/org/apache/ignite/internal/index/TestIndexManagementUtils.java
@@ -79,6 +79,10 @@ class TestIndexManagementUtils {
         TableTestUtils.createHashIndex(catalogManager, DEFAULT_SCHEMA_NAME, tableName, indexName, List.of(columnName), false);
     }
 
+    static void dropIndex(CatalogManager catalogManager, String indexName) {
+        TableTestUtils.dropIndex(catalogManager, DEFAULT_SCHEMA_NAME, indexName);
+    }
+
     static int indexId(CatalogService catalogService, String indexName, HybridClock clock) {
         return TableTestUtils.getIndexIdStrict(catalogService, indexName, clock.nowLong());
     }