You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by rp...@apache.org on 2025/03/27 16:09:17 UTC
(ignite-3) branch main updated: IGNITE-24938 Ignore attempts to create index storage in destroyed partition (#5509)
This is an automated email from the ASF dual-hosted git repository.
rpuch 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 96c14005392 IGNITE-24938 Ignore attempts to create index storage in destroyed partition (#5509)
96c14005392 is described below
commit 96c14005392372a6b815ea6eaa0953b5e6d02f44
Author: Roman Puchkovskiy <ro...@gmail.com>
AuthorDate: Thu Mar 27 20:09:10 2025 +0400
IGNITE-24938 Ignore attempts to create index storage in destroyed partition (#5509)
---
...dexRegistrationAndRebalanceConcurrencyTest.java | 61 ++++++++++++++++++++
.../ignite/internal/sql/api/ItSqlApiBaseTest.java | 10 ----
.../storage/AbstractMvTableStorageTest.java | 65 +++++++++++++++++++---
.../internal/storage/impl/TestMvTableStorage.java | 22 +++++---
.../pagememory/AbstractPageMemoryTableStorage.java | 14 ++---
.../storage/rocksdb/RocksDbTableStorage.java | 20 ++++---
.../internal/table/distributed/TableManager.java | 9 +--
7 files changed, 157 insertions(+), 44 deletions(-)
diff --git a/modules/index/src/integrationTest/java/org/apache/ignite/internal/index/ItIndexRegistrationAndRebalanceConcurrencyTest.java b/modules/index/src/integrationTest/java/org/apache/ignite/internal/index/ItIndexRegistrationAndRebalanceConcurrencyTest.java
new file mode 100644
index 00000000000..fb17de29b4c
--- /dev/null
+++ b/modules/index/src/integrationTest/java/org/apache/ignite/internal/index/ItIndexRegistrationAndRebalanceConcurrencyTest.java
@@ -0,0 +1,61 @@
+/*
+ * 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 org.apache.ignite.internal.catalog.CatalogService.DEFAULT_STORAGE_PROFILE;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+import java.time.Duration;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.ClusterPerTestIntegrationTest;
+import org.junit.jupiter.api.Test;
+
+class ItIndexRegistrationAndRebalanceConcurrencyTest extends ClusterPerTestIntegrationTest {
+ private static final String TEST_ZONE_NAME = "test_zone";
+ private static final String TEST_TABLE_NAME = "test_table";
+
+ /**
+ * This makes sure that partitions coming in/going out during index registration do not break anything.
+ */
+ @Test
+ void indexRegistrationAndRebalanceConcurrency() {
+ Ignite ignite = node(0);
+
+ ignite.sql().executeScript(
+ "CREATE ZONE " + TEST_ZONE_NAME + " WITH replicas=3, storage_profiles='" + DEFAULT_STORAGE_PROFILE + "';"
+ + "CREATE TABLE " + TEST_TABLE_NAME + " (key INT PRIMARY KEY, val VARCHAR(20)) ZONE test_zone"
+ );
+
+ waitForStoragesToBeCreated(ignite);
+
+ initiateRebalanceOnTestZone(ignite);
+
+ assertTimeoutPreemptively(
+ Duration.ofSeconds(30),
+ () -> ignite.sql().executeScript("CREATE INDEX test_index on " + TEST_TABLE_NAME + " (val)")
+ );
+ }
+
+ private static void waitForStoragesToBeCreated(Ignite ignite) {
+ ignite.sql().execute(null, "SELECT * FROM " + TEST_TABLE_NAME).close();
+ }
+
+ private static void initiateRebalanceOnTestZone(Ignite ignite) {
+ ignite.sql().executeScript("ALTER ZONE " + TEST_ZONE_NAME + " SET replicas=2");
+ }
+}
diff --git a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java
index 68e42ad85c5..2e89ed3b934 100644
--- a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java
+++ b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java
@@ -105,11 +105,6 @@ public abstract class ItSqlApiBaseTest extends BaseSqlIntegrationTest {
@Test
public void ddl() {
- if (enabledColocation()) {
- // TODO https://issues.apache.org/jira/browse/IGNITE-24885 Test hangs in case of enabledColocation
- return;
- }
-
IgniteSql sql = igniteSql();
// CREATE TABLE
@@ -350,11 +345,6 @@ public abstract class ItSqlApiBaseTest extends BaseSqlIntegrationTest {
/** Check correctness of rw and ro transactions for index scan. */
@Test
public void checkMixedTransactionsForIndex() throws Exception {
- if (enabledColocation()) {
- // TODO https://issues.apache.org/jira/browse/IGNITE-24885 Test hangs in case of enabledColocation
- return;
- }
-
sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
sql("CREATE INDEX TEST_IDX ON TEST(VAL0)");
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 699639bee2d..6e01a3df89c 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
@@ -82,6 +82,7 @@ import org.apache.ignite.internal.storage.lease.LeaseInfo;
import org.apache.ignite.internal.type.NativeTypes;
import org.apache.ignite.internal.util.Cursor;
import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
@@ -162,8 +163,6 @@ public abstract class AbstractMvTableStorageTest extends BaseMvTableStorageTest
*/
@Test
public void testCreateSortedIndex() {
- assertThrows(StorageException.class, () -> tableStorage.createSortedIndex(PARTITION_ID, sortedIdx));
-
// Index should only be available after the associated partition has been created.
tableStorage.createMvPartition(PARTITION_ID);
@@ -177,8 +176,6 @@ public abstract class AbstractMvTableStorageTest extends BaseMvTableStorageTest
*/
@Test
public void testCreateHashIndex() {
- assertThrows(StorageException.class, () -> tableStorage.createHashIndex(PARTITION_ID, hashIdx));
-
// Index should only be available after the associated partition has been created.
tableStorage.createMvPartition(PARTITION_ID);
@@ -186,6 +183,58 @@ public abstract class AbstractMvTableStorageTest extends BaseMvTableStorageTest
assertThat(tableStorage.getIndex(PARTITION_ID, hashIdx.id()), is(notNullValue()));
}
+ @Test
+ @Disabled("IGNITE-24926")
+ public void sortedIndexCreationInAbsentPartitionFails() {
+ StorageException ex = assertThrows(StorageException.class, () -> tableStorage.createSortedIndex(PARTITION_ID, sortedIdx));
+ assertThat(ex.getMessage(), is("Partition ID " + PARTITION_ID + " does not exist"));
+ }
+
+ @Test
+ @Disabled("IGNITE-24926")
+ public void hashIndexCreationInAbsentPartitionFails() {
+ StorageException ex = assertThrows(StorageException.class, () -> tableStorage.createHashIndex(PARTITION_ID, hashIdx));
+ assertThat(ex.getMessage(), is("Partition ID " + PARTITION_ID + " does not exist"));
+ }
+
+ @Test
+ @Disabled("IGNITE-24926")
+ public void sortedIndexCreationInDestroyedPartitionFails() {
+ assertThat(tableStorage.createMvPartition(PARTITION_ID), willCompleteSuccessfully());
+ assertThat(tableStorage.destroyPartition(PARTITION_ID), willCompleteSuccessfully());
+
+ StorageException ex = assertThrows(StorageException.class, () -> tableStorage.createSortedIndex(PARTITION_ID, sortedIdx));
+ assertThat(ex.getMessage(), is("Partition ID " + PARTITION_ID + " does not exist"));
+ }
+
+ @Test
+ @Disabled("IGNITE-24926")
+ public void hashIndexCreationInDestroyedPartitionFails() {
+ assertThat(tableStorage.createMvPartition(PARTITION_ID), willCompleteSuccessfully());
+ assertThat(tableStorage.destroyPartition(PARTITION_ID), willCompleteSuccessfully());
+
+ StorageException ex = assertThrows(StorageException.class, () -> tableStorage.createHashIndex(PARTITION_ID, hashIdx));
+ assertThat(ex.getMessage(), is("Partition ID " + PARTITION_ID + " does not exist"));
+ }
+
+ // TODO: IGNITE-24926 - remove this test.
+ @Test
+ public void sortedIndexCreationInDestroyedPartitionQueitlyDoesNothing() {
+ assertThat(tableStorage.createMvPartition(PARTITION_ID), willCompleteSuccessfully());
+ assertThat(tableStorage.destroyPartition(PARTITION_ID), willCompleteSuccessfully());
+
+ assertDoesNotThrow(() -> tableStorage.createSortedIndex(PARTITION_ID, sortedIdx));
+ }
+
+ // TODO: IGNITE-24926 - remove this test.
+ @Test
+ public void hashIndexCreationInDestroyedPartitionQueitlyDoesNothing() {
+ assertThat(tableStorage.createMvPartition(PARTITION_ID), willCompleteSuccessfully());
+ assertThat(tableStorage.destroyPartition(PARTITION_ID), willCompleteSuccessfully());
+
+ assertDoesNotThrow(() -> tableStorage.createHashIndex(PARTITION_ID, hashIdx));
+ }
+
/**
* Tests destroying an index.
*/
@@ -404,7 +453,8 @@ public abstract class AbstractMvTableStorageTest extends BaseMvTableStorageTest
tableStorage.createHashIndex(PARTITION_ID, hashIdx);
assertThat(tableStorage.getIndex(PARTITION_ID, hashIdx.id()), is(notNullValue()));
- assertThrows(StorageException.class, () -> tableStorage.createHashIndex(PARTITION_ID + 1, hashIdx));
+ // TODO: IGNITE-24926 - uncomment this assertion.
+ // assertThrows(StorageException.class, () -> tableStorage.createHashIndex(PARTITION_ID + 1, hashIdx));
MvPartitionStorage partitionStorage2 = getOrCreateMvPartition(PARTITION_ID + 1);
@@ -1171,8 +1221,9 @@ public abstract class AbstractMvTableStorageTest extends BaseMvTableStorageTest
// Let's check that we won't get destroyed storages.
assertNull(tableStorage.getMvPartition(PARTITION_ID));
- assertThrows(StorageException.class, () -> tableStorage.createHashIndex(PARTITION_ID, hashIdx));
- assertThrows(StorageException.class, () -> tableStorage.createSortedIndex(PARTITION_ID, sortedIdx));
+ // TODO: IGNITE-24926 - uncomment these assertions.
+ // assertThrows(StorageException.class, () -> tableStorage.createHashIndex(PARTITION_ID, hashIdx));
+ // assertThrows(StorageException.class, () -> tableStorage.createSortedIndex(PARTITION_ID, sortedIdx));
checkStorageDestroyed(mvPartitionStorage);
checkStorageDestroyed(hashIndexStorage);
diff --git a/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/impl/TestMvTableStorage.java b/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/impl/TestMvTableStorage.java
index 09ced978ec9..8dd38314b70 100644
--- a/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/impl/TestMvTableStorage.java
+++ b/modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/impl/TestMvTableStorage.java
@@ -123,6 +123,10 @@ public class TestMvTableStorage implements MvTableStorage {
return IgniteUtils.inBusyLock(busyLock, fn);
}
+ private void inBusyLock(Runnable fn) {
+ IgniteUtils.inBusyLock(busyLock, fn);
+ }
+
@Override
public CompletableFuture<MvPartitionStorage> createMvPartition(int partitionId) {
return inBusyLock(() -> mvPartitionStorages.create(partitionId, partId -> spy(new TestMvPartitionStorage(partId))));
@@ -170,14 +174,15 @@ public class TestMvTableStorage implements MvTableStorage {
@Override
public void createSortedIndex(int partitionId, StorageSortedIndexDescriptor indexDescriptor) {
- inBusyLock(() -> getOrCreateSortedIndexBusy(partitionId, indexDescriptor));
+ inBusyLock(() -> createSortedIndexBusy(partitionId, indexDescriptor));
}
- private TestSortedIndexStorage getOrCreateSortedIndexBusy(int partitionId, StorageSortedIndexDescriptor indexDescriptor) {
+ private void createSortedIndexBusy(int partitionId, StorageSortedIndexDescriptor indexDescriptor) {
TestMvPartitionStorage mvPartitionStorage = mvPartitionStorages.get(partitionId);
+ // TODO: IGNITE-24926 - throw StorageException is mvPartitionStorage is null.
if (mvPartitionStorage == null) {
- throw new StorageException(createMissingMvPartitionErrorMessage(partitionId));
+ return;
}
SortedIndices sortedIndices = sortedIndicesById.computeIfAbsent(
@@ -185,19 +190,20 @@ public class TestMvTableStorage implements MvTableStorage {
id -> new SortedIndices(indexDescriptor)
);
- return sortedIndices.getOrCreateStorage(partitionId);
+ sortedIndices.getOrCreateStorage(partitionId);
}
@Override
public void createHashIndex(int partitionId, StorageHashIndexDescriptor indexDescriptor) {
- inBusyLock(() -> getOrCreateHashIndexBusy(partitionId, indexDescriptor));
+ inBusyLock(() -> createHashIndexBusy(partitionId, indexDescriptor));
}
- private TestHashIndexStorage getOrCreateHashIndexBusy(int partitionId, StorageHashIndexDescriptor indexDescriptor) {
+ private void createHashIndexBusy(int partitionId, StorageHashIndexDescriptor indexDescriptor) {
TestMvPartitionStorage mvPartitionStorage = mvPartitionStorages.get(partitionId);
+ // TODO: IGNITE-24926 - throw StorageException is mvPartitionStorage is null.
if (mvPartitionStorage == null) {
- throw new StorageException(createMissingMvPartitionErrorMessage(partitionId));
+ return;
}
HashIndices sortedIndices = hashIndicesById.computeIfAbsent(
@@ -205,7 +211,7 @@ public class TestMvTableStorage implements MvTableStorage {
id -> new HashIndices(indexDescriptor)
);
- return sortedIndices.getOrCreateStorage(partitionId);
+ sortedIndices.getOrCreateStorage(partitionId);
}
@Override
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 d60af04da1f..e464ff50478 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
@@ -173,11 +173,10 @@ public abstract class AbstractPageMemoryTableStorage implements MvTableStorage {
busy(() -> {
AbstractPageMemoryMvPartitionStorage partitionStorage = mvPartitionStorages.get(partitionId);
- if (partitionStorage == null) {
- throw new StorageException(createMissingMvPartitionErrorMessage(partitionId));
+ // TODO: IGNITE-24926 - throw StorageException if partitionStorage is absent.
+ if (partitionStorage != null) {
+ partitionStorage.createSortedIndex(indexDescriptor);
}
-
- partitionStorage.createSortedIndex(indexDescriptor);
});
}
@@ -186,11 +185,10 @@ public abstract class AbstractPageMemoryTableStorage implements MvTableStorage {
busy(() -> {
AbstractPageMemoryMvPartitionStorage partitionStorage = mvPartitionStorages.get(partitionId);
- if (partitionStorage == null) {
- throw new StorageException(createMissingMvPartitionErrorMessage(partitionId));
+ // TODO: IGNITE-24926 - throw StorageException if partitionStorage is absent.
+ if (partitionStorage != null) {
+ partitionStorage.createHashIndex(indexDescriptor);
}
-
- partitionStorage.createHashIndex(indexDescriptor);
});
}
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 b3837a22357..6f26f0e79ec 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
@@ -257,18 +257,20 @@ public class RocksDbTableStorage implements MvTableStorage {
@Override
public void createSortedIndex(int partitionId, StorageSortedIndexDescriptor indexDescriptor) {
inBusyLock(busyLock, () -> {
- checkPartitionExists(partitionId);
-
- indexes.createSortedIndex(partitionId, indexDescriptor);
+ // TODO: IGNITE-24926 - invoke checkPartitionExists() instead to throw StorageException is partition is absent.
+ if (partitionExists(partitionId)) {
+ indexes.createSortedIndex(partitionId, indexDescriptor);
+ }
});
}
@Override
public void createHashIndex(int partitionId, StorageHashIndexDescriptor indexDescriptor) {
inBusyLock(busyLock, () -> {
- checkPartitionExists(partitionId);
-
- indexes.createHashIndex(partitionId, indexDescriptor);
+ // TODO: IGNITE-24926 - invoke checkPartitionExists() instead to throw StorageException is partition is absent.
+ if (partitionExists(partitionId)) {
+ indexes.createHashIndex(partitionId, indexDescriptor);
+ }
});
}
@@ -390,11 +392,15 @@ public class RocksDbTableStorage implements MvTableStorage {
}
private void checkPartitionExists(int partitionId) {
- if (mvPartitionStorages.get(partitionId) == null) {
+ if (!partitionExists(partitionId)) {
throw new StorageException(createMissingMvPartitionErrorMessage(partitionId));
}
}
+ private boolean partitionExists(int partitionId) {
+ return mvPartitionStorages.get(partitionId) != null;
+ }
+
@Override
public @Nullable IndexStorage getIndex(int partitionId, int indexId) {
return inBusyLock(busyLock, () -> {
diff --git a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
index 68d69f1359c..97eef3011a2 100644
--- a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
+++ b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
@@ -2831,6 +2831,7 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
.get(causalityToken)
.thenCompose(ignore -> {
TableImpl table = tables.get(tablePartitionId.tableId());
+ assert table != null : tablePartitionId;
return stopAndDestroyTablePartition(tablePartitionId, table);
});
@@ -2865,10 +2866,6 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
* @return Future that will be completed after all resources have been closed.
*/
private CompletableFuture<Void> stopTablePartition(TablePartitionId tablePartitionId, TableImpl table) {
- if (table != null) {
- closePartitionTrackers(table.internalTable(), tablePartitionId.partitionId());
- }
-
CompletableFuture<Boolean> stopReplicaFuture;
try {
@@ -2883,6 +2880,8 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
return stopReplicaFuture
.thenCompose(v -> {
+ closePartitionTrackers(table.internalTable(), tablePartitionId.partitionId());
+
minTimeCollectorService.removePartition(tablePartitionId);
return mvGc.removeStorage(tablePartitionId);
});
@@ -2911,6 +2910,7 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
destroyFutures.add(runAsync(() -> destroyReplicationProtocolStorages(tablePartitionId, table), ioExecutor));
}
+ // TODO: IGNITE-24926 - reduce set in localPartsByTableId after storages destruction.
return allOf(destroyFutures.toArray(new CompletableFuture[]{}));
}
@@ -3142,6 +3142,7 @@ public class TableManager implements IgniteTablesInternal, IgniteComponent {
public CompletableFuture<Void> restartPartition(TablePartitionId tablePartitionId, long revision, long assignmentsTimestamp) {
return inBusyLockAsync(busyLock, () -> tablesVv.get(revision).thenComposeAsync(unused -> inBusyLockAsync(busyLock, () -> {
TableImpl table = tables.get(tablePartitionId.tableId());
+ assert table != null : tablePartitionId;
return stopPartitionForRestart(tablePartitionId, table).thenComposeAsync(unused1 -> {
Assignments stableAssignments = stableAssignmentsGetLocally(metaStorageMgr, tablePartitionId, revision);