tkalkirill commented on code in PR #4462:
URL: https://github.com/apache/ignite-3/pull/4462#discussion_r1777976397
##########
modules/page-memory/build.gradle:
##########
@@ -59,6 +59,7 @@ dependencies {
integrationTestImplementation project(':ignite-storage-api')
integrationTestImplementation project(':ignite-failure-handler')
integrationTestImplementation project(':ignite-file-io')
+ integrationTestImplementation project(':ignite-storage-page-memory')
Review Comment:
Module `ignite-storage-page-memory` has a dependency on `ignite-page-memory`.
I think adding such a dependency leads to a cyclic dependency, which is not
good.
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/ReplaceCandidate.java:
##########
@@ -29,13 +29,13 @@
*/
public class ReplaceCandidate {
/** Partition generation saved in map, too old value means page may be
safely cleared. */
- private int gen;
+ private final int gen;
Review Comment:
Why remove final?
##########
modules/page-memory/src/integrationTest/java/org/apache/ignite/internal/pagememory/tree/persistence/replacement/AbstractPageReplacementTest.java:
##########
@@ -68,18 +68,17 @@
import org.apache.ignite.internal.pagememory.persistence.store.FilePageStore;
import
org.apache.ignite.internal.pagememory.persistence.store.FilePageStoreManager;
import
org.apache.ignite.internal.storage.configurations.StorageProfileConfiguration;
-import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
-import org.apache.ignite.internal.testframework.WorkDirectory;
-import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import
org.apache.ignite.internal.storage.pagememory.PersistentPageMemoryStorageEngine;
+import org.apache.ignite.internal.testframework.IgniteAbstractTest;
import org.apache.ignite.internal.util.IgniteUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
/** Integration tests for testing page replacement. */
-@ExtendWith({WorkDirectoryExtension.class, ConfigurationExtension.class})
-public class ItPageReplacementTest extends BaseIgniteAbstractTest {
+@ExtendWith(ConfigurationExtension.class)
+public abstract class AbstractPageReplacementTest extends IgniteAbstractTest {
Review Comment:
Please add documentation to the class, I remember in a personal
correspondence you suggested moving it to unit tests, since I was wrong and it
doesn’t start 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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
