sashapolo commented on code in PR #2871:
URL: https://github.com/apache/ignite-3/pull/2871#discussion_r1405726146
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java:
##########
@@ -399,4 +405,49 @@ static CatalogIndexDescriptor indexOrThrow(Catalog
catalog, int indexId) throws
return index;
}
+
+ /**
+ * Collects all table indexes (including dropped) that the table had in
the requested catalog version range.
+ *
+ * <p>It is expected that at least one version of the catalog contains
table indexes.</p>
Review Comment:
What do you mean here? That catalog should contain at least one index of the
given table?
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java:
##########
@@ -399,4 +405,49 @@ static CatalogIndexDescriptor indexOrThrow(Catalog
catalog, int indexId) throws
return index;
}
+
+ /**
+ * Collects all table indexes (including dropped) that the table had in
the requested catalog version range.
Review Comment:
```suggestion
* Collects all table indexes (including dropped) that the table has in
the requested catalog version range.
```
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java:
##########
@@ -399,4 +405,49 @@ static CatalogIndexDescriptor indexOrThrow(Catalog
catalog, int indexId) throws
return index;
}
+
+ /**
+ * Collects all table indexes (including dropped) that the table had in
the requested catalog version range.
+ *
+ * <p>It is expected that at least one version of the catalog contains
table indexes.</p>
+ *
+ * @param catalogService Catalog service.
+ * @param tableId Table ID for which indexes will be collected.
+ * @param catalogVersionFrom Catalog version from which indexes will be
collected (including).
+ * @param catalogVersionTo Catalog version up to which indexes will be
collected (including).
+ * @return Table indexes sorted by {@link CatalogIndexDescriptor#id()}.
+ */
+ public static List<CatalogIndexDescriptor> collectIndexes(
+ CatalogService catalogService,
+ int tableId,
+ int catalogVersionFrom,
+ int catalogVersionTo
+ ) {
+ assert catalogVersionFrom <= catalogVersionTo : "from=" +
catalogVersionFrom + ", to=" + catalogVersionTo;
+
+ if (catalogVersionFrom == catalogVersionTo) {
+ List<CatalogIndexDescriptor> indexes =
catalogService.indexes(catalogVersionFrom, tableId);
+
+ assert !indexes.isEmpty() : "catalogVersion=" + catalogVersionFrom
+ ", tableId=" + tableId;
+
+ return indexes;
+ }
+
+ var indexByIdMap = new Int2ObjectOpenHashMap<CatalogIndexDescriptor>();
Review Comment:
Can we use `Int2ObjectRBTreeMap` here? This way your descriptors will be
sorted by ID
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java:
##########
@@ -399,4 +405,49 @@ static CatalogIndexDescriptor indexOrThrow(Catalog
catalog, int indexId) throws
return index;
}
+
+ /**
+ * Collects all table indexes (including dropped) that the table had in
the requested catalog version range.
+ *
+ * <p>It is expected that at least one version of the catalog contains
table indexes.</p>
+ *
+ * @param catalogService Catalog service.
+ * @param tableId Table ID for which indexes will be collected.
+ * @param catalogVersionFrom Catalog version from which indexes will be
collected (including).
+ * @param catalogVersionTo Catalog version up to which indexes will be
collected (including).
+ * @return Table indexes sorted by {@link CatalogIndexDescriptor#id()}.
+ */
+ public static List<CatalogIndexDescriptor> collectIndexes(
+ CatalogService catalogService,
+ int tableId,
+ int catalogVersionFrom,
+ int catalogVersionTo
+ ) {
+ assert catalogVersionFrom <= catalogVersionTo : "from=" +
catalogVersionFrom + ", to=" + catalogVersionTo;
+
+ if (catalogVersionFrom == catalogVersionTo) {
+ List<CatalogIndexDescriptor> indexes =
catalogService.indexes(catalogVersionFrom, tableId);
+
+ assert !indexes.isEmpty() : "catalogVersion=" + catalogVersionFrom
+ ", tableId=" + tableId;
+
+ return indexes;
+ }
+
+ var indexByIdMap = new Int2ObjectOpenHashMap<CatalogIndexDescriptor>();
+
+ for (int catalogVersion = catalogVersionFrom; catalogVersion <=
catalogVersionTo; catalogVersion++) {
+ for (CatalogIndexDescriptor index :
catalogService.indexes(catalogVersion, tableId)) {
+ indexByIdMap.put(index.id(), index);
+ }
+ }
+
+ assert !indexByIdMap.isEmpty()
+ : String.format("catalogVersionFrom=%s, catalogVersionTo=%s,
tableId=%s", catalogVersionFrom, catalogVersionTo, tableId);
+
+ var res = new ArrayList<>(indexByIdMap.values());
+
+ res.sort(comparingInt(CatalogObjectDescriptor::id));
+
+ return unmodifiableList(res);
Review Comment:
Why do you need an `unmodifiableList` here? It's a local variable anyway
##########
modules/index/src/integrationTest/java/org/apache/ignite/internal/index/ItIndexManagerTest.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.index;
+
+import static java.util.stream.Collectors.toList;
+import static
org.apache.ignite.internal.index.IndexManager.collectIndexesForRecovery;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.runAsync;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.catalog.descriptors.CatalogObjectDescriptor;
+import org.apache.ignite.internal.sql.BaseSqlIntegrationTest;
+import org.apache.ignite.internal.table.TableImpl;
+import org.apache.ignite.table.Table;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+/** For {@link IndexManager} testing. */
+public class ItIndexManagerTest extends BaseSqlIntegrationTest {
Review Comment:
Why does this class extend `BaseSqlIntegrationTest` and not
`ClusterPerTestIntegrationTest`?
##########
modules/index/src/integrationTest/java/org/apache/ignite/internal/index/ItIndexManagerTest.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.index;
+
+import static java.util.stream.Collectors.toList;
+import static
org.apache.ignite.internal.index.IndexManager.collectIndexesForRecovery;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.runAsync;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.catalog.descriptors.CatalogObjectDescriptor;
+import org.apache.ignite.internal.sql.BaseSqlIntegrationTest;
+import org.apache.ignite.internal.table.TableImpl;
+import org.apache.ignite.table.Table;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+/** For {@link IndexManager} testing. */
+public class ItIndexManagerTest extends BaseSqlIntegrationTest {
+ private static final String ZONE_NAME = "ZONE_TABLE";
+
+ private static final String TABLE_NAME = "TEST_TABLE";
+
+ private static final String INDEX_NAME = "TEST_INDEX";
+
+ @AfterEach
+ void tearDown() {
+ if (node() != null) {
+ sql("DROP TABLE IF EXISTS " + TABLE_NAME);
+ sql("DROP ZONE IF EXISTS " + ZONE_NAME);
+ }
+ }
+
+ @Override
+ protected int initialNodes() {
+ return 1;
+ }
+
+ @Test
+ void testStartActiveAndDroppedIndexOnNodeRecovery() throws Exception {
+ createZoneAndTable(ZONE_NAME, TABLE_NAME, 1, 1);
+
+ insertPeople(TABLE_NAME, createPeopleBatch(100));
+
+ String indexName0 = INDEX_NAME + 0;
+ String indexName1 = INDEX_NAME + 1;
+
+ createIndex(TABLE_NAME, indexName0, "NAME");
+ createIndex(TABLE_NAME, indexName1, "SALARY");
+
+ awaitIndexesBecomeAvailable(node(), indexName0, indexName1);
+
+ dropIndex(indexName1);
+
+ // Let's restart the node.
+ CLUSTER.stopNode(0);
+ CLUSTER.startNode(0);
+
+ TableImpl tableImpl = getTableImpl(node(), TABLE_NAME);
+
+ assertThat(
+ collectIndexIdsFromTable(tableImpl, 0),
+ equalTo(collectIndexIdsFromCatalogForRecovery(node(),
tableImpl))
+ );
+ }
+
+ private static IgniteImpl node() {
+ return CLUSTER.node(0);
+ }
+
+ private static Person[] createPeopleBatch(int batchSize) {
+ return IntStream.range(0, batchSize)
+ .mapToObj(personId -> new Person(personId, "person" +
personId, 10.0 + personId))
+ .toArray(Person[]::new);
+ }
+
+ private static TableImpl getTableImpl(IgniteImpl ignite, String tableName)
{
+ CompletableFuture<Table> tableFuture =
ignite.tables().tableAsync(tableName);
Review Comment:
Why not use the sync version of the same method?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1044,9 +1045,16 @@ private void cleanUpTablesResources(Map<Integer,
TableImpl> tables) {
* @param causalityToken Causality token.
* @param catalogVersion Catalog version on which the table was created.
* @param tableDescriptor Catalog table descriptor.
+ * @param onNodeRecovery On node recovery.
Review Comment:
This is not a good description
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2085,9 +2097,11 @@ private CompletableFuture<Void>
destroyPartitionStorages(TablePartitionId tableP
);
}
- private int[] collectTableIndexIds(int tableId, int catalogVersion) {
- return catalogService.indexes(catalogVersion, tableId).stream()
- .mapToInt(CatalogIndexDescriptor::id)
+ private int[] collectTableIndexIds(int tableId, int catalogVersion,
boolean onNodeRecovery) {
+ int catalogVersionFrom = onNodeRecovery ?
catalogService.latestCatalogVersion() : catalogVersion;
Review Comment:
Can you please add a comment about why do we use the latest catalog version
when a node is starting?
##########
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CatalogUtilsTest.java:
##########
@@ -189,6 +217,187 @@ void testFromParamsAndPreviousValueWithAutoAdjustFields()
{
);
}
+ @Test
+ void testCollectIndexesAfterCreateTable() throws Exception {
+ withCatalogManager(catalogManager -> {
+ createTable(catalogManager, TABLE_NAME);
+
+ int latestCatalogVersion = catalogManager.latestCatalogVersion();
+ int earliestCatalogVersion =
catalogManager.earliestCatalogVersion();
+
+ int tableId = tableId(catalogManager, latestCatalogVersion,
TABLE_NAME);
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
latestCatalogVersion, latestCatalogVersion),
+ contains(index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME))
+ );
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
earliestCatalogVersion, latestCatalogVersion),
+ contains(index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME))
+ );
+ });
+ }
+
+ @Test
+ void testCollectIndexesAfterCreateIndex() throws Exception {
+ withCatalogManager(catalogManager -> {
+ createTable(catalogManager, TABLE_NAME);
+ createIndex(catalogManager, TABLE_NAME, INDEX_NAME);
+
+ int latestCatalogVersion = catalogManager.latestCatalogVersion();
+ int earliestCatalogVersion =
catalogManager.earliestCatalogVersion();
+
+ int tableId = tableId(catalogManager, latestCatalogVersion,
TABLE_NAME);
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
latestCatalogVersion, latestCatalogVersion),
+ contains(
+ index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME),
+ index(catalogManager, latestCatalogVersion,
INDEX_NAME)
+ )
+ );
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
earliestCatalogVersion, latestCatalogVersion),
+ contains(
+ index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME),
+ index(catalogManager, latestCatalogVersion,
INDEX_NAME)
+ )
+ );
+ });
+ }
+
+ @Test
+ void testCollectIndexesAfterCreateIndexAndMakeAvailableIndex() throws
Exception {
+ withCatalogManager(catalogManager -> {
+ String indexName0 = INDEX_NAME + 0;
+ String indexName1 = INDEX_NAME + 1;
+
+ createTable(catalogManager, TABLE_NAME);
+ createIndex(catalogManager, TABLE_NAME, indexName0);
+ createIndex(catalogManager, TABLE_NAME, indexName1);
+
+ makeIndexAvailable(catalogManager, indexName1);
Review Comment:
What scenario does this code test? The test above already checks that the
index is available after it's been created
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java:
##########
@@ -399,4 +405,49 @@ static CatalogIndexDescriptor indexOrThrow(Catalog
catalog, int indexId) throws
return index;
}
+
+ /**
+ * Collects all table indexes (including dropped) that the table had in
the requested catalog version range.
+ *
+ * <p>It is expected that at least one version of the catalog contains
table indexes.</p>
+ *
+ * @param catalogService Catalog service.
+ * @param tableId Table ID for which indexes will be collected.
+ * @param catalogVersionFrom Catalog version from which indexes will be
collected (including).
+ * @param catalogVersionTo Catalog version up to which indexes will be
collected (including).
+ * @return Table indexes sorted by {@link CatalogIndexDescriptor#id()}.
+ */
+ public static List<CatalogIndexDescriptor> collectIndexes(
Review Comment:
I can see that this methods is only used in one place, do we really need a
utility method for that?
##########
modules/index/src/integrationTest/java/org/apache/ignite/internal/index/ItIndexManagerTest.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.index;
+
+import static java.util.stream.Collectors.toList;
+import static
org.apache.ignite.internal.index.IndexManager.collectIndexesForRecovery;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.runAsync;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.catalog.descriptors.CatalogObjectDescriptor;
+import org.apache.ignite.internal.sql.BaseSqlIntegrationTest;
+import org.apache.ignite.internal.table.TableImpl;
+import org.apache.ignite.table.Table;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+/** For {@link IndexManager} testing. */
+public class ItIndexManagerTest extends BaseSqlIntegrationTest {
+ private static final String ZONE_NAME = "ZONE_TABLE";
+
+ private static final String TABLE_NAME = "TEST_TABLE";
+
+ private static final String INDEX_NAME = "TEST_INDEX";
+
+ @AfterEach
+ void tearDown() {
+ if (node() != null) {
+ sql("DROP TABLE IF EXISTS " + TABLE_NAME);
+ sql("DROP ZONE IF EXISTS " + ZONE_NAME);
+ }
+ }
+
+ @Override
+ protected int initialNodes() {
+ return 1;
+ }
+
+ @Test
+ void testStartActiveAndDroppedIndexOnNodeRecovery() throws Exception {
+ createZoneAndTable(ZONE_NAME, TABLE_NAME, 1, 1);
+
+ insertPeople(TABLE_NAME, createPeopleBatch(100));
+
+ String indexName0 = INDEX_NAME + 0;
+ String indexName1 = INDEX_NAME + 1;
+
+ createIndex(TABLE_NAME, indexName0, "NAME");
+ createIndex(TABLE_NAME, indexName1, "SALARY");
+
+ awaitIndexesBecomeAvailable(node(), indexName0, indexName1);
+
+ dropIndex(indexName1);
+
+ // Let's restart the node.
+ CLUSTER.stopNode(0);
Review Comment:
There's a `Cluster#restartNode` method
##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -444,4 +445,38 @@ private static <T> BiFunction<T, Throwable,
CompletableFuture<T>> updater(Functi
return updateFunction.apply(t);
};
}
+
+ /**
+ * Collects indexes (including deleted ones) for tables (tables from the
latest version of the catalog) from the earliest to the latest
+ * version of the catalog that need to be started on node recovery.
+ *
+ * @param catalogService Catalog service.
+ */
+ static Map<CatalogTableDescriptor, Collection<CatalogIndexDescriptor>>
collectIndexesForRecovery(CatalogService catalogService) {
+ int earliestCatalogVersion = catalogService.earliestCatalogVersion();
+ int latestCatalogVersion = catalogService.latestCatalogVersion();
+
+ var indexesByTableId = new
Int2ObjectOpenHashMap<Int2ObjectMap<CatalogIndexDescriptor>>();
+
+ for (CatalogTableDescriptor table :
catalogService.tables(latestCatalogVersion)) {
+ indexesByTableId.computeIfAbsent(table.id(), indexById -> new
Int2ObjectOpenHashMap<>());
Review Comment:
Is it possible to get duplicate table IDs here? Why not use a simple `put`?
##########
modules/catalog/src/testFixtures/java/org/apache/ignite/internal/catalog/CatalogTestUtils.java:
##########
@@ -298,4 +301,40 @@ public void stop() throws Exception {
}
}
+
+ /**
+ * Searches for a table by name in the requested version of the catalog.
+ *
+ * @param catalogService Catalog service.
+ * @param catalogVersion Catalog version in which to find the table.
+ * @param tableName Table name.
+ */
+ public static CatalogTableDescriptor table(CatalogService catalogService,
int catalogVersion, String tableName) {
+ CatalogTableDescriptor tableDescriptor =
catalogService.tables(catalogVersion).stream()
+ .filter(table -> tableName.equals(table.name()))
+ .findFirst()
+ .orElse(null);
+
+ assertNotNull(tableDescriptor, "catalogVersion=" + catalogVersion + ",
tableName=" + tableName);
+
+ return tableDescriptor;
+ }
+
+ /**
+ * Searches for a index by name in the requested version of the catalog.
Review Comment:
```suggestion
* Searches for an index by name in the requested version of the catalog.
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1087,14 +1096,17 @@ private CompletableFuture<?> createTableLocally(long
causalityToken, int catalog
* @param zoneDescriptor Catalog distributed zone descriptor.
* @param assignmentsFuture Future with assignments.
* @param catalogVersion Catalog version on which the table was created.
+ * @param onNodeRecovery On node recovery.
Review Comment:
This is not a good description
##########
modules/index/src/integrationTest/java/org/apache/ignite/internal/index/ItIndexManagerTest.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.ignite.internal.index;
+
+import static java.util.stream.Collectors.toList;
+import static
org.apache.ignite.internal.index.IndexManager.collectIndexesForRecovery;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.runAsync;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.catalog.descriptors.CatalogObjectDescriptor;
+import org.apache.ignite.internal.sql.BaseSqlIntegrationTest;
+import org.apache.ignite.internal.table.TableImpl;
+import org.apache.ignite.table.Table;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+/** For {@link IndexManager} testing. */
+public class ItIndexManagerTest extends BaseSqlIntegrationTest {
+ private static final String ZONE_NAME = "ZONE_TABLE";
+
+ private static final String TABLE_NAME = "TEST_TABLE";
+
+ private static final String INDEX_NAME = "TEST_INDEX";
+
+ @AfterEach
+ void tearDown() {
+ if (node() != null) {
+ sql("DROP TABLE IF EXISTS " + TABLE_NAME);
+ sql("DROP ZONE IF EXISTS " + ZONE_NAME);
+ }
+ }
+
+ @Override
+ protected int initialNodes() {
+ return 1;
+ }
+
+ @Test
+ void testStartActiveAndDroppedIndexOnNodeRecovery() throws Exception {
+ createZoneAndTable(ZONE_NAME, TABLE_NAME, 1, 1);
+
+ insertPeople(TABLE_NAME, createPeopleBatch(100));
+
+ String indexName0 = INDEX_NAME + 0;
+ String indexName1 = INDEX_NAME + 1;
+
+ createIndex(TABLE_NAME, indexName0, "NAME");
+ createIndex(TABLE_NAME, indexName1, "SALARY");
+
+ awaitIndexesBecomeAvailable(node(), indexName0, indexName1);
+
+ dropIndex(indexName1);
+
+ // Let's restart the node.
+ CLUSTER.stopNode(0);
+ CLUSTER.startNode(0);
+
+ TableImpl tableImpl = getTableImpl(node(), TABLE_NAME);
+
+ assertThat(
+ collectIndexIdsFromTable(tableImpl, 0),
+ equalTo(collectIndexIdsFromCatalogForRecovery(node(),
tableImpl))
+ );
+ }
+
+ private static IgniteImpl node() {
+ return CLUSTER.node(0);
+ }
+
+ private static Person[] createPeopleBatch(int batchSize) {
+ return IntStream.range(0, batchSize)
+ .mapToObj(personId -> new Person(personId, "person" +
personId, 10.0 + personId))
+ .toArray(Person[]::new);
+ }
+
+ private static TableImpl getTableImpl(IgniteImpl ignite, String tableName)
{
+ CompletableFuture<Table> tableFuture =
ignite.tables().tableAsync(tableName);
+
+ assertThat(tableFuture, willCompleteSuccessfully());
+
+ return (TableImpl) tableFuture.join();
+ }
+
+ private static List<Integer> collectIndexIdsFromTable(TableImpl table, int
partitionId) {
+ CompletableFuture<List<Integer>> future = runAsync(() ->
table.indexStorageAdapters(partitionId).get())
Review Comment:
why do you call `runAsync` here if you then wait for the future
synchronously?
##########
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CatalogUtilsTest.java:
##########
@@ -189,6 +217,187 @@ void testFromParamsAndPreviousValueWithAutoAdjustFields()
{
);
}
+ @Test
+ void testCollectIndexesAfterCreateTable() throws Exception {
+ withCatalogManager(catalogManager -> {
+ createTable(catalogManager, TABLE_NAME);
+
+ int latestCatalogVersion = catalogManager.latestCatalogVersion();
+ int earliestCatalogVersion =
catalogManager.earliestCatalogVersion();
+
+ int tableId = tableId(catalogManager, latestCatalogVersion,
TABLE_NAME);
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
latestCatalogVersion, latestCatalogVersion),
+ contains(index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME))
+ );
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
earliestCatalogVersion, latestCatalogVersion),
+ contains(index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME))
+ );
+ });
+ }
+
+ @Test
+ void testCollectIndexesAfterCreateIndex() throws Exception {
+ withCatalogManager(catalogManager -> {
+ createTable(catalogManager, TABLE_NAME);
+ createIndex(catalogManager, TABLE_NAME, INDEX_NAME);
+
+ int latestCatalogVersion = catalogManager.latestCatalogVersion();
+ int earliestCatalogVersion =
catalogManager.earliestCatalogVersion();
+
+ int tableId = tableId(catalogManager, latestCatalogVersion,
TABLE_NAME);
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
latestCatalogVersion, latestCatalogVersion),
+ contains(
+ index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME),
+ index(catalogManager, latestCatalogVersion,
INDEX_NAME)
+ )
+ );
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
earliestCatalogVersion, latestCatalogVersion),
+ contains(
+ index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME),
+ index(catalogManager, latestCatalogVersion,
INDEX_NAME)
+ )
+ );
+ });
+ }
+
+ @Test
+ void testCollectIndexesAfterCreateIndexAndMakeAvailableIndex() throws
Exception {
+ withCatalogManager(catalogManager -> {
+ String indexName0 = INDEX_NAME + 0;
+ String indexName1 = INDEX_NAME + 1;
+
+ createTable(catalogManager, TABLE_NAME);
+ createIndex(catalogManager, TABLE_NAME, indexName0);
+ createIndex(catalogManager, TABLE_NAME, indexName1);
+
+ makeIndexAvailable(catalogManager, indexName1);
+
+ int latestCatalogVersion = catalogManager.latestCatalogVersion();
+ int earliestCatalogVersion =
catalogManager.earliestCatalogVersion();
+
+ int tableId = tableId(catalogManager, latestCatalogVersion,
TABLE_NAME);
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
latestCatalogVersion, latestCatalogVersion),
+ contains(
+ index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME),
+ index(catalogManager, latestCatalogVersion,
indexName0),
+ index(catalogManager, latestCatalogVersion,
indexName1)
+ )
+ );
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
earliestCatalogVersion, latestCatalogVersion),
+ contains(
+ index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME),
+ index(catalogManager, latestCatalogVersion,
indexName0),
+ index(catalogManager, latestCatalogVersion,
indexName1)
+ )
+ );
+ });
+ }
+
+ @Test
+ void testCollectIndexesAfterDropIndexes() throws Exception {
+ withCatalogManager(catalogManager -> {
+ String indexName0 = INDEX_NAME + 0;
+ String indexName1 = INDEX_NAME + 1;
+
+ createTable(catalogManager, TABLE_NAME);
+ createIndex(catalogManager, TABLE_NAME, indexName0);
+ createIndex(catalogManager, TABLE_NAME, indexName1);
+
+ makeIndexAvailable(catalogManager, indexName1);
+
+ int catalogVersionBeforeDropIndex0 =
catalogManager.latestCatalogVersion();
+
+ dropIndex(catalogManager, indexName0);
+
+ int catalogVersionBeforeDropIndex1 =
catalogManager.latestCatalogVersion();
+
+ dropIndex(catalogManager, indexName1);
+
+ int latestCatalogVersion = catalogManager.latestCatalogVersion();
+ int earliestCatalogVersion =
catalogManager.earliestCatalogVersion();
+
+ int tableId = tableId(catalogManager, latestCatalogVersion,
TABLE_NAME);
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
latestCatalogVersion, latestCatalogVersion),
+ contains(index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME))
+ );
+
+ assertThat(
+ collectIndexes(catalogManager, tableId,
earliestCatalogVersion, latestCatalogVersion),
+ contains(
+ index(catalogManager, latestCatalogVersion,
PK_INDEX_NAME),
+ index(catalogManager,
catalogVersionBeforeDropIndex0, indexName0),
+ index(catalogManager,
catalogVersionBeforeDropIndex1, indexName1)
+ )
+ );
+ });
+ }
+
+ @Test
+ void testCollectIndexesComplexCase() throws Exception {
Review Comment:
What do you mean by "complex case"? Please leave some comments
--
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]
