You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/08/09 08:46:02 UTC
[GitHub] [ignite-3] ibessonov opened a new pull request, #989: IGNITE-17077 persistedIndex implementation for PDS
ibessonov opened a new pull request, #989:
URL: https://github.com/apache/ignite-3/pull/989
https://issues.apache.org/jira/browse/IGNITE-17077
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #989: IGNITE-17077 persistedIndex implementation for PDS
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #989:
URL: https://github.com/apache/ignite-3/pull/989#discussion_r941221260
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/PageMemoryCheckpointConfigurationSchema.java:
##########
@@ -36,6 +36,11 @@ public class PageMemoryCheckpointConfigurationSchema {
@Value(hasDefault = true)
public int frequencyDeviation = 40;
+ /** Delay before executing a checkpoint triggered by RAFT. */
+ @Range(min = 0)
+ @Value(hasDefault = true)
+ public int cpDelayMillis = 200;
Review Comment:
I would prefer to name it `checkpointDelayMillis`
##########
modules/storage-api/src/test/java/org/apache/ignite/internal/storage/AbstractMvPartitionStorageTest.java:
##########
@@ -679,4 +682,25 @@ void scanWithTxIdThrowsWhenOtherTransactionHasUncommittedChanges() {
assertThrows(TxIdMismatchException.class, cursor::next);
}
+
+ @Test
+ void testAppliedIndex() {
+ storage.runConsistently(() -> {
+ assertEquals(0, storage.lastAppliedIndex());
+ assertEquals(0, storage.persistedIndex());
+
+ storage.lastAppliedIndex(1);
+
+ assertEquals(1, storage.lastAppliedIndex());
+ assertThat(storage.persistedIndex(), is(lessThanOrEqualTo(1L)));
+
+ return null;
+ });
+
+ CompletableFuture<Void> flushFuture = storage.flush();
+
+ assertThat(flushFuture, willBe(equalTo(null)));
Review Comment:
there's a `willCompleteSuccessfully` matcher
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PersistentPageMemoryMvPartitionStorage.java:
##########
@@ -66,4 +109,53 @@ public <V> V runConsistently(WriteClosure<V> closure) throws StorageException {
checkpointTimeoutLock.checkpointReadUnlock();
}
}
+
+ /** {@inheritDoc} */
+ @Override
+ public CompletableFuture<Void> flush() {
+ CheckpointProgress lastCheckpoint = checkpointManager.lastCheckpointProgress();
+
+ CheckpointProgress scheduledCheckpoint;
+
+ if (lastCheckpoint != null && meta.metaSnapshot(lastCheckpoint.id()).lastAppliedIndex() >= meta.lastAppliedIndex()) {
+ scheduledCheckpoint = lastCheckpoint;
+ } else {
+ PersistentPageMemoryStorageEngineView engineCfg = tableStorage.engine().configuration().value();
+
+ int cpDelayMillis = engineCfg.checkpoint().cpDelayMillis();
+ scheduledCheckpoint = checkpointManager.scheduleCheckpoint(cpDelayMillis, "Triggered by replicator.");
Review Comment:
```suggestion
scheduledCheckpoint = checkpointManager.scheduleCheckpoint(cpDelayMillis, "Triggered by replicator");
```
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PersistentPageMemoryMvPartitionStorage.java:
##########
@@ -66,4 +109,53 @@ public <V> V runConsistently(WriteClosure<V> closure) throws StorageException {
checkpointTimeoutLock.checkpointReadUnlock();
}
}
+
+ /** {@inheritDoc} */
+ @Override
+ public CompletableFuture<Void> flush() {
+ CheckpointProgress lastCheckpoint = checkpointManager.lastCheckpointProgress();
+
+ CheckpointProgress scheduledCheckpoint;
+
+ if (lastCheckpoint != null && meta.metaSnapshot(lastCheckpoint.id()).lastAppliedIndex() >= meta.lastAppliedIndex()) {
Review Comment:
how can lastAppliedIndex of a snapshot be larger then the current value?
##########
modules/storage-api/src/test/java/org/apache/ignite/internal/storage/AbstractMvPartitionStorageTest.java:
##########
@@ -679,4 +682,25 @@ void scanWithTxIdThrowsWhenOtherTransactionHasUncommittedChanges() {
assertThrows(TxIdMismatchException.class, cursor::next);
}
+
+ @Test
+ void testAppliedIndex() {
Review Comment:
Please leave a comment about what this test actually tests and what invariants it checks
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/PersistentPageMemoryMvPartitionStorageTest.java:
##########
@@ -73,6 +73,8 @@ void setUp() throws Exception {
longJvmPauseDetector.start();
+ engineConfig.checkpoint().cpDelayMillis().update(0).get(1, TimeUnit.SECONDS);
Review Comment:
can it be specified as a string in `InjectConfiguration`?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #989: IGNITE-17077 persistedIndex implementation for PDS
Posted by GitBox <gi...@apache.org>.
ibessonov commented on code in PR #989:
URL: https://github.com/apache/ignite-3/pull/989#discussion_r941236097
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/PersistentPageMemoryMvPartitionStorage.java:
##########
@@ -66,4 +109,53 @@ public <V> V runConsistently(WriteClosure<V> closure) throws StorageException {
checkpointTimeoutLock.checkpointReadUnlock();
}
}
+
+ /** {@inheritDoc} */
+ @Override
+ public CompletableFuture<Void> flush() {
+ CheckpointProgress lastCheckpoint = checkpointManager.lastCheckpointProgress();
+
+ CheckpointProgress scheduledCheckpoint;
+
+ if (lastCheckpoint != null && meta.metaSnapshot(lastCheckpoint.id()).lastAppliedIndex() >= meta.lastAppliedIndex()) {
Review Comment:
Ok, I'll replace that with ==
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #989: IGNITE-17077 persistedIndex implementation for PDS
Posted by GitBox <gi...@apache.org>.
ibessonov commented on code in PR #989:
URL: https://github.com/apache/ignite-3/pull/989#discussion_r941236705
##########
modules/storage-page-memory/src/test/java/org/apache/ignite/internal/storage/pagememory/mv/PersistentPageMemoryMvPartitionStorageTest.java:
##########
@@ -73,6 +73,8 @@ void setUp() throws Exception {
longJvmPauseDetector.start();
+ engineConfig.checkpoint().cpDelayMillis().update(0).get(1, TimeUnit.SECONDS);
Review Comment:
Yes it can
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov merged pull request #989: IGNITE-17077 persistedIndex implementation for PDS
Posted by GitBox <gi...@apache.org>.
ibessonov merged PR #989:
URL: https://github.com/apache/ignite-3/pull/989
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org