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/06/22 09:19:17 UTC
[GitHub] [ignite-3] tkalkirill opened a new pull request, #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
tkalkirill opened a new pull request, #892:
URL: https://github.com/apache/ignite-3/pull/892
https://issues.apache.org/jira/browse/IGNITE-17149
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905795619
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryTableStorage.java:
##########
@@ -40,7 +40,7 @@
*/
// TODO: IGNITE-16642 Support indexes.
public abstract class AbstractPageMemoryTableStorage implements TableStorage {
- protected final AbstractPageMemoryDataRegion dataRegion;
+ protected final AbstractPageMemoryDataRegion<?> dataRegion;
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905774098
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/configuration/PageMemoryDataRegionValidatorImplTest.java:
##########
@@ -46,51 +50,120 @@
@ExtendWith(ConfigurationExtension.class)
public class PageMemoryDataRegionValidatorImplTest {
@InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
- private PageMemoryStorageEngineConfiguration engineConfig;
+ private VolatilePageMemoryStorageEngineConfiguration volatileEngineConfig;
+
+ @InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
+ private PersistentPageMemoryStorageEngineConfiguration persistentEngineConfig;
@Test
void testValidationFail() {
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905820977
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
*/
-abstract class AbstractPageMemoryDataRegion implements PageMemoryDataRegion, IgniteComponent {
- protected final PageMemoryDataRegionConfiguration cfg;
+abstract class AbstractPageMemoryDataRegion<T extends PageMemory> implements DataRegion<T>, IgniteComponent {
Review Comment:
got rid of 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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904869103
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointManager.java:
##########
@@ -167,7 +167,7 @@ public CheckpointTimeoutLock checkpointTimeoutLock() {
* @param listener Listener.
* @param dataRegion Persistent data region for which listener is corresponded to, {@code null} for all regions.
*/
- public void addCheckpointListener(CheckpointListener listener, PageMemoryDataRegion dataRegion) {
+ public void addCheckpointListener(CheckpointListener listener, DataRegion dataRegion) {
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905778020
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/ValidationUtilTest.java:
##########
@@ -266,12 +268,81 @@ public void validate(NamedListValidation annotation, ValidationContext<NamedList
);
}
+ @Test
+ void testGetOwner() {
Review Comment:
Added
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906127502
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -34,15 +37,27 @@
import org.apache.ignite.lang.IgniteInternalCheckedException;
/**
- * Implementation of {@link AbstractPageMemoryDataRegion} for in-memory case.
+ * Implementation of {@link DataRegion} for in-memory case.
*/
-class VolatilePageMemoryDataRegion extends AbstractPageMemoryDataRegion {
+class VolatilePageMemoryDataRegion implements DataRegion<VolatilePageMemory>, IgniteComponent {
private static final int FREE_LIST_GROUP_ID = 0;
- private TableFreeList tableFreeList;
+ private final VolatilePageMemoryDataRegionConfiguration cfg;
- private VersionChainFreeList versionChainFreeList;
- private RowVersionFreeList rowVersionFreeList;
+ private final PageIoRegistry ioRegistry;
+
+ private final int pageSize;
+
+ private volatile VolatilePageMemory pageMemory;
+
+ /** Must be one for the in-memory data region. */
Review Comment:
I'll remove this comment, it's not needed.
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906196175
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
In IGNITE-17210
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904951871
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/PersistentPageMemoryDataRegionConfigurationSchema.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.pagememory.configuration.schema;
+
+import static org.apache.ignite.internal.util.Constants.MiB;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.OneOf;
+
+/**
+ * Persistent data region configuration schema.
+ */
+@Config
+public class PersistentPageMemoryDataRegionConfigurationSchema extends BasePageMemoryDataRegionConfigurationSchema {
+ /** Default max size. */
+ public static final long DFLT_DATA_REGION_SIZE = 256 * MiB;
+
+ /** Random-LRU algorithm. */
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] sashapolo commented on a diff in pull request #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905893858
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
*/
-abstract class AbstractPageMemoryDataRegion implements PageMemoryDataRegion, IgniteComponent {
- protected final PageMemoryDataRegionConfiguration cfg;
+abstract class AbstractPageMemoryDataRegion<T extends PageMemory> implements DataRegion<T>, IgniteComponent {
Review Comment:
Because it breaks the intention of this interface. IgniteComponents have their own lifecycle and are managed by the `LifecycleManager`. Anyway, why do you need an interface to have `start` and `stop` methods? What's wrong with declaring them in this class directly?
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906196175
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
In [IGNITE-17210](https://issues.apache.org/jira/browse/IGNITE-17210)
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906200167
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
please add a TODO, so that we won't forget to do 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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905892136
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/PersistentPageMemoryDataRegionConfigurationSchema.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.pagememory.configuration.schema;
+
+import static org.apache.ignite.internal.util.Constants.MiB;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.OneOf;
+
+/**
+ * Persistent data region configuration schema.
+ */
+@Config
+public class PersistentPageMemoryDataRegionConfigurationSchema extends BasePageMemoryDataRegionConfigurationSchema {
+ /** Default max size. */
+ public static final long DFLT_DATA_REGION_SIZE = 256 * MiB;
+
+ /** Random-LRU algorithm. */
+ public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
+
+ /** Segmented-LRU algorithm. */
+ public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
+
+ /** CLOCK algorithm. */
+ public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
+
+ /** Memory region size in bytes. */
+ @Value(hasDefault = true)
+ public long size = DFLT_DATA_REGION_SIZE;
+
+ /** Memory pages replacement mode. */
+ @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
+ @Value(hasDefault = true)
+ public String replacementMode = CLOCK_REPLACEMENT_MODE;
+
+ /**
+ * Write to the page store without holding the segment lock (with a delay). Because other thread may require exactly the same page to be
Review Comment:
That would be perfect, thank you
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905731326
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
How about such a description?
`The maximum number of empty pages Ignite will try to keep.`
I also discovered that in 3.0 we do not use eviction for the data region in memory, I created a [IGNITE-17224](https://issues.apache.org/jira/browse/IGNITE-17224) for 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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904861785
##########
examples/src/integrationTest/java/org/apache/ignite/example/storage/ItPageMemoryStorageExampleTest.java:
##########
@@ -55,10 +57,19 @@ public void testInMemoryExample() throws Exception {
);
}
- private void addDataRegionConfig(String name, boolean persistent) throws Exception {
- ignite.clusterConfiguration().getConfiguration(PageMemoryStorageEngineConfiguration.KEY)
+ private void addVolatileDataRegionConfig(String name) throws Exception {
+ ignite.clusterConfiguration().getConfiguration(VolatilePageMemoryStorageEngineConfiguration.KEY)
.regions()
- .change(regionsChange -> regionsChange.create(name, regionChange -> regionChange.changePersistent(persistent)))
+ .change(regionsChange -> regionsChange.create(name, regionChange -> {
+ }))
Review Comment:
Fix it.
##########
examples/src/integrationTest/java/org/apache/ignite/example/storage/ItPageMemoryStorageExampleTest.java:
##########
@@ -55,10 +57,19 @@ public void testInMemoryExample() throws Exception {
);
}
- private void addDataRegionConfig(String name, boolean persistent) throws Exception {
- ignite.clusterConfiguration().getConfiguration(PageMemoryStorageEngineConfiguration.KEY)
+ private void addVolatileDataRegionConfig(String name) throws Exception {
+ ignite.clusterConfiguration().getConfiguration(VolatilePageMemoryStorageEngineConfiguration.KEY)
.regions()
- .change(regionsChange -> regionsChange.create(name, regionChange -> regionChange.changePersistent(persistent)))
+ .change(regionsChange -> regionsChange.create(name, regionChange -> {
+ }))
+ .get(1, TimeUnit.SECONDS);
+ }
+
+ private void addPersistentDataRegionConfig(String name) throws Exception {
+ ignite.clusterConfiguration().getConfiguration(PersistentPageMemoryStorageEngineConfiguration.KEY)
+ .regions()
+ .change(regionsChange -> regionsChange.create(name, regionChange -> {
+ }))
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904868505
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -246,7 +244,7 @@ public void addCheckpointListener(CheckpointListener listener, @Nullable PageMem
* @param listener Listener.
*/
public void removeCheckpointListener(CheckpointListener listener) {
- listeners.remove(new IgniteBiTuple<CheckpointListener, PageMemoryDataRegion>() {
+ listeners.remove(new IgniteBiTuple<CheckpointListener, DataRegion>() {
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904894863
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/validation/ValidationContextImpl.java:
##########
@@ -132,4 +133,24 @@ public <ROOT> ROOT getNewRoot(RootKey<?, ROOT> rootKey) {
public void addIssue(ValidationIssue issue) {
issues.add(issue);
}
+
+ /** {@inheritDoc} */
+ @Override
+ public <T> @Nullable T getOldOwner() {
+ try {
+ return currentPath.size() <= 1 ? null : find(currentPath.subList(0, currentPath.size() - 1), oldRoots, true);
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] sashapolo commented on a diff in pull request #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904924698
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/persistence/ItBplusTreeReuseListPersistentPageMemoryTest.java:
##########
@@ -22,24 +22,30 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.LongStream;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
import org.apache.ignite.internal.pagememory.PageMemory;
import org.apache.ignite.internal.pagememory.TestPageIoRegistry;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.tree.ItBplusTreeReuseSelfTest;
/**
- * Test with reuse list and {@link PageMemoryImpl}.
+ * Test with reuse list and {@link PersistentPageMemory}.
*/
-public class ItBplusTreeReuseListPageMemoryImplTest extends ItBplusTreeReuseSelfTest {
+public class ItBplusTreeReuseListPersistentPageMemoryTest extends ItBplusTreeReuseSelfTest {
+ @InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
+ private PersistentPageMemoryDataRegionConfiguration dataRegionCfg;
Review Comment:
looks like this comment got messed up a little bit. I meant that this class should extend a common superclass, please see my comment to a similar case
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904868234
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -260,29 +258,27 @@ public boolean equals(Object o) {
*
* @param dataRegions Data regions.
*/
- public List<CheckpointListener> collectCheckpointListeners(Collection<? extends PageMemoryDataRegion> dataRegions) {
+ public List<CheckpointListener> collectCheckpointListeners(Collection<? extends DataRegion> dataRegions) {
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905821119
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
Review Comment:
got rid of 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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904949279
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/persistence/ItBplusTreeReuseListPersistentPageMemoryTest.java:
##########
@@ -22,24 +22,30 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.LongStream;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
import org.apache.ignite.internal.pagememory.PageMemory;
import org.apache.ignite.internal.pagememory.TestPageIoRegistry;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.tree.ItBplusTreeReuseSelfTest;
/**
- * Test with reuse list and {@link PageMemoryImpl}.
+ * Test with reuse list and {@link PersistentPageMemory}.
*/
-public class ItBplusTreeReuseListPageMemoryImplTest extends ItBplusTreeReuseSelfTest {
+public class ItBplusTreeReuseListPersistentPageMemoryTest extends ItBplusTreeReuseSelfTest {
+ @InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
+ private PersistentPageMemoryDataRegionConfiguration dataRegionCfg;
Review Comment:
Fix it
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/persistence/ItBplusTreePersistentPageMemoryTest.java:
##########
@@ -22,25 +22,31 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.LongStream;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
import org.apache.ignite.internal.pagememory.PageMemory;
import org.apache.ignite.internal.pagememory.TestPageIoRegistry;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.tree.BplusTree;
import org.apache.ignite.internal.pagememory.tree.ItBplusTreeSelfTest;
/**
- * Class to test the {@link BplusTree} with {@link PageMemoryImpl}.
+ * Class to test the {@link BplusTree} with {@link PersistentPageMemory}.
*/
-public class ItBplusTreePageMemoryImplTest extends ItBplusTreeSelfTest {
+public class ItBplusTreePersistentPageMemoryTest extends ItBplusTreeSelfTest {
+ @InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
+ private PersistentPageMemoryDataRegionConfiguration dataRegionCfg;
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905901420
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/PersistentPageMemoryDataRegionConfigurationSchema.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.pagememory.configuration.schema;
+
+import static org.apache.ignite.internal.util.Constants.MiB;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.OneOf;
+
+/**
+ * Persistent data region configuration schema.
+ */
+@Config
+public class PersistentPageMemoryDataRegionConfigurationSchema extends BasePageMemoryDataRegionConfigurationSchema {
+ /** Default max size. */
+ public static final long DFLT_DATA_REGION_SIZE = 256 * MiB;
+
+ /** Random-LRU algorithm. */
+ public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
+
+ /** Segmented-LRU algorithm. */
+ public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
+
+ /** CLOCK algorithm. */
+ public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
+
+ /** Memory region size in bytes. */
+ @Value(hasDefault = true)
+ public long size = DFLT_DATA_REGION_SIZE;
+
+ /** Memory pages replacement mode. */
+ @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
+ @Value(hasDefault = true)
+ public String replacementMode = CLOCK_REPLACEMENT_MODE;
+
+ /**
+ * Write to the page store without holding the segment lock (with a delay). Because other thread may require exactly the same page to be
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] sashapolo commented on a diff in pull request #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906172977
##########
modules/core/src/test/java/org/apache/ignite/internal/testframework/IgniteTestUtils.java:
##########
@@ -745,4 +746,12 @@ public static <T> T await(CompletionStage<T> stage) {
return null;
}
+
+ /**
+ * Returns a consumer that does nothing.
+ */
+ public static <T> Consumer<T> doNothingConsumer() {
Review Comment:
Please set the following checkbox and everything will be fine
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904922843
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/persistence/ItBplusTreeReuseListPersistentPageMemoryTest.java:
##########
@@ -22,24 +22,30 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.LongStream;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
import org.apache.ignite.internal.pagememory.PageMemory;
import org.apache.ignite.internal.pagememory.TestPageIoRegistry;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.tree.ItBplusTreeReuseSelfTest;
/**
- * Test with reuse list and {@link PageMemoryImpl}.
+ * Test with reuse list and {@link PersistentPageMemory}.
*/
-public class ItBplusTreeReuseListPageMemoryImplTest extends ItBplusTreeReuseSelfTest {
+public class ItBplusTreeReuseListPersistentPageMemoryTest extends ItBplusTreeReuseSelfTest {
+ @InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
+ private PersistentPageMemoryDataRegionConfiguration dataRegionCfg;
Review Comment:
What do you mean?
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904905489
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/ValidationUtilTest.java:
##########
@@ -266,12 +268,81 @@ public void validate(NamedListValidation annotation, ValidationContext<NamedList
);
}
+ @Test
+ void testGetOwner() {
+ var rootsNode = new SuperRoot(key -> null, Map.of(ValidatedRootConfiguration.KEY, root));
+
+ Validator<InnerValidation, Object> innerValidator = new Validator<>() {
+ /** {@inheritDoc} */
+ @Override
+ public void validate(InnerValidation annotation, ValidationContext<Object> ctx) {
+ Object oldOwner = ctx.getOldOwner();
+ Object newOwner = ctx.getOldOwner();
+
+ if (!(oldOwner instanceof ValidatedRootView)) {
+ ctx.addIssue(new ExValidationIssue("Wrong inner owner", ctx.currentKey(), null, oldOwner));
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904903086
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/validation/ValidationContext.java:
##########
@@ -72,4 +72,18 @@
* @see ConfigurationValidationException
*/
void addIssue(ValidationIssue issue);
+
+ /**
+ * Returns the previous owner view of the current value.
Review Comment:
What about the new description?
```
/**
* Returns a previous view of the owner of the current value.
*
* <p>For primitives/nested configuration/named list, the owner is the configuration in which they are declared, for example {@code
* TableView} will be returned for the string field, {@code null} will be returned for the root configuration.
*
* @param <T> Owner view type.
*/
```
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906106291
##########
modules/core/src/test/java/org/apache/ignite/internal/testframework/IgniteTestUtils.java:
##########
@@ -745,4 +746,12 @@ public static <T> T await(CompletionStage<T> stage) {
return null;
}
+
+ /**
+ * Returns a consumer that does nothing.
+ */
+ public static <T> Consumer<T> doNothingConsumer() {
Review Comment:
I'm not able to format the code with `t -> {}` so that it is on the same line, formatting will definitely be in order with this approach, let's leave it as it is.
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905898023
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
In fact, not only this parameter will need to be changed to avoid **IgniteOutOfMemoryError**.
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905761190
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
*/
-abstract class AbstractPageMemoryDataRegion implements PageMemoryDataRegion, IgniteComponent {
- protected final PageMemoryDataRegionConfiguration cfg;
+abstract class AbstractPageMemoryDataRegion<T extends PageMemory> implements DataRegion<T>, IgniteComponent {
Review Comment:
I think I'll just get rid of 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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905759565
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemoryNoLoadTest.java:
##########
@@ -56,13 +58,16 @@
import org.junit.jupiter.api.extension.ExtendWith;
/**
- * Tests {@link PageMemoryImpl}.
+ * Tests {@link PersistentPageMemory}.
*/
@ExtendWith(WorkDirectoryExtension.class)
-public class PageMemoryImplNoLoadTest extends PageMemoryNoLoadSelfTest {
+public class PersistentPageMemoryNoLoadTest extends VolatilePageMemoryNoLoadSelfTest {
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] sashapolo commented on a diff in pull request #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906170107
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
I agree with you, but who is going to describe it in the documentation?
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905017047
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/ValidationUtilTest.java:
##########
@@ -266,12 +268,81 @@ public void validate(NamedListValidation annotation, ValidationContext<NamedList
);
}
+ @Test
+ void testGetOwner() {
Review Comment:
I would propose to add some comments in the test
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905897419
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
It will need to be changed so that **IgniteOutOfMemoryError** does not happen on the next start of the node.
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904868005
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -260,29 +258,27 @@ public boolean equals(Object o) {
*
* @param dataRegions Data regions.
*/
- public List<CheckpointListener> collectCheckpointListeners(Collection<? extends PageMemoryDataRegion> dataRegions) {
+ public List<CheckpointListener> collectCheckpointListeners(Collection<? extends DataRegion> dataRegions) {
return listeners.stream()
.filter(tuple -> tuple.getValue() == null || dataRegions.contains(tuple.getValue()))
.map(IgniteBiTuple::getKey)
.collect(toUnmodifiableList());
}
private CheckpointDirtyPagesInfoHolder beginCheckpoint(
- Collection<? extends PageMemoryDataRegion> dataRegions,
+ Collection<? extends DataRegion> dataRegions,
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906113812
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryDataRegion.java:
##########
@@ -21,21 +21,32 @@
import static org.apache.ignite.internal.util.Constants.MiB;
import java.util.Arrays;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionView;
+import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionView;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
-import org.apache.ignite.internal.pagememory.persistence.PageMemoryImpl;
+import org.apache.ignite.internal.pagememory.persistence.PersistentPageMemory;
import org.apache.ignite.internal.pagememory.persistence.checkpoint.CheckpointManager;
import org.apache.ignite.internal.pagememory.persistence.store.FilePageStoreManager;
+import org.apache.ignite.internal.storage.StorageException;
/**
- * Implementation of {@link AbstractPageMemoryDataRegion} for persistent case.
+ * Implementation of {@link DataRegion} for persistent case.
*/
-class PersistentPageMemoryDataRegion extends AbstractPageMemoryDataRegion {
+class PersistentPageMemoryDataRegion implements DataRegion<PersistentPageMemory>, IgniteComponent {
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904920107
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/PageMemoryTestUtils.java:
##########
@@ -25,15 +25,12 @@
*/
public class PageMemoryTestUtils {
/**
- * Returns mocked instance of {@link PageMemoryDataRegion}.
+ * Returns mocked instance of {@link DataRegion}.
*
- * @param persistent Data region.
* @param pageMemory Page memory.
*/
- public static PageMemoryDataRegion newDataRegion(boolean persistent, PageMemory pageMemory) {
- PageMemoryDataRegion mock = mock(PageMemoryDataRegion.class);
-
- when(mock.persistent()).thenReturn(persistent);
+ public static <T extends PageMemory> DataRegion<T> newDataRegion(T pageMemory) {
+ DataRegion<T> mock = mock(DataRegion.class);
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904921824
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/validation/ValidationContext.java:
##########
@@ -72,4 +72,18 @@
* @see ConfigurationValidationException
*/
void addIssue(ValidationIssue issue);
+
+ /**
+ * Returns the previous owner view of the current value.
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905721151
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/PersistentPageMemoryDataRegionConfigurationSchema.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.pagememory.configuration.schema;
+
+import static org.apache.ignite.internal.util.Constants.MiB;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.OneOf;
+
+/**
+ * Persistent data region configuration schema.
+ */
+@Config
+public class PersistentPageMemoryDataRegionConfigurationSchema extends BasePageMemoryDataRegionConfigurationSchema {
+ /** Default max size. */
+ public static final long DFLT_DATA_REGION_SIZE = 256 * MiB;
+
+ /** Random-LRU algorithm. */
+ public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
+
+ /** Segmented-LRU algorithm. */
+ public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
+
+ /** CLOCK algorithm. */
+ public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
+
+ /** Memory region size in bytes. */
+ @Value(hasDefault = true)
+ public long size = DFLT_DATA_REGION_SIZE;
+
+ /** Memory pages replacement mode. */
+ @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
+ @Value(hasDefault = true)
+ public String replacementMode = CLOCK_REPLACEMENT_MODE;
+
+ /**
+ * Write to the page store without holding the segment lock (with a delay). Because other thread may require exactly the same page to be
Review Comment:
This configuration came from 2.0, where it is presented as a system property **IGNITE-7606**, after looking at the code it seems that in very rare cases it will have the value **false**, I suggest removing it from the current configuration, if we see any problems with this, we will return it, wdyt?
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905918449
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
Got it. I would suggest to phrase it like this: `Maximum amount of empty pages to keep in memory`. I would also add a description about why you would need to alter this parameter.
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905892677
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
But why would I need to alter this property?
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905895482
##########
modules/core/src/test/java/org/apache/ignite/internal/testframework/IgniteTestUtils.java:
##########
@@ -745,4 +746,12 @@ public static <T> T await(CompletionStage<T> stage) {
return null;
}
+
+ /**
+ * Returns a consumer that does nothing.
+ */
+ public static <T> Consumer<T> doNothingConsumer() {
Review Comment:
Why is this method needed? Isn't `t -> {}` obvious by itself? This class is huge enough already)
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/tree/ItBplusTreePageMemoryTest.java:
##########
@@ -105,13 +99,11 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
-import org.junit.jupiter.api.extension.ExtendWith;
/**
- * Class to test the {@link BplusTree}.
+ * An abstract class for testing {@link BplusTree} using different implementations of {@link PageMemory}.
*/
-@ExtendWith(ConfigurationExtension.class)
-public class ItBplusTreeSelfTest extends BaseIgniteAbstractTest {
+public abstract class ItBplusTreePageMemoryTest extends BaseIgniteAbstractTest {
Review Comment:
I guess since this class is now abstract and does not contain any tests, it should not start with `It`
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/tree/ItBplusTreeReusePageMemoryTest.java:
##########
@@ -33,9 +33,9 @@
import org.apache.ignite.lang.IgniteInternalCheckedException;
/**
- * Test with reuse list.
+ * An abstract class for testing {@link BplusTree} with {@link ReuseList} using different implementations of {@link PageMemory}.
*/
-public class ItBplusTreeReuseSelfTest extends ItBplusTreeSelfTest {
+public abstract class ItBplusTreeReusePageMemoryTest extends ItBplusTreePageMemoryTest {
Review Comment:
same here
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryDataRegion.java:
##########
@@ -21,21 +21,32 @@
import static org.apache.ignite.internal.util.Constants.MiB;
import java.util.Arrays;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionView;
+import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionView;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
-import org.apache.ignite.internal.pagememory.persistence.PageMemoryImpl;
+import org.apache.ignite.internal.pagememory.persistence.PersistentPageMemory;
import org.apache.ignite.internal.pagememory.persistence.checkpoint.CheckpointManager;
import org.apache.ignite.internal.pagememory.persistence.store.FilePageStoreManager;
+import org.apache.ignite.internal.storage.StorageException;
/**
- * Implementation of {@link AbstractPageMemoryDataRegion} for persistent case.
+ * Implementation of {@link DataRegion} for persistent case.
*/
-class PersistentPageMemoryDataRegion extends AbstractPageMemoryDataRegion {
+class PersistentPageMemoryDataRegion implements DataRegion<PersistentPageMemory>, IgniteComponent {
Review Comment:
please see my comments about `IgniteComponent`. I'm strongly against using this interface in anything but actual components (or maybe I understand it in a wrong way). I think that `start` and `stop` methods should simply be declared in the `DataRegion` interface
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -34,15 +37,27 @@
import org.apache.ignite.lang.IgniteInternalCheckedException;
/**
- * Implementation of {@link AbstractPageMemoryDataRegion} for in-memory case.
+ * Implementation of {@link DataRegion} for in-memory case.
*/
-class VolatilePageMemoryDataRegion extends AbstractPageMemoryDataRegion {
+class VolatilePageMemoryDataRegion implements DataRegion<VolatilePageMemory>, IgniteComponent {
private static final int FREE_LIST_GROUP_ID = 0;
- private TableFreeList tableFreeList;
+ private final VolatilePageMemoryDataRegionConfiguration cfg;
- private VersionChainFreeList versionChainFreeList;
- private RowVersionFreeList rowVersionFreeList;
+ private final PageIoRegistry ioRegistry;
+
+ private final int pageSize;
+
+ private volatile VolatilePageMemory pageMemory;
+
+ /** Must be one for the in-memory data region. */
Review Comment:
What do you mean by `Must be one`?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryTableStorage.java:
##########
@@ -39,8 +40,8 @@
* Abstract table storage implementation based on {@link PageMemory}.
*/
// TODO: IGNITE-16642 Support indexes.
-public abstract class AbstractPageMemoryTableStorage implements TableStorage {
- protected final AbstractPageMemoryDataRegion dataRegion;
+public abstract class AbstractPageMemoryTableStorage<T extends DataRegion<? extends PageMemory>> implements TableStorage {
+ protected final T dataRegion;
Review Comment:
I can see that `dataRegion` is only stored here for the `createMvPartitionStorage` method which is `TestOnly`. Can we make `createMvPartitionStorage` abstract? This will allow us to:
1. Remove the protected field
2. Remove the generic parameter from this class
3. Remove the cast to `VolatilePageMemoryDataRegion` in `createMvPartitionStorage` implementation
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904918254
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/validation/ValidationContext.java:
##########
@@ -72,4 +72,18 @@
* @see ConfigurationValidationException
*/
void addIssue(ValidationIssue issue);
+
+ /**
+ * Returns the previous owner view of the current value.
Review Comment:
Looks perfect
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r905760558
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
*/
-abstract class AbstractPageMemoryDataRegion implements PageMemoryDataRegion, IgniteComponent {
- protected final PageMemoryDataRegionConfiguration cfg;
+abstract class AbstractPageMemoryDataRegion<T extends PageMemory> implements DataRegion<T>, IgniteComponent {
Review Comment:
I don't understand why it's bad?
--
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] vveider merged pull request #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
vveider merged PR #892:
URL: https://github.com/apache/ignite-3/pull/892
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906104905
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
*/
-abstract class AbstractPageMemoryDataRegion implements PageMemoryDataRegion, IgniteComponent {
- protected final PageMemoryDataRegionConfiguration cfg;
+abstract class AbstractPageMemoryDataRegion<T extends PageMemory> implements DataRegion<T>, IgniteComponent {
Review Comment:
You're right, I'll 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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906135648
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
I think here you can leave a short description.
And about how you need to change these parameters, it is better to describe in the documentation.
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906134319
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryTableStorage.java:
##########
@@ -39,8 +40,8 @@
* Abstract table storage implementation based on {@link PageMemory}.
*/
// TODO: IGNITE-16642 Support indexes.
-public abstract class AbstractPageMemoryTableStorage implements TableStorage {
- protected final AbstractPageMemoryDataRegion dataRegion;
+public abstract class AbstractPageMemoryTableStorage<T extends DataRegion<? extends PageMemory>> implements TableStorage {
+ protected final T dataRegion;
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906193432
##########
modules/core/src/test/java/org/apache/ignite/internal/testframework/IgniteTestUtils.java:
##########
@@ -745,4 +746,12 @@ public static <T> T await(CompletionStage<T> stage) {
return null;
}
+
+ /**
+ * Returns a consumer that does nothing.
+ */
+ public static <T> Consumer<T> doNothingConsumer() {
Review Comment:
Thanks, I'll 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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r906112727
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/tree/ItBplusTreePageMemoryTest.java:
##########
@@ -105,13 +99,11 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
-import org.junit.jupiter.api.extension.ExtendWith;
/**
- * Class to test the {@link BplusTree}.
+ * An abstract class for testing {@link BplusTree} using different implementations of {@link PageMemory}.
*/
-@ExtendWith(ConfigurationExtension.class)
-public class ItBplusTreeSelfTest extends BaseIgniteAbstractTest {
+public abstract class ItBplusTreePageMemoryTest extends BaseIgniteAbstractTest {
Review Comment:
Fix it
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/tree/ItBplusTreeReusePageMemoryTest.java:
##########
@@ -33,9 +33,9 @@
import org.apache.ignite.lang.IgniteInternalCheckedException;
/**
- * Test with reuse list.
+ * An abstract class for testing {@link BplusTree} with {@link ReuseList} using different implementations of {@link PageMemory}.
*/
-public class ItBplusTreeReuseSelfTest extends ItBplusTreeSelfTest {
+public abstract class ItBplusTreeReusePageMemoryTest extends ItBplusTreePageMemoryTest {
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] sashapolo commented on a diff in pull request #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904762667
##########
examples/src/integrationTest/java/org/apache/ignite/example/storage/ItPageMemoryStorageExampleTest.java:
##########
@@ -55,10 +57,19 @@ public void testInMemoryExample() throws Exception {
);
}
- private void addDataRegionConfig(String name, boolean persistent) throws Exception {
- ignite.clusterConfiguration().getConfiguration(PageMemoryStorageEngineConfiguration.KEY)
+ private void addVolatileDataRegionConfig(String name) throws Exception {
+ ignite.clusterConfiguration().getConfiguration(VolatilePageMemoryStorageEngineConfiguration.KEY)
.regions()
- .change(regionsChange -> regionsChange.create(name, regionChange -> regionChange.changePersistent(persistent)))
+ .change(regionsChange -> regionsChange.create(name, regionChange -> {
+ }))
Review Comment:
can we move this to the previous line?
##########
examples/src/integrationTest/java/org/apache/ignite/example/storage/ItPageMemoryStorageExampleTest.java:
##########
@@ -55,10 +57,19 @@ public void testInMemoryExample() throws Exception {
);
}
- private void addDataRegionConfig(String name, boolean persistent) throws Exception {
- ignite.clusterConfiguration().getConfiguration(PageMemoryStorageEngineConfiguration.KEY)
+ private void addVolatileDataRegionConfig(String name) throws Exception {
+ ignite.clusterConfiguration().getConfiguration(VolatilePageMemoryStorageEngineConfiguration.KEY)
.regions()
- .change(regionsChange -> regionsChange.create(name, regionChange -> regionChange.changePersistent(persistent)))
+ .change(regionsChange -> regionsChange.create(name, regionChange -> {
+ }))
+ .get(1, TimeUnit.SECONDS);
+ }
+
+ private void addPersistentDataRegionConfig(String name) throws Exception {
+ ignite.clusterConfiguration().getConfiguration(PersistentPageMemoryStorageEngineConfiguration.KEY)
+ .regions()
+ .change(regionsChange -> regionsChange.create(name, regionChange -> {
+ }))
Review Comment:
same here
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/persistence/ItBplusTreeReuseListPersistentPageMemoryTest.java:
##########
@@ -22,24 +22,30 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.LongStream;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
import org.apache.ignite.internal.pagememory.PageMemory;
import org.apache.ignite.internal.pagememory.TestPageIoRegistry;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.tree.ItBplusTreeReuseSelfTest;
/**
- * Test with reuse list and {@link PageMemoryImpl}.
+ * Test with reuse list and {@link PersistentPageMemory}.
*/
-public class ItBplusTreeReuseListPageMemoryImplTest extends ItBplusTreeReuseSelfTest {
+public class ItBplusTreeReuseListPersistentPageMemoryTest extends ItBplusTreeReuseSelfTest {
+ @InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
+ private PersistentPageMemoryDataRegionConfiguration dataRegionCfg;
Review Comment:
same here
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/validation/ValidationContextImpl.java:
##########
@@ -132,4 +133,24 @@ public <ROOT> ROOT getNewRoot(RootKey<?, ROOT> rootKey) {
public void addIssue(ValidationIssue issue) {
issues.add(issue);
}
+
+ /** {@inheritDoc} */
+ @Override
+ public <T> @Nullable T getOldOwner() {
+ try {
+ return currentPath.size() <= 1 ? null : find(currentPath.subList(0, currentPath.size() - 1), oldRoots, true);
Review Comment:
I would suggest to extract the common logic into a helper method. Something like `T findOwner(SuperRoot)`
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/ValidationUtilTest.java:
##########
@@ -266,12 +268,81 @@ public void validate(NamedListValidation annotation, ValidationContext<NamedList
);
}
+ @Test
+ void testGetOwner() {
+ var rootsNode = new SuperRoot(key -> null, Map.of(ValidatedRootConfiguration.KEY, root));
+
+ Validator<InnerValidation, Object> innerValidator = new Validator<>() {
+ /** {@inheritDoc} */
+ @Override
+ public void validate(InnerValidation annotation, ValidationContext<Object> ctx) {
+ Object oldOwner = ctx.getOldOwner();
+ Object newOwner = ctx.getOldOwner();
+
+ if (!(oldOwner instanceof ValidatedRootView)) {
+ ctx.addIssue(new ExValidationIssue("Wrong inner owner", ctx.currentKey(), null, oldOwner));
Review Comment:
Last parameter of the `ExValidationIssue` should be marked as `Nullable`
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/validation/ValidationContext.java:
##########
@@ -72,4 +72,18 @@
* @see ConfigurationValidationException
*/
void addIssue(ValidationIssue issue);
+
+ /**
+ * Returns the previous owner view of the current value.
Review Comment:
What is an "owner view"?
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/persistence/ItBplusTreePersistentPageMemoryTest.java:
##########
@@ -22,25 +22,31 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.LongStream;
+import org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
import org.apache.ignite.internal.pagememory.PageMemory;
import org.apache.ignite.internal.pagememory.TestPageIoRegistry;
+import org.apache.ignite.internal.pagememory.configuration.schema.PersistentPageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.UnsafeMemoryAllocatorConfigurationSchema;
import org.apache.ignite.internal.pagememory.tree.BplusTree;
import org.apache.ignite.internal.pagememory.tree.ItBplusTreeSelfTest;
/**
- * Class to test the {@link BplusTree} with {@link PageMemoryImpl}.
+ * Class to test the {@link BplusTree} with {@link PersistentPageMemory}.
*/
-public class ItBplusTreePageMemoryImplTest extends ItBplusTreeSelfTest {
+public class ItBplusTreePersistentPageMemoryTest extends ItBplusTreeSelfTest {
+ @InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
+ private PersistentPageMemoryDataRegionConfiguration dataRegionCfg;
Review Comment:
we also inject a configuration into the superclass, can we extract it into a new class as well, making `ItBplusTreeSelfTest` abstract?
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/VolatilePageMemoryDataRegionConfigurationSchema.java:
##########
@@ -43,56 +43,30 @@ public class PageMemoryDataRegionConfigurationSchema {
/** Random-2-LRU algorithm: scan-resistant version of Random-LRU. */
public static final String RANDOM_2_LRU_EVICTION_MODE = "RANDOM_2_LRU";
- /** Random-LRU algorithm. */
- public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
-
- /** Segmented-LRU algorithm. */
- public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
-
- /** CLOCK algorithm. */
- public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
-
- /** Name of the data region. */
- @InjectedName
- public String name;
-
- @Value(hasDefault = true)
- public boolean persistent = false;
-
+ /** Initial memory region size in bytes, when the used memory size exceeds this value, new chunks of memory will be allocated. */
@Value(hasDefault = true)
public long initSize = DFLT_DATA_REGION_INITIAL_SIZE;
+ /** Maximum memory region size in bytes. */
@Value(hasDefault = true)
public long maxSize = DFLT_DATA_REGION_MAX_SIZE;
- @ConfigValue
- public MemoryAllocatorConfigurationSchema memoryAllocator;
-
+ /** Memory pages eviction mode. */
@OneOf({DISABLED_EVICTION_MODE, RANDOM_LRU_EVICTION_MODE, RANDOM_2_LRU_EVICTION_MODE})
@Value(hasDefault = true)
public String evictionMode = DISABLED_EVICTION_MODE;
- @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
- @Value(hasDefault = true)
- public String replacementMode = CLOCK_REPLACEMENT_MODE;
-
+ /**
+ * Threshold for memory pages eviction initiation. For instance, if the threshold is 0.9 it means that the page memory will start the
+ * eviction only after 90% of the data region is occupied.
+ */
@Value(hasDefault = true)
public double evictionThreshold = 0.9;
- @Value(hasDefault = true)
- public int emptyPagesPoolSize = 100;
-
- @Value(hasDefault = true)
- public long checkpointPageBufSize = 0;
-
- @Value(hasDefault = true)
- public boolean lazyMemoryAllocation = true;
-
/**
- * Write to the page store without holding the segment lock (with a delay).
- *
- * <p>Because other thread may require exactly the same page to be loaded from page store, reads are protected by locking.
+ * Number of empty pages to be present in reuse lists. This parameter ensures that Ignite will be able to successfully evict old data
Review Comment:
Again, this is not clear, how this parameter should influence anything
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -80,13 +80,13 @@ class CheckpointWorkflow implements IgniteComponent {
private final CheckpointReadWriteLock checkpointReadWriteLock;
/** Persistent data regions for the checkpointing. */
- private final Collection<? extends PageMemoryDataRegion> dataRegions;
+ private final Collection<? extends DataRegion<PersistentPageMemory>> dataRegions;
/** Checkpoint write order configuration. */
private final CheckpointWriteOrder checkpointWriteOrder;
/** Collections of checkpoint listeners. */
- private final List<IgniteBiTuple<CheckpointListener, PageMemoryDataRegion>> listeners = new CopyOnWriteArrayList<>();
+ private final List<IgniteBiTuple<CheckpointListener, DataRegion>> listeners = new CopyOnWriteArrayList<>();
Review Comment:
```suggestion
private final List<IgniteBiTuple<CheckpointListener, DataRegion<PersistentPageMemory>>> listeners = new CopyOnWriteArrayList<>();
```
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -246,7 +244,7 @@ public void addCheckpointListener(CheckpointListener listener, @Nullable PageMem
* @param listener Listener.
*/
public void removeCheckpointListener(CheckpointListener listener) {
- listeners.remove(new IgniteBiTuple<CheckpointListener, PageMemoryDataRegion>() {
+ listeners.remove(new IgniteBiTuple<CheckpointListener, DataRegion>() {
Review Comment:
```suggestion
listeners.remove(new IgniteBiTuple<CheckpointListener, DataRegion<PersistentPageMemory>>() {
```
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointManager.java:
##########
@@ -167,7 +167,7 @@ public CheckpointTimeoutLock checkpointTimeoutLock() {
* @param listener Listener.
* @param dataRegion Persistent data region for which listener is corresponded to, {@code null} for all regions.
*/
- public void addCheckpointListener(CheckpointListener listener, PageMemoryDataRegion dataRegion) {
+ public void addCheckpointListener(CheckpointListener listener, DataRegion dataRegion) {
Review Comment:
```suggestion
public void addCheckpointListener(CheckpointListener listener, DataRegion<PersistentPageMemory> dataRegion) {
```
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -260,29 +258,27 @@ public boolean equals(Object o) {
*
* @param dataRegions Data regions.
*/
- public List<CheckpointListener> collectCheckpointListeners(Collection<? extends PageMemoryDataRegion> dataRegions) {
+ public List<CheckpointListener> collectCheckpointListeners(Collection<? extends DataRegion> dataRegions) {
Review Comment:
```suggestion
public List<CheckpointListener> collectCheckpointListeners(Collection<? extends DataRegion<PersistentPageMemory>> dataRegions) {
```
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/PageMemoryTestUtils.java:
##########
@@ -25,15 +25,12 @@
*/
public class PageMemoryTestUtils {
/**
- * Returns mocked instance of {@link PageMemoryDataRegion}.
+ * Returns mocked instance of {@link DataRegion}.
*
- * @param persistent Data region.
* @param pageMemory Page memory.
*/
- public static PageMemoryDataRegion newDataRegion(boolean persistent, PageMemory pageMemory) {
- PageMemoryDataRegion mock = mock(PageMemoryDataRegion.class);
-
- when(mock.persistent()).thenReturn(persistent);
+ public static <T extends PageMemory> DataRegion<T> newDataRegion(T pageMemory) {
+ DataRegion<T> mock = mock(DataRegion.class);
Review Comment:
What's the point of using Mockito here? We can simply write `return () -> pageMemory`. Also. since this method is so short, I think it's better to inline it and remove this class.
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -260,29 +258,27 @@ public boolean equals(Object o) {
*
* @param dataRegions Data regions.
*/
- public List<CheckpointListener> collectCheckpointListeners(Collection<? extends PageMemoryDataRegion> dataRegions) {
+ public List<CheckpointListener> collectCheckpointListeners(Collection<? extends DataRegion> dataRegions) {
return listeners.stream()
.filter(tuple -> tuple.getValue() == null || dataRegions.contains(tuple.getValue()))
.map(IgniteBiTuple::getKey)
.collect(toUnmodifiableList());
}
private CheckpointDirtyPagesInfoHolder beginCheckpoint(
- Collection<? extends PageMemoryDataRegion> dataRegions,
+ Collection<? extends DataRegion> dataRegions,
Review Comment:
```suggestion
Collection<? extends DataRegion<PersistentPageMemory>> dataRegions,
```
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/ValidationUtilTest.java:
##########
@@ -266,12 +268,81 @@ public void validate(NamedListValidation annotation, ValidationContext<NamedList
);
}
+ @Test
+ void testGetOwner() {
Review Comment:
Can you please add comments throughout this method with descriptions what are you actually testing?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PageMemoryMvPartitionStorage.java:
##########
@@ -78,7 +78,7 @@ public class PageMemoryMvPartitionStorage implements MvPartitionStorage {
public PageMemoryMvPartitionStorage(
int partitionId,
TableView tableConfig,
- PageMemoryDataRegion dataRegion,
+ DataRegion dataRegion,
Review Comment:
```suggestion
DataRegion<?> dataRegion,
```
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
Review Comment:
This link is no longer valid
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
*/
-abstract class AbstractPageMemoryDataRegion implements PageMemoryDataRegion, IgniteComponent {
- protected final PageMemoryDataRegionConfiguration cfg;
+abstract class AbstractPageMemoryDataRegion<T extends PageMemory> implements DataRegion<T>, IgniteComponent {
Review Comment:
`IgniteComponent` is an interface that should be used solely by "Ignite components" and not any class that wants to have start-stop semantics. Please consider adding `start` and `stop` methods manually
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryTableStorage.java:
##########
@@ -40,7 +40,7 @@
*/
// TODO: IGNITE-16642 Support indexes.
public abstract class AbstractPageMemoryTableStorage implements TableStorage {
- protected final AbstractPageMemoryDataRegion dataRegion;
+ protected final AbstractPageMemoryDataRegion<?> dataRegion;
Review Comment:
This clearly shows that `AbstractPageMemoryDataRegion` should not have a generic argument
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/AbstractPageMemoryDataRegion.java:
##########
@@ -17,24 +17,24 @@
package org.apache.ignite.internal.storage.pagememory;
-import java.util.Objects;
import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.pagememory.DataRegion;
import org.apache.ignite.internal.pagememory.PageMemory;
-import org.apache.ignite.internal.pagememory.PageMemoryDataRegion;
-import org.apache.ignite.internal.pagememory.configuration.schema.PageMemoryDataRegionConfiguration;
+import org.apache.ignite.internal.pagememory.configuration.schema.BasePageMemoryDataRegionConfiguration;
import org.apache.ignite.internal.pagememory.io.PageIoRegistry;
+import org.apache.ignite.internal.storage.StorageException;
/**
* Abstract data region for {@link PageMemoryStorageEngine}. Based on a {@link PageMemory}.
*/
-abstract class AbstractPageMemoryDataRegion implements PageMemoryDataRegion, IgniteComponent {
- protected final PageMemoryDataRegionConfiguration cfg;
+abstract class AbstractPageMemoryDataRegion<T extends PageMemory> implements DataRegion<T>, IgniteComponent {
Review Comment:
Why is this class needed anyway? It does not encapsulate much common logic and is always casted to a particular implementation. Can we make it an interface instead?
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/configuration/PageMemoryDataRegionValidatorImplTest.java:
##########
@@ -46,51 +50,120 @@
@ExtendWith(ConfigurationExtension.class)
public class PageMemoryDataRegionValidatorImplTest {
@InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
- private PageMemoryStorageEngineConfiguration engineConfig;
+ private VolatilePageMemoryStorageEngineConfiguration volatileEngineConfig;
+
+ @InjectConfiguration(polymorphicExtensions = UnsafeMemoryAllocatorConfigurationSchema.class)
+ private PersistentPageMemoryStorageEngineConfiguration persistentEngineConfig;
@Test
void testValidationFail() {
Review Comment:
I would suggest to split this test into 3, it's quite difficult to read otherwise
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -246,7 +244,7 @@ public void addCheckpointListener(CheckpointListener listener, @Nullable PageMem
* @param listener Listener.
*/
public void removeCheckpointListener(CheckpointListener listener) {
- listeners.remove(new IgniteBiTuple<CheckpointListener, PageMemoryDataRegion>() {
+ listeners.remove(new IgniteBiTuple<CheckpointListener, DataRegion>() {
Review Comment:
btw, this approach looks quite horrible to me, even though this is not related to this PR
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PageMemoryMvPartitionStorage.java:
##########
@@ -102,7 +102,7 @@ public PageMemoryMvPartitionStorage(
private VersionChainTree createVersionChainTree(
int partitionId,
TableView tableConfig,
- PageMemoryDataRegion dataRegion,
+ DataRegion dataRegion,
Review Comment:
```suggestion
DataRegion<?> dataRegion,
```
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/PersistentPageMemoryDataRegionConfigurationSchema.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.pagememory.configuration.schema;
+
+import static org.apache.ignite.internal.util.Constants.MiB;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.OneOf;
+
+/**
+ * Persistent data region configuration schema.
+ */
+@Config
+public class PersistentPageMemoryDataRegionConfigurationSchema extends BasePageMemoryDataRegionConfigurationSchema {
+ /** Default max size. */
+ public static final long DFLT_DATA_REGION_SIZE = 256 * MiB;
+
+ /** Random-LRU algorithm. */
Review Comment:
`algorithm for replacing pages`?
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/PersistentPageMemoryDataRegionConfigurationSchema.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.pagememory.configuration.schema;
+
+import static org.apache.ignite.internal.util.Constants.MiB;
+
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.validation.OneOf;
+
+/**
+ * Persistent data region configuration schema.
+ */
+@Config
+public class PersistentPageMemoryDataRegionConfigurationSchema extends BasePageMemoryDataRegionConfigurationSchema {
+ /** Default max size. */
+ public static final long DFLT_DATA_REGION_SIZE = 256 * MiB;
+
+ /** Random-LRU algorithm. */
+ public static final String RANDOM_LRU_REPLACEMENT_MODE = "RANDOM_LRU";
+
+ /** Segmented-LRU algorithm. */
+ public static final String SEGMENTED_LRU_REPLACEMENT_MODE = "SEGMENTED_LRU";
+
+ /** CLOCK algorithm. */
+ public static final String CLOCK_REPLACEMENT_MODE = "CLOCK";
+
+ /** Memory region size in bytes. */
+ @Value(hasDefault = true)
+ public long size = DFLT_DATA_REGION_SIZE;
+
+ /** Memory pages replacement mode. */
+ @OneOf({RANDOM_LRU_REPLACEMENT_MODE, SEGMENTED_LRU_REPLACEMENT_MODE, CLOCK_REPLACEMENT_MODE})
+ @Value(hasDefault = true)
+ public String replacementMode = CLOCK_REPLACEMENT_MODE;
+
+ /**
+ * Write to the page store without holding the segment lock (with a delay). Because other thread may require exactly the same page to be
Review Comment:
This javadoc doesn't make it any clearer as what this parameter actually does
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemoryNoLoadTest.java:
##########
@@ -56,13 +58,16 @@
import org.junit.jupiter.api.extension.ExtendWith;
/**
- * Tests {@link PageMemoryImpl}.
+ * Tests {@link PersistentPageMemory}.
*/
@ExtendWith(WorkDirectoryExtension.class)
-public class PageMemoryImplNoLoadTest extends PageMemoryNoLoadSelfTest {
+public class PersistentPageMemoryNoLoadTest extends VolatilePageMemoryNoLoadSelfTest {
Review Comment:
I think that this class should not extend `VolatilePageMemoryNoLoadSelfTest`. Instead, they both should extend a common superclass
--
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904868838
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -80,13 +80,13 @@ class CheckpointWorkflow implements IgniteComponent {
private final CheckpointReadWriteLock checkpointReadWriteLock;
/** Persistent data regions for the checkpointing. */
- private final Collection<? extends PageMemoryDataRegion> dataRegions;
+ private final Collection<? extends DataRegion<PersistentPageMemory>> dataRegions;
/** Checkpoint write order configuration. */
private final CheckpointWriteOrder checkpointWriteOrder;
/** Collections of checkpoint listeners. */
- private final List<IgniteBiTuple<CheckpointListener, PageMemoryDataRegion>> listeners = new CopyOnWriteArrayList<>();
+ private final List<IgniteBiTuple<CheckpointListener, DataRegion>> listeners = new CopyOnWriteArrayList<>();
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904864127
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PageMemoryMvPartitionStorage.java:
##########
@@ -102,7 +102,7 @@ public PageMemoryMvPartitionStorage(
private VersionChainTree createVersionChainTree(
int partitionId,
TableView tableConfig,
- PageMemoryDataRegion dataRegion,
+ DataRegion dataRegion,
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904865126
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PageMemoryMvPartitionStorage.java:
##########
@@ -78,7 +78,7 @@ public class PageMemoryMvPartitionStorage implements MvPartitionStorage {
public PageMemoryMvPartitionStorage(
int partitionId,
TableView tableConfig,
- PageMemoryDataRegion dataRegion,
+ DataRegion dataRegion,
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 #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #892:
URL: https://github.com/apache/ignite-3/pull/892#discussion_r904909291
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/validation/ValidationUtilTest.java:
##########
@@ -266,12 +268,81 @@ public void validate(NamedListValidation annotation, ValidationContext<NamedList
);
}
+ @Test
+ void testGetOwner() {
Review Comment:
What about the description?
```
/**
* Checks methods {@link ValidationContext#getOldOwner} and {@link ValidationContext#getNewOwner} that they return the correct owners
* when validated for leaves, nested configuration, and named list.
*/
```
--
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