[GitHub] [ignite] alex-plekhanov commented on a change in pull request #8072: IGNITE-13289 Fix failure of PagesWriteThrottleSmokeTest.testThrottle

[GitBox <gi...@apache.org>]

alex-plekhanov commented on a change in pull request #8072:
URL: https://github.com/apache/ignite/pull/8072#discussion_r459512951



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
##########
@@ -4270,6 +4270,8 @@ private long updateLastCheckpointTime() {
          */
         private void markCheckpointEnd(Checkpoint chp) throws IgniteCheckedException {
             synchronized (this) {
+                getCheckpointer().currentProgress().clearCounters();

Review comment:
       Perhaps it's better to use `chp.progress` here, since it's for sure related to the finished checkpoint.
   Currently, there can be only one checkpoint at a time and it's not an issue, but there was a plans to merge checkpoints (see https://issues.apache.org/jira/browse/IGNITE-10508)

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/checkpoint/CheckpointProgressImpl.java
##########
@@ -272,6 +272,15 @@ public void nextSnapshot(boolean nextSnapshot) {
         evictedPagesCntr = new AtomicInteger();
     }
 
+    /** {@inheritDoc} */
+    @Override public void clearCounters() {
+        currCheckpointPagesCnt = 0;

Review comment:
       Perhaps it's better to set `currCheckpointPagesCnt` after all other counters, to keep invariant `total pages` >= `written pages` for example. But it's not an issue and it's up to you.

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/checkpoint/CheckpointProgress.java
##########
@@ -109,6 +109,9 @@
      */
     public void updateEvictedPages(int deltha);
 
+    /** Clear cp progress counters */

Review comment:
       Point at the end of line




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org