You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/08/03 14:48:52 UTC
[GitHub] [ignite-3] tkalkirill opened a new pull request, #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
tkalkirill opened a new pull request, #976:
URL: https://github.com/apache/ignite-3/pull/976
https://issues.apache.org/jira/browse/IGNITE-17085
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938631170
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -226,21 +336,92 @@ TableFreeList createTableFreeList(
}
}
+ /**
+ * Returns new {@link VersionChainFreeList} instance for partition.
+ *
+ * @param tableView Table configuration.
+ * @param partId Partition ID.
+ * @param rootPageId Root page ID.
+ * @param initNew {@code True} if new metadata should be initialized.
+ * @throws StorageException If failed.
+ */
+ VersionChainFreeList createVersionChainFreeList(
Review Comment:
It seems to me that the factory is more suitable when we do not know the exact type of the object, but here we do.
Made them private
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938619507
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/RowVersionFreeList.java:
##########
@@ -83,7 +80,7 @@ public RowVersionFreeList(
LOG,
metaPageId,
initNew,
- pageListCacheLimit,
+ null,
Review Comment:
Fix it
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938649466
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -164,8 +170,113 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
- public PageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
- throw new UnsupportedOperationException("Not supported yet");
+ public PersistentPageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
+ TableView tableView = tableCfg.value();
+
+ FilePageStore filePageStore = ensurePartitionFilePageStore(tableView, partitionId);
+
+ CheckpointManager checkpointManager = dataRegion.checkpointManager();
+
+ CheckpointTimeoutLock checkpointTimeoutLock = checkpointManager.checkpointTimeoutLock();
Review Comment:
I don't know because we're trying to take the lock by timeout.
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938573638
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
What's more confusing here?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
- private static VersionChainFreeList createVersionChainFreeList(
- PageMemory pageMemory,
- ReuseList reuseList
- ) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
+ private static VersionChainFreeList createVersionChainFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
return new VersionChainFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
- reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
What's more confusing here?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
- private static VersionChainFreeList createVersionChainFreeList(
- PageMemory pageMemory,
- ReuseList reuseList
- ) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
+ private static VersionChainFreeList createVersionChainFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
return new VersionChainFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
- reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
private static RowVersionFreeList createRowVersionFreeList(
PageMemory pageMemory,
- ReuseList reuseList
+ VersionChainFreeList reuseList
) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(
- FREE_LIST_GROUP_ID,
- FREE_LIST_PARTITION_ID,
- FLAG_AUX
- );
-
return new RowVersionFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
What's more confusing 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] tkalkirill commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938626718
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/PartitionMetaTest.java:
##########
@@ -80,32 +80,89 @@ void testPageCount() {
assertEquals(2, meta.pageCount());
}
+ @Test
+ void testVersionChainTreeRootPageId() {
+ PartitionMeta meta = new PartitionMeta(null, 0, 0, 0, 0, 0, 0);
+
+ assertEquals(0, meta.versionChainTreeRootPageId());
+
+ assertDoesNotThrow(() -> meta.versionChainTreeRootPageId(null, 100));
Review Comment:
I'll leave.
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938627079
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -164,8 +170,113 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
- public PageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
- throw new UnsupportedOperationException("Not supported yet");
+ public PersistentPageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
+ TableView tableView = tableCfg.value();
+
+ FilePageStore filePageStore = ensurePartitionFilePageStore(tableView, partitionId);
+
+ CheckpointManager checkpointManager = dataRegion.checkpointManager();
+
+ CheckpointTimeoutLock checkpointTimeoutLock = checkpointManager.checkpointTimeoutLock();
Review Comment:
Maybe.
--
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] sashapolo commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938645504
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -226,21 +336,92 @@ TableFreeList createTableFreeList(
}
}
+ /**
+ * Returns new {@link VersionChainFreeList} instance for partition.
+ *
+ * @param tableView Table configuration.
+ * @param partId Partition ID.
+ * @param rootPageId Root page ID.
+ * @param initNew {@code True} if new metadata should be initialized.
+ * @throws StorageException If failed.
+ */
+ VersionChainFreeList createVersionChainFreeList(
Review Comment:
I suggested a factory here, because it will allow to extract some code from this class. We have a tendency to have very bloated classes.
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -226,21 +336,92 @@ TableFreeList createTableFreeList(
}
}
+ /**
+ * Returns new {@link VersionChainFreeList} instance for partition.
+ *
+ * @param tableView Table configuration.
+ * @param partId Partition ID.
+ * @param rootPageId Root page ID.
+ * @param initNew {@code True} if new metadata should be initialized.
+ * @throws StorageException If failed.
+ */
+ VersionChainFreeList createVersionChainFreeList(
Review Comment:
I suggested a factory here, because it will allow to extract some code from this class. We have a tendency to having very bloated classes.
--
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] sashapolo commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938644850
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -164,8 +170,113 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
- public PageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
- throw new UnsupportedOperationException("Not supported yet");
+ public PersistentPageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
+ TableView tableView = tableCfg.value();
+
+ FilePageStore filePageStore = ensurePartitionFilePageStore(tableView, partitionId);
+
+ CheckpointManager checkpointManager = dataRegion.checkpointManager();
+
+ CheckpointTimeoutLock checkpointTimeoutLock = checkpointManager.checkpointTimeoutLock();
Review Comment:
Can you create a ticket for fixing this?
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938794170
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/PartitionMetaTest.java:
##########
@@ -37,7 +37,7 @@
public class PartitionMetaTest {
@Test
void testTreeRootPageId() {
- PartitionMeta meta = new PartitionMeta(null, 0, 0, 0);
+ PartitionMeta meta = new PartitionMeta(null, 0, 0, 0, 0, 0, 0);
Review Comment:
fix it
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938572880
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -164,8 +170,113 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
- public PageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
- throw new UnsupportedOperationException("Not supported yet");
+ public PersistentPageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
+ TableView tableView = tableCfg.value();
+
+ FilePageStore filePageStore = ensurePartitionFilePageStore(tableView, partitionId);
+
+ CheckpointManager checkpointManager = dataRegion.checkpointManager();
+
+ CheckpointTimeoutLock checkpointTimeoutLock = checkpointManager.checkpointTimeoutLock();
+
+ checkpointTimeoutLock.checkpointReadLock();
+
+ try {
+ PersistentPageMemory persistentPageMemory = dataRegion.pageMemory();
+
+ int grpId = tableView.tableId();
+
+ CheckpointProgress lastCheckpointProgress = checkpointManager.lastCheckpointProgress();
+
+ UUID checkpointId = lastCheckpointProgress == null ? null : lastCheckpointProgress.id();
+
+ PartitionMeta meta = dataRegion.partitionMetaManager().readOrCreateMeta(
+ checkpointId,
+ new GroupPartitionId(grpId, partitionId),
+ filePageStore
+ );
+
+ dataRegion.partitionMetaManager().addMeta(new GroupPartitionId(grpId, partitionId), meta);
+
+ filePageStore.pages(meta.pageCount());
+
+ filePageStore.setPageAllocationListener(pageIdx -> {
+ assert checkpointTimeoutLock.checkpointLockIsHeldByThread();
+
+ CheckpointProgress last = checkpointManager.lastCheckpointProgress();
+
+ meta.incrementPageCount(last == null ? null : last.id());
+ });
+
+ boolean initNewVersionChainTree = false;
+
+ if (meta.versionChainTreeRootPageId() == 0) {
+ meta.versionChainTreeRootPageId(checkpointId, persistentPageMemory.allocatePage(grpId, partitionId, FLAG_AUX));
+
+ initNewVersionChainTree = true;
+ }
+
+ boolean initVersionChainFreeList = false;
+
+ if (meta.versionChainFreeListRootPageId() == 0) {
+ meta.versionChainFreeListRootPageId(checkpointId, persistentPageMemory.allocatePage(grpId, partitionId, FLAG_AUX));
+
+ initVersionChainFreeList = true;
+ }
+
+ boolean initRowVersionFreeList = false;
+
+ if (meta.rowVersionFreeListRootPageId() == 0) {
+ meta.rowVersionFreeListRootPageId(checkpointId, persistentPageMemory.allocatePage(grpId, partitionId, FLAG_AUX));
+
+ initRowVersionFreeList = true;
+ }
+
+ VersionChainFreeList versionChainFreeList = createVersionChainFreeList(
+ tableView,
+ partitionId,
+ meta.versionChainFreeListRootPageId(),
+ initVersionChainFreeList
+ );
+
+ autoCloseables.add(versionChainFreeList::close);
+
+ RowVersionFreeList rowVersionFreeList = createRowVersionFreeList(
+ tableView,
+ partitionId,
+ versionChainFreeList,
+ meta.rowVersionFreeListRootPageId(),
+ initRowVersionFreeList
+ );
+
+ autoCloseables.add(rowVersionFreeList::close);
+
+ VersionChainTree versionChainTree = createVersionChainTree(
+ tableView,
+ partitionId,
+ versionChainFreeList,
+ meta.versionChainTreeRootPageId(),
+ initNewVersionChainTree
+ );
+
+ return new PersistentPageMemoryMvPartitionStorage(
+ partitionId,
+ tableView,
+ dataRegion.pageMemory(),
+ versionChainFreeList,
+ rowVersionFreeList,
+ versionChainTree,
+ checkpointTimeoutLock
+ );
+ } catch (IgniteInternalCheckedException e) {
+ throw new StorageException(
+ String.format("Error getting or creating partition metadata [tableName=%s, partitionId=%s]", tableView.name(),
Review Comment:
fix it
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938570681
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryPartitionStorage.java:
##########
@@ -0,0 +1,487 @@
+/*
+ * 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.storage.pagememory;
+
+import static org.apache.ignite.internal.pagememory.PageIdAllocator.MAX_PARTITION_ID;
+
+import java.nio.ByteBuffer;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Predicate;
+import org.apache.ignite.internal.pagememory.tree.BplusTree;
+import org.apache.ignite.internal.pagememory.tree.IgniteTree;
+import org.apache.ignite.internal.storage.DataRow;
+import org.apache.ignite.internal.storage.InvokeClosure;
+import org.apache.ignite.internal.storage.OperationType;
+import org.apache.ignite.internal.storage.PartitionStorage;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.StorageUtils;
+import org.apache.ignite.internal.util.Cursor;
+import org.apache.ignite.internal.util.IgniteCursor;
+import org.apache.ignite.lang.IgniteInternalCheckedException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Abstract implementation of {@link PartitionStorage} based on a {@link BplusTree}.
+ */
+// TODO: IGNITE-16644 Support snapshots.
+abstract class AbstractPageMemoryPartitionStorage implements PartitionStorage {
Review Comment:
We will get rid of this in a separate IGNITE-17466, but for now, let it be 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] sashapolo commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938645923
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/AbstractPageMemoryMvPartitionStorageTest.java:
##########
@@ -259,9 +210,9 @@ void scanByTimestampWorksCorrectlyAfterCommitAndAbortFollowedByUncommittedWrite(
@Test
void readByTimestampWorksCorrectlyIfNoUncommittedValueExists() {
- LinkRowId rowId = storage.insert(binaryRow, txId);
+ LinkRowId rowId = (LinkRowId) insert(binaryRow, txId);
Review Comment:
And why do you need a `LinkRowId`?
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938632101
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/AbstractPageMemoryMvPartitionStorageTest.java:
##########
@@ -21,114 +21,55 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
-import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.nio.file.Path;
-import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;
-import org.apache.ignite.configuration.schemas.store.UnknownDataStorageConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.ConstantValueDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.FunctionCallDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.NullValueDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.TableConfiguration;
import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
-import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
-import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
import org.apache.ignite.internal.schema.BinaryRow;
import org.apache.ignite.internal.storage.AbstractMvPartitionStorageTest;
import org.apache.ignite.internal.storage.RowId;
-import org.apache.ignite.internal.storage.pagememory.AbstractPageMemoryTableStorage;
-import org.apache.ignite.internal.storage.pagememory.VolatilePageMemoryStorageEngine;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageChange;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageConfigurationSchema;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageView;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryStorageEngineConfiguration;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryStorageEngineConfigurationSchema;
import org.apache.ignite.internal.testframework.WorkDirectory;
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
import org.apache.ignite.internal.tx.Timestamp;
import org.apache.ignite.internal.util.Cursor;
-import org.apache.ignite.internal.util.IgniteUtils;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+/**
+ * Base test for MV partition storages based on PageMemory.
+ */
@ExtendWith(ConfigurationExtension.class)
@ExtendWith(WorkDirectoryExtension.class)
-class PageMemoryMvPartitionStorageTest extends AbstractMvPartitionStorageTest<PageMemoryMvPartitionStorage> {
- private final PageIoRegistry ioRegistry = new PageIoRegistry();
+abstract class AbstractPageMemoryMvPartitionStorageTest<T extends AbstractPageMemoryMvPartitionStorage> extends
+ AbstractMvPartitionStorageTest<T> {
+ protected final PageIoRegistry ioRegistry = new PageIoRegistry();
- private final BinaryRow binaryRow3 = binaryRow(key, new TestValue(22, "bar3"));
+ protected final BinaryRow binaryRow3 = binaryRow(key, new TestValue(22, "bar3"));
Review Comment:
fix it
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938612038
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
Fix it
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
- private static VersionChainFreeList createVersionChainFreeList(
- PageMemory pageMemory,
- ReuseList reuseList
- ) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
+ private static VersionChainFreeList createVersionChainFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
return new VersionChainFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
- reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
Fix it
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938633049
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/AbstractPageMemoryMvPartitionStorageTest.java:
##########
@@ -259,9 +210,9 @@ void scanByTimestampWorksCorrectlyAfterCommitAndAbortFollowedByUncommittedWrite(
@Test
void readByTimestampWorksCorrectlyIfNoUncommittedValueExists() {
- LinkRowId rowId = storage.insert(binaryRow, txId);
+ LinkRowId rowId = (LinkRowId) insert(binaryRow, txId);
Review Comment:
Because the method returns `RowId`
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938652461
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/AbstractPageMemoryMvPartitionStorageTest.java:
##########
@@ -259,9 +210,9 @@ void scanByTimestampWorksCorrectlyAfterCommitAndAbortFollowedByUncommittedWrite(
@Test
void readByTimestampWorksCorrectlyIfNoUncommittedValueExists() {
- LinkRowId rowId = storage.insert(binaryRow, txId);
+ LinkRowId rowId = (LinkRowId) insert(binaryRow, txId);
Review Comment:
Fix it
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938650473
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -226,21 +336,92 @@ TableFreeList createTableFreeList(
}
}
+ /**
+ * Returns new {@link VersionChainFreeList} instance for partition.
+ *
+ * @param tableView Table configuration.
+ * @param partId Partition ID.
+ * @param rootPageId Root page ID.
+ * @param initNew {@code True} if new metadata should be initialized.
+ * @throws StorageException If failed.
+ */
+ VersionChainFreeList createVersionChainFreeList(
Review Comment:
I propose to leave it as it is, the factory will slightly reduce the code, but it can complicate it.
--
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] sashapolo commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938768957
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -226,21 +336,92 @@ TableFreeList createTableFreeList(
}
}
+ /**
+ * Returns new {@link VersionChainFreeList} instance for partition.
+ *
+ * @param tableView Table configuration.
+ * @param partId Partition ID.
+ * @param rootPageId Root page ID.
+ * @param initNew {@code True} if new metadata should be initialized.
+ * @throws StorageException If failed.
+ */
+ VersionChainFreeList createVersionChainFreeList(
Review Comment:
I disagree, but ok
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938635282
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/AbstractPageMemoryMvPartitionStorageTest.java:
##########
@@ -21,114 +21,55 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
-import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.nio.file.Path;
-import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;
-import org.apache.ignite.configuration.schemas.store.UnknownDataStorageConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.ConstantValueDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.FunctionCallDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.NullValueDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.TableConfiguration;
import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
-import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
-import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
import org.apache.ignite.internal.schema.BinaryRow;
import org.apache.ignite.internal.storage.AbstractMvPartitionStorageTest;
import org.apache.ignite.internal.storage.RowId;
-import org.apache.ignite.internal.storage.pagememory.AbstractPageMemoryTableStorage;
-import org.apache.ignite.internal.storage.pagememory.VolatilePageMemoryStorageEngine;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageChange;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageConfigurationSchema;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageView;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryStorageEngineConfiguration;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryStorageEngineConfigurationSchema;
import org.apache.ignite.internal.testframework.WorkDirectory;
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
import org.apache.ignite.internal.tx.Timestamp;
import org.apache.ignite.internal.util.Cursor;
-import org.apache.ignite.internal.util.IgniteUtils;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+/**
+ * Base test for MV partition storages based on PageMemory.
+ */
@ExtendWith(ConfigurationExtension.class)
@ExtendWith(WorkDirectoryExtension.class)
-class PageMemoryMvPartitionStorageTest extends AbstractMvPartitionStorageTest<PageMemoryMvPartitionStorage> {
- private final PageIoRegistry ioRegistry = new PageIoRegistry();
+abstract class AbstractPageMemoryMvPartitionStorageTest<T extends AbstractPageMemoryMvPartitionStorage> extends
Review Comment:
Fix it
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938628310
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/PartitionMetaTest.java:
##########
@@ -80,32 +80,89 @@ void testPageCount() {
assertEquals(2, meta.pageCount());
}
+ @Test
+ void testVersionChainTreeRootPageId() {
Review Comment:
Yes, but also to check metadata snapshots for those setters
--
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] ibessonov merged pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
ibessonov merged PR #976:
URL: https://github.com/apache/ignite-3/pull/976
--
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] sashapolo commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938592991
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/PartitionMetaTest.java:
##########
@@ -80,32 +80,89 @@ void testPageCount() {
assertEquals(2, meta.pageCount());
}
+ @Test
+ void testVersionChainTreeRootPageId() {
+ PartitionMeta meta = new PartitionMeta(null, 0, 0, 0, 0, 0, 0);
+
+ assertEquals(0, meta.versionChainTreeRootPageId());
+
+ assertDoesNotThrow(() -> meta.versionChainTreeRootPageId(null, 100));
Review Comment:
I think `assertDoesNotThrow` is pretty useless, but you can leave it if you want
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -164,8 +170,113 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
- public PageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
- throw new UnsupportedOperationException("Not supported yet");
+ public PersistentPageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
+ TableView tableView = tableCfg.value();
+
+ FilePageStore filePageStore = ensurePartitionFilePageStore(tableView, partitionId);
+
+ CheckpointManager checkpointManager = dataRegion.checkpointManager();
+
+ CheckpointTimeoutLock checkpointTimeoutLock = checkpointManager.checkpointTimeoutLock();
Review Comment:
Not related to this PR, but I think that `CheckpointTimeoutLock` is a very misleading name
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
Because it was more clear that this allocated page is a meta page
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/AbstractPageMemoryMvPartitionStorageTest.java:
##########
@@ -259,9 +210,9 @@ void scanByTimestampWorksCorrectlyAfterCommitAndAbortFollowedByUncommittedWrite(
@Test
void readByTimestampWorksCorrectlyIfNoUncommittedValueExists() {
- LinkRowId rowId = storage.insert(binaryRow, txId);
+ LinkRowId rowId = (LinkRowId) insert(binaryRow, txId);
Review Comment:
what is this cast for?
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/AbstractPageMemoryMvPartitionStorageTest.java:
##########
@@ -21,114 +21,55 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
-import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.nio.file.Path;
-import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;
-import org.apache.ignite.configuration.schemas.store.UnknownDataStorageConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.ConstantValueDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.FunctionCallDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.NullValueDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.TableConfiguration;
import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
-import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
-import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
import org.apache.ignite.internal.schema.BinaryRow;
import org.apache.ignite.internal.storage.AbstractMvPartitionStorageTest;
import org.apache.ignite.internal.storage.RowId;
-import org.apache.ignite.internal.storage.pagememory.AbstractPageMemoryTableStorage;
-import org.apache.ignite.internal.storage.pagememory.VolatilePageMemoryStorageEngine;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageChange;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageConfigurationSchema;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageView;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryStorageEngineConfiguration;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryStorageEngineConfigurationSchema;
import org.apache.ignite.internal.testframework.WorkDirectory;
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
import org.apache.ignite.internal.tx.Timestamp;
import org.apache.ignite.internal.util.Cursor;
-import org.apache.ignite.internal.util.IgniteUtils;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+/**
+ * Base test for MV partition storages based on PageMemory.
+ */
@ExtendWith(ConfigurationExtension.class)
@ExtendWith(WorkDirectoryExtension.class)
-class PageMemoryMvPartitionStorageTest extends AbstractMvPartitionStorageTest<PageMemoryMvPartitionStorage> {
- private final PageIoRegistry ioRegistry = new PageIoRegistry();
+abstract class AbstractPageMemoryMvPartitionStorageTest<T extends AbstractPageMemoryMvPartitionStorage> extends
+ AbstractMvPartitionStorageTest<T> {
+ protected final PageIoRegistry ioRegistry = new PageIoRegistry();
- private final BinaryRow binaryRow3 = binaryRow(key, new TestValue(22, "bar3"));
+ protected final BinaryRow binaryRow3 = binaryRow(key, new TestValue(22, "bar3"));
Review Comment:
can still be private
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -226,21 +336,92 @@ TableFreeList createTableFreeList(
}
}
+ /**
+ * Returns new {@link VersionChainFreeList} instance for partition.
+ *
+ * @param tableView Table configuration.
+ * @param partId Partition ID.
+ * @param rootPageId Root page ID.
+ * @param initNew {@code True} if new metadata should be initialized.
+ * @throws StorageException If failed.
+ */
+ VersionChainFreeList createVersionChainFreeList(
Review Comment:
also, all of them can be `private`
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/AbstractPageMemoryMvPartitionStorageTest.java:
##########
@@ -21,114 +21,55 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
-import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.nio.file.Path;
-import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;
-import org.apache.ignite.configuration.schemas.store.UnknownDataStorageConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.ConstantValueDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.FunctionCallDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.HashIndexConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.NullValueDefaultConfigurationSchema;
-import org.apache.ignite.configuration.schemas.table.TableConfiguration;
import org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
-import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
-import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
import org.apache.ignite.internal.schema.BinaryRow;
import org.apache.ignite.internal.storage.AbstractMvPartitionStorageTest;
import org.apache.ignite.internal.storage.RowId;
-import org.apache.ignite.internal.storage.pagememory.AbstractPageMemoryTableStorage;
-import org.apache.ignite.internal.storage.pagememory.VolatilePageMemoryStorageEngine;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageChange;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageConfigurationSchema;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryDataStorageView;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryStorageEngineConfiguration;
-import org.apache.ignite.internal.storage.pagememory.configuration.schema.VolatilePageMemoryStorageEngineConfigurationSchema;
import org.apache.ignite.internal.testframework.WorkDirectory;
import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
import org.apache.ignite.internal.tx.Timestamp;
import org.apache.ignite.internal.util.Cursor;
-import org.apache.ignite.internal.util.IgniteUtils;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+/**
+ * Base test for MV partition storages based on PageMemory.
+ */
@ExtendWith(ConfigurationExtension.class)
@ExtendWith(WorkDirectoryExtension.class)
-class PageMemoryMvPartitionStorageTest extends AbstractMvPartitionStorageTest<PageMemoryMvPartitionStorage> {
- private final PageIoRegistry ioRegistry = new PageIoRegistry();
+abstract class AbstractPageMemoryMvPartitionStorageTest<T extends AbstractPageMemoryMvPartitionStorage> extends
Review Comment:
Why does `AbstractMvPartitionStorageTest` have a generic parameter? I was able to remove it from this class and from all other inheritors without any implications
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/PartitionMetaTest.java:
##########
@@ -80,32 +80,89 @@ void testPageCount() {
assertEquals(2, meta.pageCount());
}
+ @Test
+ void testVersionChainTreeRootPageId() {
Review Comment:
What are these tests for? Just to test setters and getters?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -226,21 +336,92 @@ TableFreeList createTableFreeList(
}
}
+ /**
+ * Returns new {@link VersionChainFreeList} instance for partition.
+ *
+ * @param tableView Table configuration.
+ * @param partId Partition ID.
+ * @param rootPageId Root page ID.
+ * @param initNew {@code True} if new metadata should be initialized.
+ * @throws StorageException If failed.
+ */
+ VersionChainFreeList createVersionChainFreeList(
Review Comment:
Maybe all these *create* methods could be moved to a factory? This will also reduce the number of parameters, since some of them will be stored in the factory itself
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938571561
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -164,8 +170,113 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
- public PageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
- throw new UnsupportedOperationException("Not supported yet");
+ public PersistentPageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
Review Comment:
Let's get rid of the old method in ticket IGNITE-17466, what's stopping you from renaming?
--
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 #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938574852
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PageMemoryMvStorageIoModule.java:
##########
@@ -28,7 +28,7 @@
import org.apache.ignite.internal.storage.pagememory.mv.io.VersionChainMetaIo;
/**
- * {@link PageIoModule} related to {@link PageMemoryMvPartitionStorage} implementation.
+ * {@link PageIoModule} related to {@link AbstractPageMemoryMvPartitionStorage} implementation.
Review Comment:
fix it
--
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] ibessonov commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
ibessonov commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938553216
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryPartitionStorage.java:
##########
@@ -0,0 +1,487 @@
+/*
+ * 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.storage.pagememory;
+
+import static org.apache.ignite.internal.pagememory.PageIdAllocator.MAX_PARTITION_ID;
+
+import java.nio.ByteBuffer;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Predicate;
+import org.apache.ignite.internal.pagememory.tree.BplusTree;
+import org.apache.ignite.internal.pagememory.tree.IgniteTree;
+import org.apache.ignite.internal.storage.DataRow;
+import org.apache.ignite.internal.storage.InvokeClosure;
+import org.apache.ignite.internal.storage.OperationType;
+import org.apache.ignite.internal.storage.PartitionStorage;
+import org.apache.ignite.internal.storage.SearchRow;
+import org.apache.ignite.internal.storage.StorageException;
+import org.apache.ignite.internal.storage.StorageUtils;
+import org.apache.ignite.internal.util.Cursor;
+import org.apache.ignite.internal.util.IgniteCursor;
+import org.apache.ignite.lang.IgniteInternalCheckedException;
+import org.apache.ignite.lang.IgniteInternalException;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Abstract implementation of {@link PartitionStorage} based on a {@link BplusTree}.
+ */
+// TODO: IGNITE-16644 Support snapshots.
+abstract class AbstractPageMemoryPartitionStorage implements PartitionStorage {
Review Comment:
I don't get this class. PartitionStorage is deprecated, what's the point? Can you please explain?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -164,8 +170,113 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
- public PageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
- throw new UnsupportedOperationException("Not supported yet");
+ public PersistentPageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
Review Comment:
Do we need an old method for creating partition? It can just throw UnsupportedOperationException.
Why did you rename partId to partitionId in method that has nothing to do with your code btw? Why was it necessary?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
Why did you inline this? Old code was clean enough, and new code became more complicated. I don't understand the necessity.
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/RowVersionFreeList.java:
##########
@@ -83,7 +80,7 @@ public RowVersionFreeList(
LOG,
metaPageId,
initNew,
- pageListCacheLimit,
+ null,
Review Comment:
If this thing doesn't exist anymore then why do we have this "null" argument?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
- private static VersionChainFreeList createVersionChainFreeList(
- PageMemory pageMemory,
- ReuseList reuseList
- ) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
+ private static VersionChainFreeList createVersionChainFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
return new VersionChainFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
- reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
private static RowVersionFreeList createRowVersionFreeList(
PageMemory pageMemory,
- ReuseList reuseList
+ VersionChainFreeList reuseList
) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(
- FREE_LIST_GROUP_ID,
- FREE_LIST_PARTITION_ID,
- FLAG_AUX
- );
-
return new RowVersionFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
And here. Why?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PageMemoryMvStorageIoModule.java:
##########
@@ -28,7 +28,7 @@
import org.apache.ignite.internal.storage.pagememory.mv.io.VersionChainMetaIo;
/**
- * {@link PageIoModule} related to {@link PageMemoryMvPartitionStorage} implementation.
+ * {@link PageIoModule} related to {@link AbstractPageMemoryMvPartitionStorage} implementation.
Review Comment:
Looks weird when it points to an abstract class. I think we can list 2 MvTableStorage implementations here, would be better.
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -164,8 +170,113 @@ public void destroy() throws StorageException {
/** {@inheritDoc} */
@Override
- public PageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
- throw new UnsupportedOperationException("Not supported yet");
+ public PersistentPageMemoryMvPartitionStorage createMvPartitionStorage(int partitionId) {
+ TableView tableView = tableCfg.value();
+
+ FilePageStore filePageStore = ensurePartitionFilePageStore(tableView, partitionId);
+
+ CheckpointManager checkpointManager = dataRegion.checkpointManager();
+
+ CheckpointTimeoutLock checkpointTimeoutLock = checkpointManager.checkpointTimeoutLock();
+
+ checkpointTimeoutLock.checkpointReadLock();
+
+ try {
+ PersistentPageMemory persistentPageMemory = dataRegion.pageMemory();
+
+ int grpId = tableView.tableId();
+
+ CheckpointProgress lastCheckpointProgress = checkpointManager.lastCheckpointProgress();
+
+ UUID checkpointId = lastCheckpointProgress == null ? null : lastCheckpointProgress.id();
+
+ PartitionMeta meta = dataRegion.partitionMetaManager().readOrCreateMeta(
+ checkpointId,
+ new GroupPartitionId(grpId, partitionId),
+ filePageStore
+ );
+
+ dataRegion.partitionMetaManager().addMeta(new GroupPartitionId(grpId, partitionId), meta);
+
+ filePageStore.pages(meta.pageCount());
+
+ filePageStore.setPageAllocationListener(pageIdx -> {
+ assert checkpointTimeoutLock.checkpointLockIsHeldByThread();
+
+ CheckpointProgress last = checkpointManager.lastCheckpointProgress();
+
+ meta.incrementPageCount(last == null ? null : last.id());
+ });
+
+ boolean initNewVersionChainTree = false;
+
+ if (meta.versionChainTreeRootPageId() == 0) {
+ meta.versionChainTreeRootPageId(checkpointId, persistentPageMemory.allocatePage(grpId, partitionId, FLAG_AUX));
+
+ initNewVersionChainTree = true;
+ }
+
+ boolean initVersionChainFreeList = false;
+
+ if (meta.versionChainFreeListRootPageId() == 0) {
+ meta.versionChainFreeListRootPageId(checkpointId, persistentPageMemory.allocatePage(grpId, partitionId, FLAG_AUX));
+
+ initVersionChainFreeList = true;
+ }
+
+ boolean initRowVersionFreeList = false;
+
+ if (meta.rowVersionFreeListRootPageId() == 0) {
+ meta.rowVersionFreeListRootPageId(checkpointId, persistentPageMemory.allocatePage(grpId, partitionId, FLAG_AUX));
+
+ initRowVersionFreeList = true;
+ }
+
+ VersionChainFreeList versionChainFreeList = createVersionChainFreeList(
+ tableView,
+ partitionId,
+ meta.versionChainFreeListRootPageId(),
+ initVersionChainFreeList
+ );
+
+ autoCloseables.add(versionChainFreeList::close);
+
+ RowVersionFreeList rowVersionFreeList = createRowVersionFreeList(
+ tableView,
+ partitionId,
+ versionChainFreeList,
+ meta.rowVersionFreeListRootPageId(),
+ initRowVersionFreeList
+ );
+
+ autoCloseables.add(rowVersionFreeList::close);
+
+ VersionChainTree versionChainTree = createVersionChainTree(
+ tableView,
+ partitionId,
+ versionChainFreeList,
+ meta.versionChainTreeRootPageId(),
+ initNewVersionChainTree
+ );
+
+ return new PersistentPageMemoryMvPartitionStorage(
+ partitionId,
+ tableView,
+ dataRegion.pageMemory(),
+ versionChainFreeList,
+ rowVersionFreeList,
+ versionChainTree,
+ checkpointTimeoutLock
+ );
+ } catch (IgniteInternalCheckedException e) {
+ throw new StorageException(
+ String.format("Error getting or creating partition metadata [tableName=%s, partitionId=%s]", tableView.name(),
Review Comment:
Not the best description. Just remove the word "metadata" and it'll get better
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
- private static VersionChainFreeList createVersionChainFreeList(
- PageMemory pageMemory,
- ReuseList reuseList
- ) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
+ private static VersionChainFreeList createVersionChainFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
return new VersionChainFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
- reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
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] tkalkirill commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938612266
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -104,60 +103,43 @@ public void start() {
}
private TableFreeList createTableFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
return new TableFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
- private static VersionChainFreeList createVersionChainFreeList(
- PageMemory pageMemory,
- ReuseList reuseList
- ) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX);
-
+ private static VersionChainFreeList createVersionChainFreeList(PageMemory pageMemory) throws IgniteInternalCheckedException {
return new VersionChainFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
- reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
true,
- null,
PageEvictionTrackerNoOp.INSTANCE,
IoStatisticsHolderNoOp.INSTANCE
);
}
private static RowVersionFreeList createRowVersionFreeList(
PageMemory pageMemory,
- ReuseList reuseList
+ VersionChainFreeList reuseList
) throws IgniteInternalCheckedException {
- long metaPageId = pageMemory.allocatePage(
- FREE_LIST_GROUP_ID,
- FREE_LIST_PARTITION_ID,
- FLAG_AUX
- );
-
return new RowVersionFreeList(
FREE_LIST_GROUP_ID,
FREE_LIST_PARTITION_ID,
pageMemory,
reuseList,
PageLockListenerNoOp.INSTANCE,
- metaPageId,
+ pageMemory.allocatePage(FREE_LIST_GROUP_ID, FREE_LIST_PARTITION_ID, FLAG_AUX),
Review Comment:
Fix it
--
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] ibessonov commented on a diff in pull request #976: IGNITE-17085 Support persistent case for page-memory-based MV storage
Posted by GitBox <gi...@apache.org>.
ibessonov commented on code in PR #976:
URL: https://github.com/apache/ignite-3/pull/976#discussion_r938772677
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/PartitionMetaTest.java:
##########
@@ -37,7 +37,7 @@
public class PartitionMetaTest {
@Test
void testTreeRootPageId() {
- PartitionMeta meta = new PartitionMeta(null, 0, 0, 0);
+ PartitionMeta meta = new PartitionMeta(null, 0, 0, 0, 0, 0, 0);
Review Comment:
This constructor starts looking ridiculous, maybe we should use some default constructor and only set fields that we need?
--
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