IGNITE-14385: Add checkpoit information to performance statistics. #8928

Sega76 · 2021-03-25T00:14:26Z

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

There is a single JIRA ticket related to the pull request.
The web-link to the pull request is attached to the JIRA ticket.
The JIRA ticket has the Patch Available state.
The pull request body describes changes that have been made.
The description explains WHAT and WHY was made instead of HOW.
The pull request title is treated as the final commit message.
The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
A reviewer has been mentioned through the JIRA comments
(see the Maintainers list)
The pull request has been checked by the Teamcity Bot and
the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

...in/java/org/apache/ignite/internal/processors/cache/persistence/checkpoint/Checkpointer.java

...pache/ignite/internal/processors/cache/persistence/pagemem/PagesWriteSpeedBasedThrottle.java

...src/main/java/org/apache/ignite/internal/processors/performancestatistics/OperationType.java

nizhikov · 2021-04-12T12:46:19Z

...apache/ignite/internal/processors/cache/persistence/pagemem/BPlusTreePageMemoryImplTest.java

@@ -120,7 +120,8 @@
                }
            },
            () -> true,
-            new DataRegionMetricsImpl(new DataRegionConfiguration(), cctx.metric(), NO_OP_METRICS),
+            new DataRegionMetricsImpl(new DataRegionConfiguration(), cctx.metric(), cctx.performanceStatistics(),


Should we add PerformanceStatistics instance to ctx on line 68?

...nite/internal/processors/cache/persistence/pagemem/BPlusTreeReuseListPageMemoryImplTest.java

.../internal/processors/cache/persistence/pagemem/IgnitePageMemReplaceDelayedWriteUnitTest.java

nizhikov · 2021-04-12T12:47:54Z

...che/ignite/internal/processors/cache/persistence/pagemem/IndexStoragePageMemoryImplTest.java

@@ -135,7 +135,8 @@
                }
            },
            () -> true,
-            new DataRegionMetricsImpl(new DataRegionConfiguration(), cctx.metric(), NO_OP_METRICS),
+            new DataRegionMetricsImpl(
+                new DataRegionConfiguration(), cctx.metric(), cctx.performanceStatistics(), NO_OP_METRICS),


Should we add PerformanceStatistics instance to ctx on line 68?

...rg/apache/ignite/internal/processors/cache/persistence/pagemem/PageMemoryImplNoLoadTest.java

.../apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsProcessor.java

nizhikov · 2021-04-13T06:05:38Z

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java

+
+        MetricRegistry mreg = srv.context().metric().registry(DATASTORAGE_METRIC_PREFIX);
+
+        AtomicLongMetric lastBeforeLockDuration = mreg.findMetric("LastCheckpointBeforeLockDuration");


Let's inline all these variables.

nizhikov · 2021-04-13T06:05:54Z

...ache/ignite/internal/processors/performancestatistics/AbstractPerformanceStatisticsTest.java

@@ -81,6 +82,18 @@ public static void startCollectStatistics() throws Exception {
        waitForStatisticsEnabled(true);
    }

+    /** Starts collecting performance statistics with immediately flush. */
+    protected static void startCollectStatisticsWithImmediatelyFlush() throws Exception {


We should get rid of this method.

nizhikov · 2021-04-13T06:07:01Z

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java

+
+        forceCheckpoint();
+
+        assertTrue(waitForCondition(() -> {


Let's rewrite test as follows:

Start psproc

forceCheckpoint

Wait for checkpoint finish (based on metric value or similar)

Stop psproc.

Check is there checkpoint data in statistics (no multiple read of statistics).

nizhikov · 2021-04-13T06:07:13Z

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java

+                AtomicInteger cnt = new AtomicInteger();
+
+                readFiles(statisticsFiles(), new TestHandler() {
+                    @Override public void checkpoint(UUID nodeId, long beforeLockDuration, long lockWaitDuration,


Please, fix the code formatting.

nizhikov · 2021-04-13T06:07:26Z

...ache/ignite/internal/processors/performancestatistics/AbstractPerformanceStatisticsTest.java

@@ -197,6 +210,20 @@ protected static PerformanceStatisticsMBean statisticsMBean(String igniteInstanc
            boolean timedOut) {
            // No-op.
        }
+
+        /** {@inheritDoc} */
+        @Override public void checkpoint(UUID nodeId, long beforeLockDuration, long lockWaitDuration,


Please, fix code formatting.

nizhikov · 2021-04-13T06:07:48Z

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java

+
+        startCollectStatisticsWithImmediatelyFlush();
+
+        int keysCnt = 1024;


This variables can be inlined.

nizhikov · 2021-04-13T06:09:33Z

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java

+        private static final long serialVersionUID = 0L;
+
+        /** Default checkpoint park nanos. */
+        private static final int CHECKPOINT_PARK_NANOS = 5_000_000;


This should be long.

nizhikov · 2021-04-13T06:13:38Z

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java

+    /**
+     * Create File I/O that emulates poor checkpoint write speed.
+     */
+    private static class SlowCheckpointFileIOFactory implements FileIOFactory {


Let's reuse the same class from PagesWriteThrottleSmokeTest or CheckpointBufferDeadlockTest instead of copy-pasting it.

nizhikov · 2021-04-13T08:28:54Z

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java

+
+        AtomicLongMetric lastStart = mreg.findMetric("LastCheckpointStart");
+
+        // wait for checkpoint to finish on node start


Capital letter on the start and dot in the end.

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java

…s. (apache#8928)

Sega76 added 2 commits March 25, 2021 03:12

IGNITE-14385: init

6994522

IGNITE-14385: added tests

28a08de

Sega76 force-pushed the IGNITE-14385 branch from c9a0fa7 to e9f9720 Compare March 29, 2021 00:12

Sega76 changed the title ~~IGNITE-14385: Add start/end time of checkpoint, rebalance, PME to performance statistics.~~ IGNITE-14385: Add checkpoit information to performance statistics. Mar 29, 2021

IGNITE-14385: added tests

bb0a0d4

Sega76 force-pushed the IGNITE-14385 branch from e9f9720 to bb0a0d4 Compare March 29, 2021 07:24

IGNITE-14385: update checkpoint info

013ef58

Sega76 force-pushed the IGNITE-14385 branch from c1fc9f9 to 013ef58 Compare March 30, 2021 08:14

Sega76 added 7 commits March 30, 2021 20:08

IGNITE-14385: update checkpoint info

f58fbe1

Merge remote-tracking branch 'ignite/master' into IGNITE-14385

80f9d0d

IGNITE-14385: update checkpoint test

3424575

IGNITE-14385: update checkpoint test

a436a12

IGNITE-14385: added throttling

56dc109

IGNITE-14385: fix throttling test

a2c56f0

IGNITE-14385: fix throttling test

8f2ec9c

Sega76 force-pushed the IGNITE-14385 branch from 1534566 to 1dd6bb6 Compare April 2, 2021 09:47

IGNITE-14385: fix tests

c5db86d

Sega76 force-pushed the IGNITE-14385 branch from 1dd6bb6 to c5db86d Compare April 2, 2021 09:50

IGNITE-14385: fix tests

b6471e9

NSAmelchev reviewed Apr 5, 2021

View reviewed changes

...src/main/java/org/apache/ignite/internal/processors/performancestatistics/OperationType.java Outdated Show resolved Hide resolved

Sega76 added 8 commits April 6, 2021 10:37

IGNITE-14385: fix comments

ef80b08

IGNITE-14385: fix comments

4bace44

IGNITE-14385: fix comments

1813a76

IGNITE-14385: fix comments

b40bdd5

IGNITE-14385: fix comments

6c9cda7

IGNITE-14385: fix comments

fd00075

IGNITE-14385: fix comments

c98d23b

IGNITE-14385: fix tests

73b9b8b

Sega76 force-pushed the IGNITE-14385 branch from f0aedb7 to 73b9b8b Compare April 7, 2021 13:21

nizhikov reviewed Apr 12, 2021

View reviewed changes

...nite/internal/processors/cache/persistence/pagemem/BPlusTreeReuseListPageMemoryImplTest.java Outdated Show resolved Hide resolved

nizhikov reviewed Apr 12, 2021

View reviewed changes

.../internal/processors/cache/persistence/pagemem/IgnitePageMemReplaceDelayedWriteUnitTest.java Outdated Show resolved Hide resolved

nizhikov reviewed Apr 12, 2021

View reviewed changes

...rg/apache/ignite/internal/processors/cache/persistence/pagemem/PageMemoryImplNoLoadTest.java Outdated Show resolved Hide resolved

IGNITE-14385: fix comments

fb7ad5a

nizhikov reviewed Apr 12, 2021

View reviewed changes

.../apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsProcessor.java Show resolved Hide resolved

Sega76 added 2 commits April 12, 2021 20:04

IGNITE-14385: fix comments

790ee65

IGNITE-14385: fix comments

12b8abd

nizhikov reviewed Apr 13, 2021

View reviewed changes

Sega76 added 2 commits April 13, 2021 11:13

IGNITE-14385: fix comments

8924dd1

IGNITE-14385: fix comments

65ed6eb

nizhikov reviewed Apr 13, 2021

View reviewed changes

Sega76 added 3 commits April 13, 2021 11:38

IGNITE-14385: fix comments

a828bd5

IGNITE-14385: fix comments

046ab81

IGNITE-14385: fix comments

d7b046b

nizhikov approved these changes Apr 13, 2021

View reviewed changes

IGNITE-14385: fix comments

458463d

NSAmelchev reviewed Apr 14, 2021

View reviewed changes

...rc/test/java/org/apache/ignite/internal/processors/performancestatistics/CheckpointTest.java Show resolved Hide resolved

IGNITE-14385: writing statistics before updating metrics

88465d3

NSAmelchev approved these changes Apr 15, 2021

View reviewed changes

NSAmelchev merged commit 118c64e into apache:master Apr 15, 2021

xintrian pushed a commit to xintrian/ignite that referenced this pull request May 27, 2022

IGNITE-14385: Add checkpoint information to the performance statistic…

a8df256

…s. (apache#8928)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IGNITE-14385: Add checkpoit information to performance statistics. #8928

IGNITE-14385: Add checkpoit information to performance statistics. #8928

Sega76 commented Mar 25, 2021 •

edited

nizhikov Apr 12, 2021

Sega76 Apr 12, 2021

nizhikov Apr 12, 2021

Sega76 Apr 12, 2021

nizhikov Apr 13, 2021

nizhikov Apr 13, 2021

nizhikov Apr 13, 2021

nizhikov Apr 13, 2021

Sega76 Apr 13, 2021

nizhikov Apr 13, 2021

Sega76 Apr 13, 2021

nizhikov Apr 13, 2021

Sega76 Apr 13, 2021

nizhikov Apr 13, 2021

Sega76 Apr 13, 2021

nizhikov Apr 13, 2021

Sega76 Apr 13, 2021

nizhikov Apr 13, 2021

Sega76 Apr 13, 2021


		MetricRegistry mreg = srv.context().metric().registry(DATASTORAGE_METRIC_PREFIX);

		AtomicLongMetric lastBeforeLockDuration = mreg.findMetric("LastCheckpointBeforeLockDuration");


		startCollectStatisticsWithImmediatelyFlush();

		int keysCnt = 1024;


		AtomicLongMetric lastStart = mreg.findMetric("LastCheckpointStart");

		// wait for checkpoint to finish on node start

IGNITE-14385: Add checkpoit information to performance statistics. #8928

IGNITE-14385: Add checkpoit information to performance statistics. #8928

Conversation

Sega76 commented Mar 25, 2021 • edited

The Contribution Checklist

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sega76 commented Mar 25, 2021 •

edited