IGNITE-18565 Modify getOrCreateMvPartition and getMvPartition of MvTableStorage to return the future#1666
Conversation
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/engine/MvTableStorage.java
Outdated
Show resolved
Hide resolved
| class DestroyStorageOperation implements StorageOperation { | ||
| private final CompletableFuture<Void> destroyFuture = new CompletableFuture<>(); | ||
|
|
||
| private final AtomicReference<CreateStorageOperation> createStorageOperationReference = new AtomicReference<>(); |
There was a problem hiding this comment.
Looks like this can be just AtomicBoolean
There was a problem hiding this comment.
Not suitable, I need to atomically replace it with the creation operation when the destruction operation is completed if necessary.
| return new DestroyStorageOperation(); | ||
| }); | ||
|
|
||
| return completedFuture(null) |
There was a problem hiding this comment.
Why we need a completedFuture here? Can't we just use destroyStorageFunction.apply(storageByPartitionId.getAndSet(partitionId, null))
There was a problem hiding this comment.
This is a trick to make sure the operation gets rid of if it succeeds or fails.
...s/storage-api/src/main/java/org/apache/ignite/internal/storage/util/MvPartitionStorages.java
Outdated
Show resolved
Hide resolved
...s/storage-api/src/main/java/org/apache/ignite/internal/storage/util/MvPartitionStorages.java
Outdated
Show resolved
Hide resolved
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/util/StorageUtils.java
Outdated
Show resolved
Hide resolved
...les/storage-api/src/test/java/org/apache/ignite/internal/storage/TestMvTableStorageTest.java
Show resolved
Hide resolved
|
|
||
| assertThat(createMvPartitionFuture, willCompleteSuccessfully()); | ||
|
|
||
| return createMvPartitionFuture.join(); |
There was a problem hiding this comment.
Probably should be get with timeout
There was a problem hiding this comment.
It is not necessary because above we make sure that the future completed.
assertThat(createMvPartitionFuture, willCompleteSuccessfully());
There was a problem hiding this comment.
how about getNow() then?
There was a problem hiding this comment.
I do not think that this is necessary, the future is already definitely completed.
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
Show resolved
Hide resolved
...s/storage-api/src/main/java/org/apache/ignite/internal/storage/util/MvPartitionStorages.java
Show resolved
Hide resolved
|
|
||
| /** Partition storages. */ | ||
| private volatile AtomicReferenceArray<RocksDbMvPartitionStorage> partitions; | ||
| private volatile MvPartitionStorages<RocksDbMvPartitionStorage> mvPartitionStorages; |
There was a problem hiding this comment.
Can we create this object in the constructor?
There was a problem hiding this comment.
From the point of view of the product code, the answer is "yes we can", but from the point of view of the designs should do in org.apache.ignite.internal.storage.engine.MvTableStorage#start.
...orage-api/src/test/java/org/apache/ignite/internal/storage/util/MvPartitionStoragesTest.java
Show resolved
Hide resolved
...orage-api/src/test/java/org/apache/ignite/internal/storage/util/MvPartitionStoragesTest.java
Show resolved
Hide resolved
...orage-api/src/test/java/org/apache/ignite/internal/storage/util/MvPartitionStoragesTest.java
Show resolved
Hide resolved
…bleStorage to return the future (apache#1666)
…bleStorage to return the future (apache#1666)
2 participants
https://issues.apache.org/jira/browse/IGNITE-18565