sashapolo commented on code in PR #1286:
URL: https://github.com/apache/ignite-3/pull/1286#discussion_r1011290278
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -162,4 +163,48 @@ default IndexStorage getOrCreateIndex(int partitionId,
UUID indexId) {
* @throws StorageException If an error has occurred during the
destruction of the storage.
*/
void destroy() throws StorageException;
+
+ /**
+ * Prepares the partition storage for rebalancing: makes a backup of the
current partition storage and creates a new one.
Review Comment:
Again, please replace `new one` with `a new storage`
##########
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/AbstractMvTableStorageTest.java:
##########
@@ -299,6 +312,92 @@ public void testMisconfiguredIndices() {
);
}
+ @Test
+ protected void testStartRebalanceMvPartition() throws Exception {
+ ExecutorService executorService = Executors.newSingleThreadExecutor();
+
+ try {
+ MvPartitionStorage partitionStorage =
tableStorage.getOrCreateMvPartition(PARTITION_ID);
+
+ partitionStorage.runConsistently(() -> {
+ partitionStorage.addWriteCommitted(
+ new RowId(PARTITION_ID),
+ binaryRow(new TestKey(0, "0"), new TestValue(1, "1")),
+ clock.now()
+ );
+
+ partitionStorage.lastAppliedIndex(100);
+
+ return null;
+ });
+
+ partitionStorage.flush().get(1, TimeUnit.SECONDS);
+
+ tableStorage.startRebalanceMvPartition(PARTITION_ID,
executorService).get(1, TimeUnit.SECONDS);
+
+ MvPartitionStorage newPartitionStorage0 =
tableStorage.getMvPartition(PARTITION_ID);
+
+ assertNotNull(newPartitionStorage0);
+ assertNotSame(partitionStorage, newPartitionStorage0);
+
+ assertEquals(0L, newPartitionStorage0.lastAppliedIndex());
+ assertEquals(0L, newPartitionStorage0.persistedIndex());
+ assertEquals(0, newPartitionStorage0.rowsCount());
+
+ tableStorage.startRebalanceMvPartition(PARTITION_ID,
executorService).get(1, TimeUnit.SECONDS);
+
+ MvPartitionStorage newPartitionStorage1 =
tableStorage.getMvPartition(PARTITION_ID);
+
+ assertSame(newPartitionStorage0, newPartitionStorage1);
+ } finally {
+ IgniteUtils.shutdownAndAwaitTermination(executorService, 1,
TimeUnit.SECONDS);
+ }
+ }
+
+ @Test
+ protected void testAbortRebalanceMvPartition() throws Exception {
Review Comment:
You only changed one modifier to `public`, others are still `protected`,
that's strange
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -162,4 +163,48 @@ default IndexStorage getOrCreateIndex(int partitionId,
UUID indexId) {
* @throws StorageException If an error has occurred during the
destruction of the storage.
*/
void destroy() throws StorageException;
+
+ /**
+ * Prepares the partition storage for rebalancing: makes a backup of the
current partition storage and creates a new one.
+ *
+ * <p>This method must be called before each full rebalance of the
partition storage, so that in case of errors or cancellation of the
Review Comment:
```suggestion
* <p>This method must be called before every full rebalance of the
partition storage, so that in case of errors or cancellation of the
```
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -162,4 +163,48 @@ default IndexStorage getOrCreateIndex(int partitionId,
UUID indexId) {
* @throws StorageException If an error has occurred during the
destruction of the storage.
*/
void destroy() throws StorageException;
+
+ /**
+ * Prepares the partition storage for rebalancing: makes a backup of the
current partition storage and creates a new one.
+ *
+ * <p>This method must be called before each full rebalance of the
partition storage, so that in case of errors or cancellation of the
+ * full rebalance, we can restore the partition storage from the backup.
+ *
+ * <p>Full rebalance will be completed when one of the methods is called:
+ * <ol>
+ * <li>{@link #abortRebalanceMvPartition(int, Executor)} - in case of
a full rebalance cancellation or failure, so that we can
+ * restore the partition storage from a backup;</li>
+ * <li>{@link #finishRebalanceMvPartition(int, Executor)} - in case of
a successful full rebalance, to remove the backup of the
+ * partition storage.</li>
+ * </ol>
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
+ * @return Future, if completed without errors, then {@link
#getMvPartition} will return a new (empty) partition storage.
+ */
+ CompletableFuture<Void> startRebalanceMvPartition(int partitionId,
Executor executor);
+
+ /**
+ * Aborts rebalancing of the partition storage if it was started: restores
the partition storage from a backup and deletes the new one.
+ *
+ * <p>If a full rebalance has not {@link #startRebalanceMvPartition(int,
Executor) started}, then nothing will happen.
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
+ * @return Future, upon completion of which (even if with an error) {@link
#getMvPartition} will return the partition storage restored
+ * from the backup.
+ */
+ CompletableFuture<Void> abortRebalanceMvPartition(int partitionId,
Executor executor);
+
+ /**
+ * Finishes a successful partition storage rebalance if it has been
started: deletes the backup of the partition storage and saves a new
Review Comment:
Same about `new one`
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -162,4 +163,32 @@ default IndexStorage getOrCreateIndex(int partitionId,
UUID indexId) {
* @throws StorageException If an error has occurred during the
destruction of the storage.
*/
void destroy() throws StorageException;
+
+ /**
+ * Prepares the partition storage for rebalancing: makes a backup and
creates a new one.
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
Review Comment:
That's better, I agree
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -162,4 +163,48 @@ default IndexStorage getOrCreateIndex(int partitionId,
UUID indexId) {
* @throws StorageException If an error has occurred during the
destruction of the storage.
*/
void destroy() throws StorageException;
+
+ /**
+ * Prepares the partition storage for rebalancing: makes a backup of the
current partition storage and creates a new one.
+ *
+ * <p>This method must be called before each full rebalance of the
partition storage, so that in case of errors or cancellation of the
+ * full rebalance, we can restore the partition storage from the backup.
+ *
+ * <p>Full rebalance will be completed when one of the methods is called:
+ * <ol>
+ * <li>{@link #abortRebalanceMvPartition(int, Executor)} - in case of
a full rebalance cancellation or failure, so that we can
+ * restore the partition storage from a backup;</li>
+ * <li>{@link #finishRebalanceMvPartition(int, Executor)} - in case of
a successful full rebalance, to remove the backup of the
+ * partition storage.</li>
+ * </ol>
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
+ * @return Future, if completed without errors, then {@link
#getMvPartition} will return a new (empty) partition storage.
+ */
+ CompletableFuture<Void> startRebalanceMvPartition(int partitionId,
Executor executor);
+
+ /**
+ * Aborts rebalancing of the partition storage if it was started: restores
the partition storage from a backup and deletes the new one.
Review Comment:
Same about the `new one`
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -162,4 +163,48 @@ default IndexStorage getOrCreateIndex(int partitionId,
UUID indexId) {
* @throws StorageException If an error has occurred during the
destruction of the storage.
*/
void destroy() throws StorageException;
+
+ /**
+ * Prepares the partition storage for rebalancing: makes a backup of the
current partition storage and creates a new one.
+ *
+ * <p>This method must be called before each full rebalance of the
partition storage, so that in case of errors or cancellation of the
+ * full rebalance, we can restore the partition storage from the backup.
+ *
+ * <p>Full rebalance will be completed when one of the methods is called:
+ * <ol>
+ * <li>{@link #abortRebalanceMvPartition(int, Executor)} - in case of
a full rebalance cancellation or failure, so that we can
+ * restore the partition storage from a backup;</li>
+ * <li>{@link #finishRebalanceMvPartition(int, Executor)} - in case of
a successful full rebalance, to remove the backup of the
+ * partition storage.</li>
+ * </ol>
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
+ * @return Future, if completed without errors, then {@link
#getMvPartition} will return a new (empty) partition storage.
+ */
+ CompletableFuture<Void> startRebalanceMvPartition(int partitionId,
Executor executor);
+
+ /**
+ * Aborts rebalancing of the partition storage if it was started: restores
the partition storage from a backup and deletes the new one.
+ *
+ * <p>If a full rebalance has not {@link #startRebalanceMvPartition(int,
Executor) started}, then nothing will happen.
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
+ * @return Future, upon completion of which (even if with an error) {@link
#getMvPartition} will return the partition storage restored
+ * from the backup.
+ */
+ CompletableFuture<Void> abortRebalanceMvPartition(int partitionId,
Executor executor);
+
+ /**
+ * Finishes a successful partition storage rebalance if it has been
started: deletes the backup of the partition storage and saves a new
+ * one.
+ *
+ * <p>If a full rebalance has not {@link #startRebalanceMvPartition(int,
Executor) started}, then nothing will happen.
Review Comment:
```suggestion
* <p>If a full rebalance has not been {@link
#startRebalanceMvPartition(int, Executor) started}, then nothing will happen.
```
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -162,4 +163,48 @@ default IndexStorage getOrCreateIndex(int partitionId,
UUID indexId) {
* @throws StorageException If an error has occurred during the
destruction of the storage.
*/
void destroy() throws StorageException;
+
+ /**
+ * Prepares the partition storage for rebalancing: makes a backup of the
current partition storage and creates a new one.
+ *
+ * <p>This method must be called before each full rebalance of the
partition storage, so that in case of errors or cancellation of the
+ * full rebalance, we can restore the partition storage from the backup.
+ *
+ * <p>Full rebalance will be completed when one of the methods is called:
+ * <ol>
+ * <li>{@link #abortRebalanceMvPartition(int, Executor)} - in case of
a full rebalance cancellation or failure, so that we can
+ * restore the partition storage from a backup;</li>
+ * <li>{@link #finishRebalanceMvPartition(int, Executor)} - in case of
a successful full rebalance, to remove the backup of the
+ * partition storage.</li>
+ * </ol>
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
+ * @return Future, if completed without errors, then {@link
#getMvPartition} will return a new (empty) partition storage.
+ */
+ CompletableFuture<Void> startRebalanceMvPartition(int partitionId,
Executor executor);
+
+ /**
+ * Aborts rebalancing of the partition storage if it was started: restores
the partition storage from a backup and deletes the new one.
+ *
+ * <p>If a full rebalance has not {@link #startRebalanceMvPartition(int,
Executor) started}, then nothing will happen.
Review Comment:
```suggestion
* <p>If a full rebalance has not been {@link
#startRebalanceMvPartition(int, Executor) started}, then nothing will happen.
```
##########
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/impl/TestMvTableStorage.java:
##########
@@ -101,12 +106,10 @@ public MvPartitionStorage getMvPartition(int partitionId)
{
@Override
public void destroyPartition(int partitionId) throws StorageException {
- Integer boxedPartitionId = partitionId;
Review Comment:
I would say it was a small (maybe useless optimization), but nevertheless,
this change is not related to the PR and does not improve the code much
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java:
##########
@@ -162,4 +163,48 @@ default IndexStorage getOrCreateIndex(int partitionId,
UUID indexId) {
* @throws StorageException If an error has occurred during the
destruction of the storage.
*/
void destroy() throws StorageException;
+
+ /**
+ * Prepares the partition storage for rebalancing: makes a backup of the
current partition storage and creates a new one.
+ *
+ * <p>This method must be called before each full rebalance of the
partition storage, so that in case of errors or cancellation of the
+ * full rebalance, we can restore the partition storage from the backup.
+ *
+ * <p>Full rebalance will be completed when one of the methods is called:
+ * <ol>
+ * <li>{@link #abortRebalanceMvPartition(int, Executor)} - in case of
a full rebalance cancellation or failure, so that we can
+ * restore the partition storage from a backup;</li>
+ * <li>{@link #finishRebalanceMvPartition(int, Executor)} - in case of
a successful full rebalance, to remove the backup of the
+ * partition storage.</li>
+ * </ol>
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
+ * @return Future, if completed without errors, then {@link
#getMvPartition} will return a new (empty) partition storage.
+ */
+ CompletableFuture<Void> startRebalanceMvPartition(int partitionId,
Executor executor);
+
+ /**
+ * Aborts rebalancing of the partition storage if it was started: restores
the partition storage from a backup and deletes the new one.
+ *
+ * <p>If a full rebalance has not {@link #startRebalanceMvPartition(int,
Executor) started}, then nothing will happen.
+ *
+ * @param partitionId Partition ID.
+ * @param executor Executor.
+ * @return Future, upon completion of which (even if with an error) {@link
#getMvPartition} will return the partition storage restored
Review Comment:
hm, what will an error mean then?
--
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]
