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/23 10:22:07 UTC
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #892: IGNITE-17149 Separation of the PageMemoryStorageEngineConfigurationSchema into in-memory and persistent
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