tkalkirill commented on code in PR #5392: URL: https://github.com/apache/ignite-3/pull/5392#discussion_r1989523042
########## modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemoryMetricSource.java: ########## @@ -0,0 +1,78 @@ +/* + * 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; + +import java.util.HashMap; +import java.util.Map; +import org.apache.ignite.internal.metrics.Metric; +import org.apache.ignite.internal.metrics.MetricSet; +import org.apache.ignite.internal.metrics.MetricSource; +import org.jetbrains.annotations.Nullable; + +/** + * Metric source for persistent page memory. + */ +public class PersistentPageMemoryMetricSource implements MetricSource { Review Comment: I would ask you to indicate in the documentation for the fields that they are guarded by the monitor, otherwise I immediately notice bugs. ########## modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemory.java: ########## @@ -275,6 +283,15 @@ public PersistentPageMemory( this.writeThrottle = null; } + private void initMetrics() { + metricSource.addMetric(new IntGauge( Review Comment: Do you think we should document metrics somewhere? ########## modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/throttling/PagesWriteSpeedBasedThrottle.java: ########## @@ -78,32 +83,101 @@ public class PagesWriteSpeedBasedThrottle implements PagesWriteThrottlePolicy { /** Checkpoint Buffer-related logic used to keep it safe. */ private final CheckpointBufferOverflowWatchdog cpBufferWatchdog; + private final LongAdderMetric totalThrottlingTime = new LongAdderMetric( + "TotalThrottlingTime", + "Total throttling threads time in milliseconds. The Ignite throttles threads that generate " + + "dirty pages during the ongoing checkpoint." + ); + /** * Constructor. * * @param pageMemory Page memory. * @param cpProgress Database manager. * @param stateChecker Checkpoint lock state provider. + * @param metricSource Metric source. */ public PagesWriteSpeedBasedThrottle( PersistentPageMemory pageMemory, Supplier<CheckpointProgress> cpProgress, - CheckpointLockStateChecker stateChecker + CheckpointLockStateChecker stateChecker, + PersistentPageMemoryMetricSource metricSource ) { this.pageMemory = pageMemory; this.cpProgress = cpProgress; cpLockStateChecker = stateChecker; - cleanPagesProtector = new SpeedBasedMemoryConsumptionThrottlingStrategy(pageMemory, cpProgress, - markSpeedAndAvgParkTime); + cleanPagesProtector = new SpeedBasedMemoryConsumptionThrottlingStrategy(pageMemory, cpProgress, markSpeedAndAvgParkTime); cpBufferWatchdog = new CheckpointBufferOverflowWatchdog(pageMemory); + + initMetrics(metricSource); + } + + private void initMetrics(PersistentPageMemoryMetricSource metricSource) { + metricSource.addMetric(totalThrottlingTime); + + metricSource.addMetric(new DoubleGauge( + "SpeedBasedThrottlingPercentage", + "Measurement shows how much throttling time is involved into average marking time.", + this::throttleWeight + )); + metricSource.addMetric(new LongGauge( + "MarkDirtySpeed", + "Speed of marking pages dirty. Value from past 750-1000 millis only. Pages/second.", + this::getMarkDirtySpeed + )); + metricSource.addMetric(new LongGauge( + "CpWriteSpeed", + "Speed average checkpoint write speed. Current and 3 past checkpoints used. Pages/second.", + this::getCpWriteSpeed + )); + metricSource.addMetric(new LongGauge( + "LastEstimatedSpeedForMarkAll", + "Last estimated speed for marking all clear pages as dirty till the end of checkpoint.", + this::getLastEstimatedSpeedForMarkAll + )); + metricSource.addMetric(new DoubleGauge( + "CurrDirtyRatio", + "Current dirty pages ratio.", + this::getCurrDirtyRatio + )); + metricSource.addMetric(new DoubleGauge( + "TargetDirtyRatio", + "Target (maximum) dirty pages ratio, after which throttling will start.", + this::getTargetDirtyRatio + )); + metricSource.addMetric(new LongGauge( + "ThrottleParkTime", + "Exponential backoff counter.", + this::throttleParkTime + )); + metricSource.addMetric(new IntGauge( + "CpTotalPages", + "Number of pages in current checkpoint.", + cleanPagesProtector::cpTotalPages + )); + metricSource.addMetric(new IntGauge( + "CpEvictedPages", + "Number of evicted pages.", + cleanPagesProtector::cpEvictedPages + )); + metricSource.addMetric(new IntGauge( + "CpWrittenPages", + "Number of written pages.", + this::cpWrittenPages + )); + metricSource.addMetric(new IntGauge( + "CpSyncedPages", + "Counter for fsynced checkpoint pages.", + cleanPagesProtector::cpSyncedPages + )); } @Override public void onMarkDirty(boolean isPageInCheckpoint) { assert cpLockStateChecker.checkpointLockIsHeldByThread(); - long curNanoTime = System.nanoTime(); - long throttleParkTimeNs = computeThrottlingParkTime(isPageInCheckpoint, curNanoTime); + long startTime = System.nanoTime(); Review Comment: Maybe add the postfix "Ns"? ########## modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemory.java: ########## @@ -275,6 +283,15 @@ public PersistentPageMemory( this.writeThrottle = null; } + private void initMetrics() { + metricSource.addMetric(new IntGauge( + "CheckpointBufferPagesCount", "Number of pages used in checkpoint buffer.", this::usedCheckpointBufferPages + )); + metricSource.addMetric(new IntGauge( + "CheckpointBufferPagesSize", "Number of used pages in checkpoint buffer.", this::maxCheckpointBufferPages Review Comment: Maybe my English is not very good, but I did not really understand how this description is fundamentally different from the description above. If everything is ok, then close my comment. ########## modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/throttling/TargetRatioPagesWriteThrottle.java: ########## @@ -125,6 +137,8 @@ public TargetRatioPagesWriteThrottle( + " for timeout(ms)=" + TimeUnit.NANOSECONDS.toMillis(throttleParkTimeNs)); } + long startTime = System.nanoTime(); Review Comment: Maybe add the postfix "Ns"? ########## modules/table/src/test/java/org/apache/ignite/internal/table/distributed/TableManagerRecoveryTest.java: ########## @@ -545,7 +548,7 @@ public StorageEngine createEngine( ) throws StorageException { return spy(super.createEngine( igniteInstanceName, - configRegistry, + metricManager, configRegistry, Review Comment: ```suggestion metricManager, configRegistry, ``` -- 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
