NSAmelchev commented on code in PR #13710:
URL: https://github.com/apache/ignite/pull/13710#discussion_r4445534351


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotRestoreProcess.java:
##########
@@ -1797,6 +1786,52 @@ private void copyLocalAsync(
             });
     }
 
+    /**
+     * Copies a partition of a compressed cache group page by page writing 
only the compressed bytes of each page, so the
+     * rest of the page is left unallocated by the file system and the copy is 
sparse without punching holes in it.
+     */
+    private void copyCompressed(SnapshotRestoreContext opCtx, int grpId, File 
from, File to) {
+        FileIOFactory ioFactory = 
ctx.cache().context().snapshotMgr().ioFactory();
+        FilePageStoreManager storeMgr = 
(FilePageStoreManager)ctx.cache().context().pageStore();
+        int pageSize = ctx.cache().context().database().pageSize();
+
+        try (FileIO src = ioFactory.create(from, READ); FileIO dest = 
ioFactory.create(to, CREATE, WRITE)) {
+            ByteBuffer buf = locBuff.get();
+
+            buf.clear();
+            src.readFully(buf, 0);
+            buf.flip();
+
+            buf.getLong(); // Signature.
+
+            int hdrSize = storeMgr.getPageStoreFactory(grpId, 
null).headerSize(buf.getInt());
+
+            buf.rewind();
+            buf.limit(hdrSize);
+            dest.writeFully(buf, 0);
+
+            for (long off = hdrSize, len = src.size(); off < len; off += 
pageSize) {
+                if (opCtx.stopChecker.getAsBoolean())
+                    throw new IgniteInterruptedException("The operation has 
been stopped on copy file: " + from.getAbsolutePath());

Review Comment:
   I’d keep them separate: the same pair of checks is already in copyLocalAsync 
and punchHole, and each message says what happened: the stopped operation names 
the file, the interrupt names the thread.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotRestoreProcess.java:
##########
@@ -1797,6 +1786,52 @@ private void copyLocalAsync(
             });
     }
 
+    /**
+     * Copies a partition of a compressed cache group page by page writing 
only the compressed bytes of each page, so the
+     * rest of the page is left unallocated by the file system and the copy is 
sparse without punching holes in it.
+     */
+    private void copyCompressed(SnapshotRestoreContext opCtx, int grpId, File 
from, File to) {

Review Comment:
   It has one caller, so I’d keep it next to that caller for now. Snapshot 
creation can’t reuse it as is: sendPart copies a live partition while 
checkpoints keep writing to it, and the delta fixes the copy afterwards, so a 
page header read during the copy can’t be trusted. The remote path receives a 
stream, so it has no pages to iterate over. If either path gets this 
optimization, moving the method next to copy is a small change.



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

Reply via email to