IGNITE-17011 [Native Persistence 3.0] Porting FilePageStoreManager from 2.0 #815
Conversation
|
@ibessonov Please make code review. |
|
@ibessonov Please make code review. |
modules/core/src/main/java/org/apache/ignite/internal/thread/IgniteThread.java
Outdated
Show resolved
Hide resolved
modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
Show resolved
Hide resolved
| /** | ||
| * Returns file page store path. | ||
| */ | ||
| Path filePath() { |
There was a problem hiding this comment.
This is needed for current tests, then it will be reworked when there is a new checkpoint algorithm.
| /** Logger. */ | ||
| private final IgniteLogger log; | ||
|
|
||
| /** Starting directory for all file page stores, for example: 'db/group-123/index.bin'. */ |
There was a problem hiding this comment.
I'm not sure that directory format is agreed upon right now, I'd avoid mentioning it
There was a problem hiding this comment.
I'll leave it as it is for now, we'll change it later and don't forget to take into account that the directories for the engines should be separate.
.../main/java/org/apache/ignite/internal/pagememory/persistence/store/FilePageStoreManager.java
Outdated
Show resolved
Hide resolved
| ) throws IgniteInternalCheckedException { | ||
| this.log = log; | ||
| this.filePageStoreFileIoFactory = filePageStoreFileIoFactory; | ||
| this.dbDir = storagePath.resolve("db"); |
There was a problem hiding this comment.
We need a common folders structure between all engines. Code like resolve("db") shouldn't be here, what do you think?
There was a problem hiding this comment.
I'll leave it as it is for now, we'll change it later and don't forget to take into account that the directories for the engines should be separate.
.../main/java/org/apache/ignite/internal/pagememory/persistence/store/FilePageStoreManager.java
Outdated
Show resolved
Hide resolved
| FilePageStore[] partitionFilePageStores = new FilePageStore[partitions]; | ||
|
|
||
| for (int i = 0; i < partitions; i++) { | ||
| partitionFilePageStores[i] = filePageStoreFactory.createPageStore( |
There was a problem hiding this comment.
Is this the original code? I don't see the presence of affinity here, is that intended?
There was a problem hiding this comment.
At this step, we create FilePageStore for all partitions, but for partition affinity, we will call FilePageStore#ensure (create a file physically) when creating PartitionStorage.
.../main/java/org/apache/ignite/internal/pagememory/persistence/store/GroupPageStoreHolder.java
Outdated
Show resolved
Hide resolved
|
|
||
| workers.add(worker); | ||
|
|
||
| new IgniteThread(worker).start(); |
There was a problem hiding this comment.
Ok, creating a new thread for every operation - what is this? We should lake a closer look at it afterwards.
There was a problem hiding this comment.
At the moment, the thread is created at the node stop, but will need to be reviewed later.
.../org/apache/ignite/internal/pagememory/persistence/store/LongOperationAsyncExecutorTest.java
Show resolved
Hide resolved
| @@ -223,6 +224,10 @@ void testAwaitAsyncTaskCompletion() throws Exception { | |||
| () -> finishTask1Future.get(100, TimeUnit.MILLISECONDS) | |||
| ); | |||
|
|
|||
| // Checks that the exception will be from task1. | |||
| assertThat(exception.getCause(), instanceOf(IgniteInternalException.class)); | |||
There was a problem hiding this comment.
This is wonderful.
https://issues.apache.org/jira/browse/IGNITE-17011