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


##########
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);

Review Comment:
   Do we calculate here a proper header size only?



##########
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:
   Should we move this knowledge how to properly copy compressed partition 
inside `IgniteSnapshotManager` to avoid such a duplicated work in future? 



##########
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:
   Could be combined with Thread.interrupted() like 'stopped or interruped'.



##########
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());
+
+                if (Thread.interrupted())
+                    throw new IgniteInterruptedException("Thread has been 
interrupted: " + Thread.currentThread().getName());
+
+                buf.clear();
+                src.readFully(buf, off);
+                buf.flip();
+
+                if (PageIO.getCompressionType(buf) != 
CompressionProcessor.UNCOMPRESSED_PAGE)
+                    buf.limit(PageIO.getCompressedSize(buf));
+
+                dest.writeFully(buf, off);
+            }
+        }
+        catch (IOException e) {
+            throw new IgniteException(e);
+        }
+    }
+
     /** */
     private void punchHole(int grpId, int partId, File partFile) throws 
Exception {

Review Comment:
   It looks like we have a similar copy path when receiving a partition from a 
remote node. Should we skip zero-filled pages on the fly there too, so the 
resulting file stays sparse?



##########
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());
+
+                if (Thread.interrupted())
+                    throw new IgniteInterruptedException("Thread has been 
interrupted: " + Thread.currentThread().getName());
+
+                buf.clear();
+                src.readFully(buf, off);
+                buf.flip();
+
+                if (PageIO.getCompressionType(buf) != 
CompressionProcessor.UNCOMPRESSED_PAGE)
+                    buf.limit(PageIO.getCompressedSize(buf));

Review Comment:
   Does this guarantee that free space is available as we iterate over pages 
with actual `pageSize` step? Should we reuse punchHole()? 



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