You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ib...@apache.org on 2026/02/04 05:18:12 UTC

(ignite-3) branch main updated: IGNITE-24933 Remove late init for throttling policy (#7526)

This is an automated email from the ASF dual-hosted git repository.

ibessonov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git


The following commit(s) were added to refs/heads/main by this push:
     new 75f06185c3a IGNITE-24933 Remove late init for throttling policy (#7526)
75f06185c3a is described below

commit 75f06185c3aa5fd04973eb76595c486e690a7cb5
Author: Ivan Bessonov <be...@gmail.com>
AuthorDate: Wed Feb 4 08:18:05 2026 +0300

    IGNITE-24933 Remove late init for throttling policy (#7526)
---
 .../PersistentDataRegionConfiguration.java         | 20 +++++++++++--
 .../persistence/PersistentPageMemory.java          | 16 +++-------
 .../throttling/ThrottlingPolicyFactory.java        | 35 ++++++++++++++++++++++
 .../throttling/PageMemoryThrottlingTest.java       | 35 ++++++++++++----------
 .../org/apache/ignite/raft/ItRaftMetricTest.java   |  2 ++
 .../pagememory/PersistentPageMemoryDataRegion.java | 23 +++++++-------
 6 files changed, 88 insertions(+), 43 deletions(-)

diff --git a/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/PersistentDataRegionConfiguration.java b/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/PersistentDataRegionConfiguration.java
index fb8cc478a6d..b992b4961ab 100644
--- a/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/PersistentDataRegionConfiguration.java
+++ b/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/PersistentDataRegionConfiguration.java
@@ -17,12 +17,15 @@
 
 package org.apache.ignite.internal.pagememory.configuration;
 
+import org.apache.ignite.internal.pagememory.persistence.throttling.ThrottlingPolicyFactory;
+
 /** * Configuration for a persistent data region. */
 public class PersistentDataRegionConfiguration implements DataRegionConfiguration {
     private final String name;
     private final int pageSize;
     private final long size;
     private final ReplacementMode replacementMode;
+    private final ThrottlingPolicyFactory throttlingPolicyFactory;
 
     @Override
     public String name() {
@@ -43,11 +46,18 @@ public class PersistentDataRegionConfiguration implements DataRegionConfiguratio
         return replacementMode;
     }
 
-    private PersistentDataRegionConfiguration(String name, int pageSize, long size, ReplacementMode replacementMode) {
+    public ThrottlingPolicyFactory throttlingPolicyFactory() {
+        return throttlingPolicyFactory;
+    }
+
+    private PersistentDataRegionConfiguration(
+            String name, int pageSize, long size, ReplacementMode replacementMode, ThrottlingPolicyFactory throttlingPolicyFactory
+    ) {
         this.name = name;
         this.pageSize = pageSize;
         this.size = size;
         this.replacementMode = replacementMode;
+        this.throttlingPolicyFactory = throttlingPolicyFactory;
     }
 
     /** Creates a builder for {@link PersistentDataRegionConfiguration} instance. */
@@ -61,6 +71,7 @@ public class PersistentDataRegionConfiguration implements DataRegionConfiguratio
         private int pageSize;
         private long size;
         private ReplacementMode replacementMode = ReplacementMode.CLOCK;
+        private ThrottlingPolicyFactory throttlingPolicyFactory = pageMemory -> null;
 
         public PersistentDataRegionConfigurationBuilder name(String name) {
             this.name = name;
@@ -82,8 +93,13 @@ public class PersistentDataRegionConfiguration implements DataRegionConfiguratio
             return this;
         }
 
+        public PersistentDataRegionConfigurationBuilder throttlingPolicyFactory(ThrottlingPolicyFactory throttlingPolicyFactory) {
+            this.throttlingPolicyFactory = throttlingPolicyFactory;
+            return this;
+        }
+
         public PersistentDataRegionConfiguration build() {
-            return new PersistentDataRegionConfiguration(name, pageSize, size, replacementMode);
+            return new PersistentDataRegionConfiguration(name, pageSize, size, replacementMode, throttlingPolicyFactory);
         }
     }
 }
diff --git a/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemory.java b/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemory.java
index 13ad2db4923..af5e76ba087 100644
--- a/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemory.java
+++ b/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemory.java
@@ -199,7 +199,7 @@ public class PersistentPageMemory implements PageMemory {
     private volatile @Nullable PagePool checkpointPool;
 
     /** Pages write throttle. */
-    private volatile @Nullable PagesWriteThrottlePolicy writeThrottle;
+    private final @Nullable PagesWriteThrottlePolicy writeThrottle;
 
     /**
      * Delayed page replacement (rotation with disk) tracker. Because other thread may require exactly the same page to be loaded from
@@ -271,7 +271,9 @@ public class PersistentPageMemory implements PageMemory {
                 partitionDestructionLockManager
         );
 
-        this.writeThrottle = null;
+        // This is the last statement in the constructor, the `this` leak is fine.
+        //noinspection ThisEscapedInObjectConstruction
+        this.writeThrottle = dataRegionConfiguration.throttlingPolicyFactory().createThrottlingPolicy(this);
     }
 
     private static PageReplacementPolicyFactory pickPageReplacementPolicyFactory(ReplacementMode replacementMode) {
@@ -287,16 +289,6 @@ public class PersistentPageMemory implements PageMemory {
         }
     }
 
-    /**
-     * Temporary method to enable throttling in tests.
-     *
-     * @param writeThrottle Page write throttling instance.
-     */
-    // TODO IGNITE-24933 Remove this method.
-    public void initThrottling(PagesWriteThrottlePolicy writeThrottle) {
-        this.writeThrottle = writeThrottle;
-    }
-
     /** {@inheritDoc} */
     @Override
     public void start() throws IgniteInternalException {
diff --git a/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/throttling/ThrottlingPolicyFactory.java b/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/throttling/ThrottlingPolicyFactory.java
new file mode 100644
index 00000000000..eee1b7fd3a2
--- /dev/null
+++ b/modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/throttling/ThrottlingPolicyFactory.java
@@ -0,0 +1,35 @@
+/*
+ * 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.persistence.throttling;
+
+import org.apache.ignite.internal.pagememory.persistence.PersistentPageMemory;
+
+/**
+ * Factory interface to create instances of {@link PagesWriteThrottlePolicy}.
+ */
+@FunctionalInterface
+public interface ThrottlingPolicyFactory {
+    /**
+     * Creates an instance of a throttler. The cyclic dependency between throttlers and page memory is intentional here. Different
+     * implementations require an access to different parts of the data region, which would make the generalization of all these parts
+     * really awkward in this interface. We chose the lesser evil.
+     *
+     * @param pageMemory A page memory instance to throttle.
+     */
+    PagesWriteThrottlePolicy createThrottlingPolicy(PersistentPageMemory pageMemory);
+}
diff --git a/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/throttling/PageMemoryThrottlingTest.java b/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/throttling/PageMemoryThrottlingTest.java
index 07de030767e..ce28cc0a28c 100644
--- a/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/throttling/PageMemoryThrottlingTest.java
+++ b/modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/throttling/PageMemoryThrottlingTest.java
@@ -79,7 +79,6 @@ import org.apache.ignite.internal.util.OffheapReadWriteLock;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -133,8 +132,7 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
         ioRegistry = null;
     }
 
-    @BeforeEach
-    void setUp() throws Exception {
+    private void setUp(ThrottlingPolicyFactory throttleFactory) throws Exception {
         FailureManager failureManager = mock(FailureManager.class);
         when(failureManager.process(any())).thenThrow(new AssertionError("Unexpected error"));
 
@@ -161,7 +159,11 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
         );
 
         pageMemory = new PersistentPageMemory(
-                PersistentDataRegionConfiguration.builder().pageSize(PAGE_SIZE).size(SEGMENT_SIZE + CHECKPOINT_BUFFER_SIZE).build(),
+                PersistentDataRegionConfiguration.builder()
+                        .pageSize(PAGE_SIZE)
+                        .size(SEGMENT_SIZE + CHECKPOINT_BUFFER_SIZE)
+                        .throttlingPolicyFactory(throttleFactory)
+                        .build(),
                 new PersistentPageMemoryMetricSource("test"),
                 ioRegistry,
                 new long[]{SEGMENT_SIZE},
@@ -224,7 +226,7 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
     void pageAllocationNotifiedThrottler() throws Exception {
         PagesWriteThrottlePolicy writeThrottle = mock(PagesWriteThrottlePolicy.class);
 
-        pageMemory.initThrottling(writeThrottle);
+        setUp(pm -> writeThrottle);
 
         checkpointManager.checkpointTimeoutLock().checkpointReadLock();
 
@@ -244,7 +246,7 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
     void pageUnlockWithoutMarkingDirty() throws Exception {
         PagesWriteThrottlePolicy writeThrottle = mock(PagesWriteThrottlePolicy.class);
 
-        pageMemory.initThrottling(writeThrottle);
+        setUp(pm -> writeThrottle);
 
         long pageId;
 
@@ -276,10 +278,10 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
      * Tests that two consecutive page updates lead to a single {@link PagesWriteThrottlePolicy#onMarkDirty(boolean)} call.
      */
     @Test
-    void pageMarkedDirtyOnlyOnce() {
+    void pageMarkedDirtyOnlyOnce() throws Exception {
         PagesWriteThrottlePolicy writeThrottle = mock(PagesWriteThrottlePolicy.class);
 
-        pageMemory.initThrottling(writeThrottle);
+        setUp(pm -> writeThrottle);
 
         AtomicLong pageId = new AtomicLong();
 
@@ -306,10 +308,10 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
      * Tests that checkpoint events are properly propagated to the throttler.
      */
     @Test
-    void checkpointEvents() {
+    void checkpointEvents() throws Exception {
         PagesWriteThrottlePolicy writeThrottle = mock(PagesWriteThrottlePolicy.class);
 
-        pageMemory.initThrottling(writeThrottle);
+        setUp(pm -> writeThrottle);
 
         AtomicLong pageId = new AtomicLong();
 
@@ -334,7 +336,8 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
     @Test
     void wakeupThrottledThreads() throws Exception {
         PagesWriteThrottlePolicy writeThrottle = mock(PagesWriteThrottlePolicy.class);
-        pageMemory.initThrottling(writeThrottle);
+
+        setUp(pm -> writeThrottle);
 
         int pages = CHECKPOINT_BUFFER_SIZE / PAGE_SIZE * 9 / 10;
         long[] pageIds = new long[pages];
@@ -378,19 +381,19 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
      */
     @ParameterizedTest
     @ValueSource(booleans = {false, true})
-    void hugeLoadDoesNotBreakCheckpointReadLock(boolean speedBasedThrottling) {
+    void hugeLoadDoesNotBreakCheckpointReadLock(boolean speedBasedThrottling) throws Exception {
         PersistentPageMemoryMetricSource metricSource = new PersistentPageMemoryMetricSource("test");
 
-        PagesWriteThrottlePolicy writeThrottle;
+        ThrottlingPolicyFactory throttleFactory;
         if (speedBasedThrottling) {
-            writeThrottle = new PagesWriteSpeedBasedThrottle(
+            throttleFactory = pageMemory -> new PagesWriteSpeedBasedThrottle(
                     pageMemory,
                     checkpointManager::currentCheckpointProgress,
                     checkpointManager.checkpointTimeoutLock()::checkpointLockIsHeldByThread,
                     metricSource
             );
         } else {
-            writeThrottle = new TargetRatioPagesWriteThrottle(
+            throttleFactory = pageMemory -> new TargetRatioPagesWriteThrottle(
                     DEFAULT_LOGGING_THRESHOLD,
                     pageMemory,
                     checkpointManager::currentCheckpointProgress,
@@ -399,7 +402,7 @@ public class PageMemoryThrottlingTest extends IgniteAbstractTest {
             );
         }
 
-        pageMemory.initThrottling(writeThrottle);
+        setUp(throttleFactory);
 
         long[] pageIds = new long[SEGMENT_SIZE / PAGE_SIZE * 2];
 
diff --git a/modules/raft/src/integrationTest/java/org/apache/ignite/raft/ItRaftMetricTest.java b/modules/raft/src/integrationTest/java/org/apache/ignite/raft/ItRaftMetricTest.java
index ea10fd26436..1cc5dec07ad 100644
--- a/modules/raft/src/integrationTest/java/org/apache/ignite/raft/ItRaftMetricTest.java
+++ b/modules/raft/src/integrationTest/java/org/apache/ignite/raft/ItRaftMetricTest.java
@@ -29,6 +29,7 @@ import org.apache.ignite.InitParametersBuilder;
 import org.apache.ignite.internal.ClusterPerClassIntegrationTest;
 import org.apache.ignite.internal.metrics.sources.RaftMetricSource;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 /** Tests for {@link RaftMetricSource}. */
@@ -58,6 +59,7 @@ public class ItRaftMetricTest extends ClusterPerClassIntegrationTest {
     }
 
     @Test
+    @Disabled("https://issues.apache.org/jira/browse/IGNITE-27728")
     void testLeaderCountDecreases() {
         createZoneAndTable();
 
diff --git a/modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryDataRegion.java b/modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryDataRegion.java
index ecbe3d0922f..0d706b0ed7b 100644
--- a/modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryDataRegion.java
+++ b/modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryDataRegion.java
@@ -58,6 +58,7 @@ import org.apache.ignite.internal.pagememory.persistence.store.FilePageStoreMana
 import org.apache.ignite.internal.pagememory.persistence.throttling.PagesWriteSpeedBasedThrottle;
 import org.apache.ignite.internal.pagememory.persistence.throttling.PagesWriteThrottlePolicy;
 import org.apache.ignite.internal.pagememory.persistence.throttling.TargetRatioPagesWriteThrottle;
+import org.apache.ignite.internal.pagememory.persistence.throttling.ThrottlingPolicyFactory;
 import org.apache.ignite.internal.pagememory.persistence.throttling.ThrottlingType;
 import org.apache.ignite.internal.storage.StorageException;
 import org.apache.ignite.internal.storage.engine.StorageEngine;
@@ -190,8 +191,6 @@ public class PersistentPageMemoryDataRegion implements DataRegion<PersistentPage
                 checkpointManager.partitionDestructionLockManager()
         );
 
-        initThrottling(pageMemory);
-
         pageMemory.start();
 
         initMetrics();
@@ -204,7 +203,7 @@ public class PersistentPageMemoryDataRegion implements DataRegion<PersistentPage
         this.pageMemory = pageMemory;
     }
 
-    private static PersistentDataRegionConfiguration regionConfiguration(
+    private PersistentDataRegionConfiguration regionConfiguration(
             PersistentPageMemoryProfileView cfg,
             long sizeBytes,
             int pageSize
@@ -214,29 +213,28 @@ public class PersistentPageMemoryDataRegion implements DataRegion<PersistentPage
                 .pageSize(pageSize)
                 .size(sizeBytes)
                 .replacementMode(ReplacementMode.valueOf(cfg.replacementMode()))
+                .throttlingPolicyFactory(throttlingPolicyFactory())
                 .build();
     }
 
-    // TODO IGNITE-24933 refactor.
-    private void initThrottling(PersistentPageMemory pageMemory) {
+    private ThrottlingPolicyFactory throttlingPolicyFactory() {
         ThrottlingType throttlingType = getThrottlingType();
 
         switch (throttlingType) {
             case DISABLED:
-                break;
+                return pageMemory -> null;
 
             case TARGET_RATIO:
-                pageMemory.initThrottling(new TargetRatioPagesWriteThrottle(
+                return pageMemory -> new TargetRatioPagesWriteThrottle(
                         getLoggingThreshold(),
                         pageMemory,
                         checkpointManager::currentCheckpointProgressForThrottling,
                         checkpointManager.checkpointTimeoutLock()::checkpointLockIsHeldByThread,
                         metricSource
-                ));
-                break;
+                );
 
             case SPEED_BASED:
-                pageMemory.initThrottling(new PagesWriteSpeedBasedThrottle(
+                return pageMemory -> new PagesWriteSpeedBasedThrottle(
                         getLoggingThreshold(),
                         getMinDirtyPages(),
                         getMaxDirtyPages(),
@@ -244,11 +242,10 @@ public class PersistentPageMemoryDataRegion implements DataRegion<PersistentPage
                         checkpointManager::currentCheckpointProgressForThrottling,
                         checkpointManager.checkpointTimeoutLock()::checkpointLockIsHeldByThread,
                         metricSource
-                ));
-                break;
+                );
 
             default:
-                assert false : "Impossible throttling type: " + throttlingType;
+                throw new IllegalArgumentException("Impossible throttling type: " + throttlingType);
         }
     }