You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "SammyVimes (via GitHub)" <gi...@apache.org> on 2023/06/09 20:44:24 UTC

[GitHub] [ignite-3] SammyVimes opened a new pull request, #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

SammyVimes opened a new pull request, #2177:
URL: https://github.com/apache/ignite-3/pull/2177

   https://issues.apache.org/jira/browse/IGNITE-19363


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1230542972


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1834,6 +1853,23 @@ public CompletableFuture<TableImpl> tableAsync(int id) {
         }
     }
 
+    /**
+     * Asynchronously gets the partitions set of a table using causality token.
+     *
+     * @param causalityToken Causality token.
+     * @return Future.
+     */
+    public CompletableFuture<PartitionSet> partitionSetAsync(long causalityToken, int tableId) {

Review Comment:
   No, at least not by design of the versioned value



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes merged pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes merged PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1230536814


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -682,14 +692,20 @@ private CompletableFuture<?> createTablePartitionsLocally(
         int partitions = newAssignments.size();
 
         CompletableFuture<?>[] futures = new CompletableFuture<?>[partitions];
+
+        // TODO: https://issues.apache.org/jira/browse/IGNITE-19713 Process assignments and set partitions only for assigned partitions.
+        PartitionSet parts = new BitSetPartitionSet();

Review Comment:
   We don't need such a constructor. Right now we do iterate over all the partitions, but it's just incorrect (see ToDo)



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1228022120


##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -268,7 +268,6 @@ CompletableFuture<Void> finishRebalancePartition(
      * @return Index Storage.
      * @throws StorageException If the given partition does not exist.
      */
-    // TODO: IGNITE-19112 Change or get rid of

Review Comment:
   The ticket is not yet completed, why did you remove it?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/PartitionSet.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.table.distributed;
+
+import java.util.stream.IntStream;
+
+/**
+ * Represents a collection of partition indices.

Review Comment:
   What is the difference between a partition index and ID?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/PartitionSet.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.table.distributed;
+
+import java.util.stream.IntStream;
+
+/**
+ * Represents a collection of partition indices.
+ */
+public interface PartitionSet {
+    /**
+     * Adds the partition to the partition set.
+     *
+     * @param partitionIndex Partition index.
+     */
+    void set(int partitionIndex);
+
+    /**
+     * Returns {@code true} if partition with {@code partitionIndex} is present in this set, {@code false} otherwise.
+     *
+     * @param partitionIndex Partition index.
+     */
+    boolean get(int partitionIndex);
+
+    /**
+     * Returns a stream of indices for which this set contains a bit in the set state.
+     * The indices are returned in order, from lowest to highest.
+     */
+    IntStream stream();
+
+    PartitionSet copy();

Review Comment:
   Missing javadoc



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/BitSetPartitionSet.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.table.distributed;
+
+import java.util.BitSet;
+import java.util.stream.IntStream;
+
+/**
+ * {@link BitSet} implementation of the {@link PartitionSet}.
+ */
+public class BitSetPartitionSet implements PartitionSet {

Review Comment:
   Missing unit tests



##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -71,8 +73,7 @@ public class TableImpl implements Table {
 
     private final Map<Integer, CompletableFuture<?>> indexesToWait = new ConcurrentHashMap<>();
 
-    private final Map<Integer, IndexStorageAdapterFactory> indexStorageAdapterFactories = new ConcurrentHashMap<>();
-    private final Map<Integer, IndexLockerFactory> indexLockerFactories = new ConcurrentHashMap<>();
+    private final Map<Integer, IndexWrapper> indexStorageAdapters = new ConcurrentHashMap<>();

Review Comment:
   I suggest renaming to **indexWrapperById**



##########
modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java:
##########
@@ -89,6 +90,16 @@ public void setUp() {
             return completedFuture(new TableImpl(tbl, new HeapLockManager()));
         });
 
+        when(tableManagerMock.getTable(anyInt())).thenAnswer(inv -> {
+            InternalTable tbl = mock(InternalTable.class);
+
+            Mockito.doReturn(inv.getArgument(0)).when(tbl).tableId();
+
+            return new TableImpl(tbl, new HeapLockManager());
+        });
+
+        when(tableManagerMock.partitionSetAsync(anyLong(), anyInt())).thenReturn(completedFuture(new BitSetPartitionSet()));

Review Comment:
   Maybe add `BitSetPartitionSet.EMPTY` ?



##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -465,11 +466,17 @@ private CompletableFuture<?> createIndexLocally(
         CompletableFuture<?> fireEventFuture =
                 fireEvent(IndexEvent.CREATE, new IndexEventParameters(causalityToken, tableId, indexId, eventIndexDescriptor));
 
-        CompletableFuture<TableImpl> tableFuture = tableManager.tableAsync(causalityToken, tableId);
+        TableImpl table = tableManager.getTable(tableId);
 
-        CompletableFuture<SchemaRegistry> schemaRegistryFuture = schemaManager.schemaRegistry(causalityToken, tableId);
+        assert table != null;
 
-        CompletableFuture<?> createIndexFuture = tableFuture.thenAcceptBoth(schemaRegistryFuture, (table, schemaRegistry) -> {
+        CompletableFuture<SchemaRegistry> schemaRegistryFut = schemaManager.schemaRegistry(causalityToken, tableId);
+
+        // TODO: https://issues.apache.org/jira/browse/IGNITE-19712 Listen to assignment changes and start new index storages
+        // For now we only create index storages once here.

Review Comment:
   We create indexes not only here, but also here: `org.apache.ignite.internal.table.distributed.replicator.PartitionReplicaListener#startBuildIndex`



##########
modules/index/src/test/java/org/apache/ignite/internal/index/IndexManagerTest.java:
##########
@@ -89,6 +90,16 @@ public void setUp() {
             return completedFuture(new TableImpl(tbl, new HeapLockManager()));
         });
 
+        when(tableManagerMock.getTable(anyInt())).thenAnswer(inv -> {
+            InternalTable tbl = mock(InternalTable.class);
+
+            Mockito.doReturn(inv.getArgument(0)).when(tbl).tableId();

Review Comment:
   use static import



##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/TableManagerTest.java:
##########
@@ -662,12 +661,6 @@ private void testStoragesGetClearedInMiddleOfFailedRebalance(boolean isTxStorage
             });
         }).join();

Review Comment:
   Please change to `assertThat(changeTablesConfigFututre, willSucceedFast());`



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -277,6 +279,11 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
      */
     private final Map<Integer, TableImpl> pendingTables = new ConcurrentHashMap<>();
 
+    /**
+     * Started tables.
+     */

Review Comment:
   ```suggestion
       /** Started tables. */
   ```



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -682,14 +692,20 @@ private CompletableFuture<?> createTablePartitionsLocally(
         int partitions = newAssignments.size();
 
         CompletableFuture<?>[] futures = new CompletableFuture<?>[partitions];
+
+        // TODO: https://issues.apache.org/jira/browse/IGNITE-19713 Process assignments and set partitions only for assigned partitions.
+        PartitionSet parts = new BitSetPartitionSet();

Review Comment:
   maybe `new BitSetPartitionSet(partitions);`



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1834,6 +1853,23 @@ public CompletableFuture<TableImpl> tableAsync(int id) {
         }
     }
 
+    /**
+     * Asynchronously gets the partitions set of a table using causality token.
+     *
+     * @param causalityToken Causality token.
+     * @return Future.
+     */
+    public CompletableFuture<PartitionSet> partitionSetAsync(long causalityToken, int tableId) {
+        if (!busyLock.enterBusy()) {
+            throw new IgniteException(new NodeStoppingException());
+        }
+        try {

Review Comment:
   ```suggestion
           }
           
           try {
   ```



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2337,32 +2389,49 @@ private static PeersAndLearners configurationFromAssignments(Collection<Assignme
     }
 
     /**
-     * Creates partition stores. If one of the storages has not completed the rebalance, then the storages are cleared.
+     * Gets partition stores.
      *
      * @param table Table.
      * @param partitionId Partition ID.
-     * @return Future of creating or getting partition stores.
+     * @return PartitionStorages.
      */
-    // TODO: IGNITE-18939 Create storages only once, then only get them
-    private CompletableFuture<PartitionStorages> getOrCreatePartitionStorages(TableImpl table, int partitionId) {
+    private PartitionStorages getPartitionStorages(TableImpl table, int partitionId) {
         InternalTable internalTable = table.internalTable();
 
         MvPartitionStorage mvPartition = internalTable.storage().getMvPartition(partitionId);
 
-        return (mvPartition != null ? completedFuture(mvPartition) : internalTable.storage().createMvPartition(partitionId))
-                .thenComposeAsync(mvPartitionStorage -> {
-                    TxStateStorage txStateStorage = internalTable.txStateStorage().getOrCreateTxStateStorage(partitionId);
+        assert mvPartition != null;
 
-                    if (mvPartitionStorage.persistedIndex() == MvPartitionStorage.REBALANCE_IN_PROGRESS
-                            || txStateStorage.persistedIndex() == TxStateStorage.REBALANCE_IN_PROGRESS) {
-                        return allOf(
-                                internalTable.storage().clearPartition(partitionId),
-                                txStateStorage.clear()
-                        ).thenApply(unused -> new PartitionStorages(mvPartitionStorage, txStateStorage));
-                    } else {
-                        return completedFuture(new PartitionStorages(mvPartitionStorage, txStateStorage));
-                    }
-                }, ioExecutor);
+        TxStateStorage txStateStorage = internalTable.txStateStorage().getTxStateStorage(partitionId);
+
+        assert txStateStorage != null;
+
+        return new PartitionStorages(mvPartition, txStateStorage);
+    }
+
+    private CompletableFuture<Void> createPartitionStorages(TableImpl table, PartitionSet partitions) {

Review Comment:
   In this method, you create storages if they have not been created, which is not correct in our case, the creation of storages should occur once and the method must explicitly create storages.



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2501,4 +2570,13 @@ private PartitionUpdateHandlers createPartitionUpdateHandlers(
 
         return new PartitionUpdateHandlers(storageUpdateHandler, indexUpdateHandler, gcUpdateHandler);
     }
+
+    /**
+     * Returns table instance.
+     *
+     * @param tableId Table id.
+     */
+    public TableImpl getTable(int tableId) {

Review Comment:
   can it be `null`?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -322,40 +312,96 @@ public void registerHashIndex(
      */
     public void registerSortedIndex(
             StorageSortedIndexDescriptor indexDescriptor,
-            Function<BinaryRow, BinaryTuple> searchRowResolver
+            Function<BinaryRow, BinaryTuple> searchRowResolver,
+            PartitionSet partitions
     ) {
         int indexId = indexDescriptor.id();
 
-        indexLockerFactories.put(
-                indexId,
-                partitionId -> new SortedIndexLocker(
-                        indexId,
-                        partitionId,
-                        lockManager,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
-        indexStorageAdapterFactories.put(
-                indexId,
-                partitionId -> new TableSchemaAwareIndexStorage(
-                        indexId,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
+        partitions.stream().forEach(partitionId -> {
+            tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor);
+        });
+
+        indexStorageAdapters.put(indexId, new SortedIndexWrapper(indexId, searchRowResolver));
 
         completeWaitIndex(indexId);
     }
 
+    /** Class that creates index storage and locker decorators for given partition. */
+    private abstract static class IndexWrapper {
+        final int indexId;
+        final Function<BinaryRow, BinaryTuple> indexRowResolver;
+
+        private IndexWrapper(int indexId, Function<BinaryRow, BinaryTuple> indexRowResolver) {
+            this.indexId = indexId;
+            this.indexRowResolver = indexRowResolver;
+        }
+
+        abstract TableSchemaAwareIndexStorage getStorage(int partitionId);
+
+        abstract IndexLocker getLocker(int partitionId);

Review Comment:
   createLocker



##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -322,40 +312,96 @@ public void registerHashIndex(
      */
     public void registerSortedIndex(
             StorageSortedIndexDescriptor indexDescriptor,
-            Function<BinaryRow, BinaryTuple> searchRowResolver
+            Function<BinaryRow, BinaryTuple> searchRowResolver,
+            PartitionSet partitions
     ) {
         int indexId = indexDescriptor.id();
 
-        indexLockerFactories.put(
-                indexId,
-                partitionId -> new SortedIndexLocker(
-                        indexId,
-                        partitionId,
-                        lockManager,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
-        indexStorageAdapterFactories.put(
-                indexId,
-                partitionId -> new TableSchemaAwareIndexStorage(
-                        indexId,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
+        partitions.stream().forEach(partitionId -> {
+            tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor);
+        });
+
+        indexStorageAdapters.put(indexId, new SortedIndexWrapper(indexId, searchRowResolver));
 
         completeWaitIndex(indexId);
     }
 
+    /** Class that creates index storage and locker decorators for given partition. */
+    private abstract static class IndexWrapper {

Review Comment:
   Let's move this class and its successors into separate classes.



##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -322,40 +312,96 @@ public void registerHashIndex(
      */
     public void registerSortedIndex(
             StorageSortedIndexDescriptor indexDescriptor,
-            Function<BinaryRow, BinaryTuple> searchRowResolver
+            Function<BinaryRow, BinaryTuple> searchRowResolver,
+            PartitionSet partitions
     ) {
         int indexId = indexDescriptor.id();
 
-        indexLockerFactories.put(
-                indexId,
-                partitionId -> new SortedIndexLocker(
-                        indexId,
-                        partitionId,
-                        lockManager,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
-        indexStorageAdapterFactories.put(
-                indexId,
-                partitionId -> new TableSchemaAwareIndexStorage(
-                        indexId,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
+        partitions.stream().forEach(partitionId -> {
+            tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor);
+        });
+
+        indexStorageAdapters.put(indexId, new SortedIndexWrapper(indexId, searchRowResolver));
 
         completeWaitIndex(indexId);
     }
 
+    /** Class that creates index storage and locker decorators for given partition. */
+    private abstract static class IndexWrapper {
+        final int indexId;
+        final Function<BinaryRow, BinaryTuple> indexRowResolver;
+
+        private IndexWrapper(int indexId, Function<BinaryRow, BinaryTuple> indexRowResolver) {
+            this.indexId = indexId;
+            this.indexRowResolver = indexRowResolver;
+        }
+
+        abstract TableSchemaAwareIndexStorage getStorage(int partitionId);

Review Comment:
   createStorage



##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -269,7 +270,7 @@ public Supplier<Map<Integer, IndexLocker>> indexesLockers(int partId) {
     /**
      * The future completes when the primary key index is ready to use.
      *
-     * @return Future whcih complete when a primary index for the table is .
+     * @return Future which complete when a primary index for the table is .

Review Comment:
   Lol



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/PartitionSet.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.table.distributed;
+
+import java.util.stream.IntStream;
+
+/**
+ * Represents a collection of partition indices.
+ */
+public interface PartitionSet {

Review Comment:
   You have only one implementation, why a separate interface?



##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -465,11 +466,17 @@ private CompletableFuture<?> createIndexLocally(
         CompletableFuture<?> fireEventFuture =
                 fireEvent(IndexEvent.CREATE, new IndexEventParameters(causalityToken, tableId, indexId, eventIndexDescriptor));
 
-        CompletableFuture<TableImpl> tableFuture = tableManager.tableAsync(causalityToken, tableId);
+        TableImpl table = tableManager.getTable(tableId);
 
-        CompletableFuture<SchemaRegistry> schemaRegistryFuture = schemaManager.schemaRegistry(causalityToken, tableId);
+        assert table != null;

Review Comment:
   ```suggestion
           assert table != null : tableId;
   ```



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -265,6 +264,9 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
      */
     private final IncrementalVersionedValue<Map<Integer, TableImpl>> tablesByIdVv;
 
+    /** Versioned store for partition storage by table id. */
+    private final IncrementalVersionedValue<Map<Integer, PartitionSet>> partStoragesByIdVv;

Review Comment:
   These are not storages, but simply partitions that are currently on the node, the name of the variable and the description are very confusing.



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -706,123 +722,112 @@ private CompletableFuture<?> createTablePartitionsLocally(
 
                 TablePartitionId replicaGrpId = new TablePartitionId(tblId, partId);
 
-                placementDriver.updateAssignment(replicaGrpId, newConfiguration.peers().stream().map(Peer::consistentId).collect(toList()));
+                placementDriver.updateAssignment(replicaGrpId, newConfiguration.peers().stream().map(Peer::consistentId)
+                        .collect(toList()));
 
-                PendingComparableValuesTracker<HybridTimestamp, Void> safeTimeTracker =
-                        new PendingComparableValuesTracker<>(new HybridTimestamp(1, 0));
-                PendingComparableValuesTracker<Long, Void> storageIndexTracker =
-                        new PendingComparableValuesTracker<>(0L);
+                var safeTimeTracker = new PendingComparableValuesTracker<HybridTimestamp, Void>(
+                        new HybridTimestamp(1, 0)
+                );
+                var storageIndexTracker = new PendingComparableValuesTracker<Long, Void>(0L);
 
                 ((InternalTableImpl) internalTbl).updatePartitionTrackers(partId, safeTimeTracker, storageIndexTracker);
 
-                CompletableFuture<PartitionStorages> partitionStoragesFut = getOrCreatePartitionStorages(table, partId);
+                PartitionStorages partitionStorages = getPartitionStorages(table, partId);
 
-                CompletableFuture<PartitionDataStorage> partitionDataStorageFut = partitionStoragesFut
-                        .thenApply(partitionStorages -> partitionDataStorage(partitionStorages.getMvPartitionStorage(),
-                                internalTbl, partId));
+                PartitionDataStorage partitionDataStorage = partitionDataStorage(partitionStorages.getMvPartitionStorage(),
+                        internalTbl, partId);
 
-                CompletableFuture<PartitionUpdateHandlers> partitionUpdateHandlersFut = partitionDataStorageFut
-                        .thenApply(storage -> {
-                            PartitionUpdateHandlers partitionUpdateHandlers = createPartitionUpdateHandlers(
-                                    partId,
-                                    storage,
-                                    table,
-                                    dsCfg,
-                                    safeTimeTracker
-                            );
+                PartitionUpdateHandlers partitionUpdateHandlers = createPartitionUpdateHandlers(
+                        partId,
+                        partitionDataStorage,
+                        table,
+                        dsCfg,
+                        safeTimeTracker
+                );
 
-                            mvGc.addStorage(replicaGrpId, partitionUpdateHandlers.gcUpdateHandler);
-
-                            return partitionUpdateHandlers;
-                        });
+                mvGc.addStorage(replicaGrpId, partitionUpdateHandlers.gcUpdateHandler);
 
                 CompletableFuture<Void> startGroupFut;
 
                 // start new nodes, only if it is table creation, other cases will be covered by rebalance logic
                 if (localMemberAssignment != null) {
-                    startGroupFut = partitionStoragesFut.thenComposeAsync(partitionStorages -> {
-                        CompletableFuture<Boolean> fut;
-
-                        // If Raft is running in in-memory mode or the PDS has been cleared, we need to remove the current node
-                        // from the Raft group in order to avoid the double vote problem.
-                        // <MUTED> See https://issues.apache.org/jira/browse/IGNITE-16668 for details.
-                        // TODO: https://issues.apache.org/jira/browse/IGNITE-19046 Restore "|| !hasData"
-                        if (internalTbl.storage().isVolatile()) {
-                            fut = queryDataNodesCount(tblId, partId, newConfiguration.peers()).thenApply(dataNodesCount -> {
-                                boolean fullPartitionRestart = dataNodesCount == 0;
-
-                                if (fullPartitionRestart) {
-                                    return true;
-                                }
+                    CompletableFuture<Boolean> fut;

Review Comment:
   Please rename this



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -682,14 +692,20 @@ private CompletableFuture<?> createTablePartitionsLocally(
         int partitions = newAssignments.size();
 
         CompletableFuture<?>[] futures = new CompletableFuture<?>[partitions];
+
+        // TODO: https://issues.apache.org/jira/browse/IGNITE-19713 Process assignments and set partitions only for assigned partitions.
+        PartitionSet parts = new BitSetPartitionSet();
+
         for (int i = 0; i < futures.length; i++) {
             futures[i] = new CompletableFuture<>();
+
+            parts.set(i);
         }
 
         String localMemberName = localNode().name();
 
         // Create new raft nodes according to new assignments.
-        Supplier<CompletableFuture<Void>> updateAssignmentsClosure = () -> {
+        Supplier<CompletableFuture<Void>>  updateAssignmentsClosure = () -> {

Review Comment:
   ```suggestion
           Supplier<CompletableFuture<Void>> updateAssignmentsClosure = () -> {
   ```



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1282,6 +1297,17 @@ private void dropTableLocally(long causalityToken, String name, int tblId, int p
                 removeStorageFromGcFutures[p] = mvGc.removeStorage(replicationGroupId);
             }
 
+            partStoragesByIdVv.update(causalityToken, (previousVal, e) -> inBusyLock(busyLock, () -> {
+                if (e != null) {
+                    return failedFuture(e);
+                }
+
+                HashMap<Integer, PartitionSet> newMap = new HashMap<>(previousVal);

Review Comment:
   ```suggestion
                   var newMap = new HashMap<>(previousVal);
   ```



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1834,6 +1853,23 @@ public CompletableFuture<TableImpl> tableAsync(int id) {
         }
     }
 
+    /**
+     * Asynchronously gets the partitions set of a table using causality token.
+     *
+     * @param causalityToken Causality token.
+     * @return Future.
+     */
+    public CompletableFuture<PartitionSet> partitionSetAsync(long causalityToken, int tableId) {

Review Comment:
   Can the future return **null**?



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1230543701


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2337,32 +2389,49 @@ private static PeersAndLearners configurationFromAssignments(Collection<Assignme
     }
 
     /**
-     * Creates partition stores. If one of the storages has not completed the rebalance, then the storages are cleared.
+     * Gets partition stores.
      *
      * @param table Table.
      * @param partitionId Partition ID.
-     * @return Future of creating or getting partition stores.
+     * @return PartitionStorages.
      */
-    // TODO: IGNITE-18939 Create storages only once, then only get them
-    private CompletableFuture<PartitionStorages> getOrCreatePartitionStorages(TableImpl table, int partitionId) {
+    private PartitionStorages getPartitionStorages(TableImpl table, int partitionId) {
         InternalTable internalTable = table.internalTable();
 
         MvPartitionStorage mvPartition = internalTable.storage().getMvPartition(partitionId);
 
-        return (mvPartition != null ? completedFuture(mvPartition) : internalTable.storage().createMvPartition(partitionId))
-                .thenComposeAsync(mvPartitionStorage -> {
-                    TxStateStorage txStateStorage = internalTable.txStateStorage().getOrCreateTxStateStorage(partitionId);
+        assert mvPartition != null;
 
-                    if (mvPartitionStorage.persistedIndex() == MvPartitionStorage.REBALANCE_IN_PROGRESS
-                            || txStateStorage.persistedIndex() == TxStateStorage.REBALANCE_IN_PROGRESS) {
-                        return allOf(
-                                internalTable.storage().clearPartition(partitionId),
-                                txStateStorage.clear()
-                        ).thenApply(unused -> new PartitionStorages(mvPartitionStorage, txStateStorage));
-                    } else {
-                        return completedFuture(new PartitionStorages(mvPartitionStorage, txStateStorage));
-                    }
-                }, ioExecutor);
+        TxStateStorage txStateStorage = internalTable.txStateStorage().getTxStateStorage(partitionId);
+
+        assert txStateStorage != null;
+
+        return new PartitionStorages(mvPartition, txStateStorage);
+    }
+
+    private CompletableFuture<Void> createPartitionStorages(TableImpl table, PartitionSet partitions) {

Review Comment:
   This is not in the scope of this ticket, however it will be done in the consequent ticket



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1230497527


##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -322,40 +312,96 @@ public void registerHashIndex(
      */
     public void registerSortedIndex(
             StorageSortedIndexDescriptor indexDescriptor,
-            Function<BinaryRow, BinaryTuple> searchRowResolver
+            Function<BinaryRow, BinaryTuple> searchRowResolver,
+            PartitionSet partitions
     ) {
         int indexId = indexDescriptor.id();
 
-        indexLockerFactories.put(
-                indexId,
-                partitionId -> new SortedIndexLocker(
-                        indexId,
-                        partitionId,
-                        lockManager,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
-        indexStorageAdapterFactories.put(
-                indexId,
-                partitionId -> new TableSchemaAwareIndexStorage(
-                        indexId,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
+        partitions.stream().forEach(partitionId -> {
+            tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor);
+        });
+
+        indexStorageAdapters.put(indexId, new SortedIndexWrapper(indexId, searchRowResolver));
 
         completeWaitIndex(indexId);
     }
 
+    /** Class that creates index storage and locker decorators for given partition. */
+    private abstract static class IndexWrapper {
+        final int indexId;
+        final Function<BinaryRow, BinaryTuple> indexRowResolver;
+
+        private IndexWrapper(int indexId, Function<BinaryRow, BinaryTuple> indexRowResolver) {
+            this.indexId = indexId;
+            this.indexRowResolver = indexRowResolver;
+        }
+
+        abstract TableSchemaAwareIndexStorage getStorage(int partitionId);

Review Comment:
   It doesn't really create a storage, but it creates a wrapper. Do you think it should be called `createStorage`?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -322,40 +312,96 @@ public void registerHashIndex(
      */
     public void registerSortedIndex(
             StorageSortedIndexDescriptor indexDescriptor,
-            Function<BinaryRow, BinaryTuple> searchRowResolver
+            Function<BinaryRow, BinaryTuple> searchRowResolver,
+            PartitionSet partitions
     ) {
         int indexId = indexDescriptor.id();
 
-        indexLockerFactories.put(
-                indexId,
-                partitionId -> new SortedIndexLocker(
-                        indexId,
-                        partitionId,
-                        lockManager,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
-        indexStorageAdapterFactories.put(
-                indexId,
-                partitionId -> new TableSchemaAwareIndexStorage(
-                        indexId,
-                        tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor),
-                        searchRowResolver
-                )
-        );
+        partitions.stream().forEach(partitionId -> {
+            tbl.storage().getOrCreateSortedIndex(partitionId, indexDescriptor);
+        });
+
+        indexStorageAdapters.put(indexId, new SortedIndexWrapper(indexId, searchRowResolver));
 
         completeWaitIndex(indexId);
     }
 
+    /** Class that creates index storage and locker decorators for given partition. */
+    private abstract static class IndexWrapper {
+        final int indexId;
+        final Function<BinaryRow, BinaryTuple> indexRowResolver;
+
+        private IndexWrapper(int indexId, Function<BinaryRow, BinaryTuple> indexRowResolver) {
+            this.indexId = indexId;
+            this.indexRowResolver = indexRowResolver;
+        }
+
+        abstract TableSchemaAwareIndexStorage getStorage(int partitionId);
+
+        abstract IndexLocker getLocker(int partitionId);

Review Comment:
   Same here



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1230535548


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -682,14 +692,20 @@ private CompletableFuture<?> createTablePartitionsLocally(
         int partitions = newAssignments.size();
 
         CompletableFuture<?>[] futures = new CompletableFuture<?>[partitions];
+
+        // TODO: https://issues.apache.org/jira/browse/IGNITE-19713 Process assignments and set partitions only for assigned partitions.
+        PartitionSet parts = new BitSetPartitionSet();
+
         for (int i = 0; i < futures.length; i++) {
             futures[i] = new CompletableFuture<>();
+
+            parts.set(i);
         }
 
         String localMemberName = localNode().name();
 
         // Create new raft nodes according to new assignments.
-        Supplier<CompletableFuture<Void>> updateAssignmentsClosure = () -> {
+        Supplier<CompletableFuture<Void>>  updateAssignmentsClosure = () -> {

Review Comment:
   Good catch, good eye



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1230494264


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/PartitionSet.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.table.distributed;
+
+import java.util.stream.IntStream;
+
+/**
+ * Represents a collection of partition indices.

Review Comment:
   There is none



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1231794263


##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -465,11 +466,16 @@ private CompletableFuture<?> createIndexLocally(
         CompletableFuture<?> fireEventFuture =
                 fireEvent(IndexEvent.CREATE, new IndexEventParameters(causalityToken, tableId, indexId, eventIndexDescriptor));
 
-        CompletableFuture<TableImpl> tableFuture = tableManager.tableAsync(causalityToken, tableId);
+        TableImpl table = tableManager.getTable(tableId);
 
-        CompletableFuture<SchemaRegistry> schemaRegistryFuture = schemaManager.schemaRegistry(causalityToken, tableId);
+        assert table != null : tableId;
 
-        CompletableFuture<?> createIndexFuture = tableFuture.thenAcceptBoth(schemaRegistryFuture, (table, schemaRegistry) -> {
+        CompletableFuture<SchemaRegistry> schemaRegistryFut = schemaManager.schemaRegistry(causalityToken, tableId);

Review Comment:
   Let's rename it to **schemaRegistryFuture**.



##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -465,11 +466,16 @@ private CompletableFuture<?> createIndexLocally(
         CompletableFuture<?> fireEventFuture =
                 fireEvent(IndexEvent.CREATE, new IndexEventParameters(causalityToken, tableId, indexId, eventIndexDescriptor));
 
-        CompletableFuture<TableImpl> tableFuture = tableManager.tableAsync(causalityToken, tableId);
+        TableImpl table = tableManager.getTable(tableId);
 
-        CompletableFuture<SchemaRegistry> schemaRegistryFuture = schemaManager.schemaRegistry(causalityToken, tableId);
+        assert table != null : tableId;
 
-        CompletableFuture<?> createIndexFuture = tableFuture.thenAcceptBoth(schemaRegistryFuture, (table, schemaRegistry) -> {
+        CompletableFuture<SchemaRegistry> schemaRegistryFut = schemaManager.schemaRegistry(causalityToken, tableId);
+
+        // TODO: https://issues.apache.org/jira/browse/IGNITE-19712 Listen to assignment changes and start new index storages.
+        CompletableFuture<PartitionSet> tableStoragesFut = tableManager.localPartitionSetAsync(causalityToken, tableId);

Review Comment:
   Let's rename it to **tablePartitionFuture**.



##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -268,7 +268,6 @@ CompletableFuture<Void> finishRebalancePartition(
      * @return Index Storage.
      * @throws StorageException If the given partition does not exist.
      */
-    // TODO: IGNITE-19112 Change or get rid of

Review Comment:
   I will ask you to either return the **TODO**, or add it to the mention of this method in the ticket.



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2356,32 +2405,50 @@ private static PeersAndLearners configurationFromAssignments(Collection<Assignme
     }
 
     /**
-     * Creates partition stores. If one of the storages has not completed the rebalance, then the storages are cleared.
+     * Gets partition stores.
      *
      * @param table Table.
      * @param partitionId Partition ID.
-     * @return Future of creating or getting partition stores.
+     * @return PartitionStorages.
      */
-    // TODO: IGNITE-18939 Create storages only once, then only get them
-    private CompletableFuture<PartitionStorages> getOrCreatePartitionStorages(TableImpl table, int partitionId) {
+    private PartitionStorages getPartitionStorages(TableImpl table, int partitionId) {
         InternalTable internalTable = table.internalTable();
 
         MvPartitionStorage mvPartition = internalTable.storage().getMvPartition(partitionId);
 
-        return (mvPartition != null ? completedFuture(mvPartition) : internalTable.storage().createMvPartition(partitionId))
-                .thenComposeAsync(mvPartitionStorage -> {
-                    TxStateStorage txStateStorage = internalTable.txStateStorage().getOrCreateTxStateStorage(partitionId);
+        assert mvPartition != null;
 
-                    if (mvPartitionStorage.persistedIndex() == MvPartitionStorage.REBALANCE_IN_PROGRESS
-                            || txStateStorage.persistedIndex() == TxStateStorage.REBALANCE_IN_PROGRESS) {
-                        return allOf(
-                                internalTable.storage().clearPartition(partitionId),
-                                txStateStorage.clear()
-                        ).thenApply(unused -> new PartitionStorages(mvPartitionStorage, txStateStorage));
-                    } else {
-                        return completedFuture(new PartitionStorages(mvPartitionStorage, txStateStorage));
-                    }
-                }, ioExecutor);
+        TxStateStorage txStateStorage = internalTable.txStateStorage().getTxStateStorage(partitionId);
+
+        assert txStateStorage != null;
+
+        return new PartitionStorages(mvPartition, txStateStorage);
+    }
+
+    // TODO: https://issues.apache.org/jira/browse/IGNITE-19739 Create storages only once.

Review Comment:
   This ticket talks about indexes, but it should be about partitions, right?



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1230492955


##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -268,7 +268,6 @@ CompletableFuture<Void> finishRebalancePartition(
      * @return Index Storage.
      * @throws StorageException If the given partition does not exist.
      */
-    // TODO: IGNITE-19112 Change or get rid of

Review Comment:
   Because we need this method, it shouldn't be removed. Also, in the ticket there is no mention of the `getIndex` method



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1230532652


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/PartitionSet.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.table.distributed;
+
+import java.util.stream.IntStream;
+
+/**
+ * Represents a collection of partition indices.
+ */
+public interface PartitionSet {

Review Comment:
   We will need a separate implementation for sparse partition sets. For example if we have assignment
   
   [1, 10000, 100000000]. This will be several kbytes with bitset



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2177: IGNITE-19363 Split start of indexes and start of partition raft group nodes

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2177:
URL: https://github.com/apache/ignite-3/pull/2177#discussion_r1231116586


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/BitSetPartitionSet.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.table.distributed;
+
+import java.util.BitSet;
+import java.util.stream.IntStream;
+
+/**
+ * {@link BitSet} implementation of the {@link PartitionSet}.
+ */
+public class BitSetPartitionSet implements PartitionSet {

Review Comment:
   Done



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org