[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #901: IGNITE-17230 Support splt-file page store

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

SammyVimes commented on code in PR #901:
URL: https://github.com/apache/ignite-3/pull/901#discussion_r932265975


##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointWorkflow.java:
##########
@@ -219,15 +226,17 @@ public Checkpoint markCheckpointBegin(
      */
     public void markCheckpointEnd(Checkpoint chp) throws IgniteInternalCheckedException {
         synchronized (this) {
-            chp.progress.clearCounters();
-
             for (DataRegion<PersistentPageMemory> dataRegion : dataRegions) {
                 dataRegion.pageMemory().finishCheckpoint();
             }
         }
 
         if (chp.hasDelta()) {
             checkpointMarkersStorage.onCheckpointEnd(chp.progress.id());
+
+            chp.progress.pagesToWrite(null);

Review Comment:
   Do I understand correctly that if `hasDelta == false`, then dirty pages will be null, right? So we don't need to set null if hasDelta == false



##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointManager.java:
##########
@@ -221,4 +233,56 @@ static boolean safeToUpdateAllPageMemories(Collection<? extends DataRegion<Persi
 
         return true;
     }
+
+    /**
+     * Writes a page to delta file page store.
+     *
+     * <p>Must be used at breakpoint and page replacement.
+     *
+     * @param pageMemory Page memory.
+     * @param pageId Page ID.
+     * @param pageBuf Page buffer to write from.
+     * @param calculateCrc If {@code false} crc calculation will be forcibly skipped.
+     * @throws IgniteInternalCheckedException If page writing failed (IO error occurred).
+     */
+    public void writePageToDeltaFilePageStore(
+            PersistentPageMemory pageMemory,
+            FullPageId pageId,
+            ByteBuffer pageBuf,
+            boolean calculateCrc
+    ) throws IgniteInternalCheckedException {
+        FilePageStore filePageStore = filePageStoreManager.getStore(pageId.groupId(), pageId.partitionId());
+
+        CheckpointProgress lastCheckpointProgress = lastCheckpointProgress();
+
+        assert lastCheckpointProgress != null : "Checkpoint has not happened yet";
+        assert lastCheckpointProgress.inProgress() : "Checkpoint must be in progress";
+
+        CheckpointDirtyPages pagesToWrite = lastCheckpointProgress.pagesToWrite();
+
+        assert pagesToWrite != null : "Dirty pages must be sorted out";
+
+        CompletableFuture<DeltaFilePageStoreIo> deltaFilePageStoreFuture = filePageStore.getOrCreateNewDeltaFile(

Review Comment:
   Do we need a future  here? I see that we mostly use join on these futures



##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/PersistentPageMemory.java:
##########
@@ -1295,6 +1295,9 @@ public class Segment extends ReentrantReadWriteLock {
         /** Padding to read from word beginning. */
         private static final int ACQUIRED_PAGES_PADDING = 4;
 
+        /** Page memory. */
+        private final PersistentPageMemory pageMemory;

Review Comment:
   Segment class is inner, so you can just use `PersistentPageMemory.this`



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