Phillippko commented on code in PR #4372:
URL: https://github.com/apache/ignite-3/pull/4372#discussion_r1754514175
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/compaction/Compactor.java:
##########
@@ -280,9 +287,19 @@ void doCompaction() {
// Wait and check for errors.
CompletableFuture.allOf(futures).join();
- // TODO Expand the comment.
https://issues.apache.org/jira/browse/IGNITE-23056, handle an exception too.
Review Comment:
"handle an exception too"
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMetricsTracker.java:
##########
@@ -191,78 +195,78 @@ public void onSplitAndSortCheckpointPagesStart() {
* <p>Not thread safe.
*/
public void onSplitAndSortCheckpointPagesEnd() {
- splitAndSortCheckpointPagesEndTimestamp = coarseCurrentTimeMillis();
+ splitAndSortPagesEndNanos = System.nanoTime();
}
/**
* Returns total checkpoint duration.
*
* <p>Not thread safe.
*/
- public long totalDuration() {
- return checkpointEndTimestamp - checkpointStartTimestamp;
+ public long totalDuration(TimeUnit timeUnit) {
+ return timeUnit.convert(endNanos - startNanos, NANOSECONDS);
}
/**
* Returns checkpoint write lock wait duration in mills.
*
* <p>Not thread safe.
*/
- public long writeLockWaitDuration() {
- return checkpointOnMarkCheckpointBeginStartTimestamp -
checkpointWriteLockWaitStartTimestamp;
+ public long writeLockWaitDuration(TimeUnit timeUnit) {
+ return timeUnit.convert(onMarkCheckpointBeginStartNanos -
writeLockWaitStartNanos, NANOSECONDS);
}
/**
* Returns checkpoint action before taken write lock duration in mills.
*
* <p>Not thread safe.
*/
- public long beforeWriteLockDuration() {
- return checkpointWriteLockWaitStartTimestamp -
checkpointStartTimestamp;
+ public long beforeWriteLockDuration(TimeUnit timeUnit) {
+ return timeUnit.convert(writeLockWaitStartNanos - startNanos,
NANOSECONDS);
}
/**
* Returns execution all {@link CheckpointListener#onMarkCheckpointBegin}
under write lock duration in mills.
*
* <p>Not thread safe.
*/
- public long onMarkCheckpointBeginDuration() {
- return checkpointOnMarkCheckpointBeginEndTimestamp -
checkpointOnMarkCheckpointBeginStartTimestamp;
+ public long onMarkCheckpointBeginDuration(TimeUnit timeUnit) {
+ return timeUnit.convert(onMarkCheckpointBeginEndNanos -
onMarkCheckpointBeginStartNanos, NANOSECONDS);
}
/**
* Returns checkpoint write lock hold duration in mills.
*
* <p>Not thread safe.
*/
- public long writeLockHoldDuration() {
- return checkpointWriteLockReleaseTimestamp -
checkpointOnMarkCheckpointBeginStartTimestamp;
+ public long writeLockHoldDuration(TimeUnit timeUnit) {
+ return timeUnit.convert(writeLockReleaseNanos -
onMarkCheckpointBeginStartNanos, NANOSECONDS);
}
/**
* Returns pages write duration in mills.
*
* <p>Not thread safe.
*/
- public long pagesWriteDuration() {
- return checkpointFsyncStartTimestamp -
checkpointPagesWriteStartTimestamp;
+ public long pagesWriteDuration(TimeUnit timeUnit) {
+ return timeUnit.convert(fsyncStartNanos - pagesWriteStartNanos,
NANOSECONDS);
}
/**
* Returns checkpoint fsync duration in mills.
*
* <p>Not thread safe.
*/
- public long fsyncDuration() {
- return checkpointEndTimestamp - checkpointFsyncStartTimestamp;
+ public long fsyncDuration(TimeUnit timeUnit) {
+ return timeUnit.convert(endNanos - fsyncStartNanos, NANOSECONDS);
}
/**
* Returns duration of splitting and sorting checkpoint pages in mills.
Review Comment:
```suggestion
* Returns duration of splitting and sorting checkpoint pages in the
given time unit.
```
(the same for other methods)
##########
modules/page-memory/src/test/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointMetricsTrackerTest.java:
##########
@@ -17,16 +17,14 @@
package org.apache.ignite.internal.pagememory.persistence.checkpoint;
Review Comment:
Noticed that we have tests for checkpoint metrics tracker, but not for
compaction. Not sure if they're really needed though
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/compaction/Compactor.java:
##########
@@ -280,9 +287,19 @@ void doCompaction() {
// Wait and check for errors.
CompletableFuture.allOf(futures).join();
Review Comment:
Is it OKay that join is used?
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/PersistentPageMemoryMvPartitionStorageTest.java:
##########
@@ -54,7 +54,7 @@ class PersistentPageMemoryMvPartitionStorageTest extends
AbstractPageMemoryMvPar
@InjectConfiguration("mock.checkpoint.checkpointDelayMillis = 0")
private PersistentPageMemoryStorageEngineConfiguration engineConfig;
- @InjectConfiguration("mock.profiles.default = {engine = \"aipersist\"}")
+ @InjectConfiguration("mock.profiles.default = {engine = aipersist, size="
+ (256 * IgniteUtils.MB) + "}")
Review Comment:
Let's use static import
And why adding this setting?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
