alex-plekhanov commented on code in PR #12026:
URL: https://github.com/apache/ignite/pull/12026#discussion_r2109268911


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -177,4 +205,85 @@ public Random2LruPageEvictionTracker(
 
         GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
0L);
     }
+
+    /** {@inheritDoc} */
+    @Override public void trackFragmentPage(long pageId, long tailPageId) {
+        // Store link to tail fragment page in each fragment page.
+        linkFragmentPages(pageId, tailPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void trackTailFragmentPage(long tailPageId, long 
headPageId) {
+        // Store link to head fragment page in tail fragment page.
+        linkFragmentPages(tailPageId, headPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void forgetFragmentPage(long pageId) throws 
IgniteCheckedException {
+        int pageIdx = PageIdUtils.pageIndex(pageId);
+
+        int trackingIdx = trackingIdx(pageIdx);
+
+        long latestTs = compactTimestamp(U.currentTimeMillis());
+
+        assert latestTs >= 0 && latestTs < Integer.MAX_VALUE;
+
+        GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
asLong((int)latestTs, 0));
+    }
+
+    /**
+     * Link two pages containing fragments of row.
+     * Link is stored as a page index in the second integer in the tracking 
data.
+     * First integer in the tracking data is set to -1.
+     *
+     * @param pageId Page id.
+     * @param nextPageId Id of page to link to.
+     */
+    private void linkFragmentPages(long pageId, long nextPageId) {
+        int pageIdx = PageIdUtils.pageIndex(pageId);
+
+        int nextPageIdx = PageIdUtils.pageIndex(nextPageId);
+
+        boolean success;
+
+        do {
+            int trackingIdx = trackingIdx(pageIdx);
+
+            long trackinData = GridUnsafe.getLongVolatile(null, trackingArrPtr 
+ trackingIdx * 8L);
+
+            int firstTs = first(trackinData);
+
+            if (firstTs == -1)
+                return;
+
+            success = GridUnsafe.compareAndSwapLong(null, trackingArrPtr + 
trackingIdx * 8L, trackinData, asLong(-1, nextPageIdx));
+        } while (!success);
+    }
+
+    /** */
+    private static final long FIRST_INT_MASK = ~(-1L << Integer.SIZE);
+
+    /** */
+    private static final long SECOND_INT_MASK = -1L << Integer.SIZE;
+
+    /**
+     * Encode a pair of integer values into a single long.
+     */
+    private long asLong(int first, int second) {
+        return (((long)second) << Integer.SIZE) | ((long)first & 
FIRST_INT_MASK);
+    }
+
+    /**
+     * Decode long and extract firsts integer value.
+     */
+    private int first(long l) {
+        return (int)(l & FIRST_INT_MASK);

Review Comment:
   return (int)l;



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -177,4 +205,85 @@ public Random2LruPageEvictionTracker(
 
         GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
0L);
     }
+
+    /** {@inheritDoc} */
+    @Override public void trackFragmentPage(long pageId, long tailPageId) {
+        // Store link to tail fragment page in each fragment page.
+        linkFragmentPages(pageId, tailPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void trackTailFragmentPage(long tailPageId, long 
headPageId) {
+        // Store link to head fragment page in tail fragment page.
+        linkFragmentPages(tailPageId, headPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void forgetFragmentPage(long pageId) throws 
IgniteCheckedException {
+        int pageIdx = PageIdUtils.pageIndex(pageId);
+
+        int trackingIdx = trackingIdx(pageIdx);
+
+        long latestTs = compactTimestamp(U.currentTimeMillis());
+
+        assert latestTs >= 0 && latestTs < Integer.MAX_VALUE;
+
+        GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
asLong((int)latestTs, 0));
+    }
+
+    /**
+     * Link two pages containing fragments of row.
+     * Link is stored as a page index in the second integer in the tracking 
data.
+     * First integer in the tracking data is set to -1.
+     *
+     * @param pageId Page id.
+     * @param nextPageId Id of page to link to.
+     */
+    private void linkFragmentPages(long pageId, long nextPageId) {
+        int pageIdx = PageIdUtils.pageIndex(pageId);
+
+        int nextPageIdx = PageIdUtils.pageIndex(nextPageId);
+
+        boolean success;
+
+        do {
+            int trackingIdx = trackingIdx(pageIdx);
+
+            long trackinData = GridUnsafe.getLongVolatile(null, trackingArrPtr 
+ trackingIdx * 8L);
+
+            int firstTs = first(trackinData);
+
+            if (firstTs == -1)
+                return;
+
+            success = GridUnsafe.compareAndSwapLong(null, trackingArrPtr + 
trackingIdx * 8L, trackinData, asLong(-1, nextPageIdx));
+        } while (!success);
+    }
+
+    /** */
+    private static final long FIRST_INT_MASK = ~(-1L << Integer.SIZE);
+
+    /** */
+    private static final long SECOND_INT_MASK = -1L << Integer.SIZE;
+
+    /**
+     * Encode a pair of integer values into a single long.
+     */
+    private long asLong(int first, int second) {
+        return (((long)second) << Integer.SIZE) | ((long)first & 
FIRST_INT_MASK);
+    }
+
+    /**
+     * Decode long and extract firsts integer value.
+     */
+    private int first(long l) {
+        return (int)(l & FIRST_INT_MASK);
+    }
+
+    /**
+     * Decode long and extract second integer value.
+     */
+    private int second(long l) {
+        return (int)((l & SECOND_INT_MASK) >> Integer.SIZE);

Review Comment:
   return (int)(l >> Integer.SIZE)



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/freelist/AbstractFreeList.java:
##########
@@ -416,6 +431,9 @@ else if (putIsNeeded)
                     put(null, pageId, page, pageAddr, newBucket, statHolder);
             }
 
+            if (nextLink != 0 && !io.isEmpty(pageAddr))

Review Comment:
   Condition `if (nextLink != 0 && !io.isEmpty(pageAddr))` indicating that it's 
a head page. 
   Not sure if we should touch non-empty head page at all after deletion. But 
for sure we shouldn't forgot it (it should contain correct timestamps, not 
links). 



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -177,4 +205,85 @@ public Random2LruPageEvictionTracker(
 
         GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
0L);
     }
+
+    /** {@inheritDoc} */
+    @Override public void trackFragmentPage(long pageId, long tailPageId) {
+        // Store link to tail fragment page in each fragment page.
+        linkFragmentPages(pageId, tailPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void trackTailFragmentPage(long tailPageId, long 
headPageId) {
+        // Store link to head fragment page in tail fragment page.
+        linkFragmentPages(tailPageId, headPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void forgetFragmentPage(long pageId) throws 
IgniteCheckedException {
+        int pageIdx = PageIdUtils.pageIndex(pageId);
+
+        int trackingIdx = trackingIdx(pageIdx);
+
+        long latestTs = compactTimestamp(U.currentTimeMillis());
+
+        assert latestTs >= 0 && latestTs < Integer.MAX_VALUE;
+
+        GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
asLong((int)latestTs, 0));
+    }
+
+    /**
+     * Link two pages containing fragments of row.
+     * Link is stored as a page index in the second integer in the tracking 
data.
+     * First integer in the tracking data is set to -1.
+     *
+     * @param pageId Page id.
+     * @param nextPageId Id of page to link to.
+     */
+    private void linkFragmentPages(long pageId, long nextPageId) {
+        int pageIdx = PageIdUtils.pageIndex(pageId);
+
+        int nextPageIdx = PageIdUtils.pageIndex(nextPageId);
+
+        boolean success;
+
+        do {
+            int trackingIdx = trackingIdx(pageIdx);
+
+            long trackinData = GridUnsafe.getLongVolatile(null, trackingArrPtr 
+ trackingIdx * 8L);
+
+            int firstTs = first(trackinData);
+
+            if (firstTs == -1)
+                return;
+
+            success = GridUnsafe.compareAndSwapLong(null, trackingArrPtr + 
trackingIdx * 8L, trackinData, asLong(-1, nextPageIdx));
+        } while (!success);
+    }
+
+    /** */
+    private static final long FIRST_INT_MASK = ~(-1L << Integer.SIZE);
+
+    /** */
+    private static final long SECOND_INT_MASK = -1L << Integer.SIZE;
+
+    /**
+     * Encode a pair of integer values into a single long.
+     */
+    private long asLong(int first, int second) {

Review Comment:
   IgniteUtils.toLong



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -177,4 +205,85 @@ public Random2LruPageEvictionTracker(
 
         GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
0L);
     }
+
+    /** {@inheritDoc} */
+    @Override public void trackFragmentPage(long pageId, long tailPageId) {
+        // Store link to tail fragment page in each fragment page.
+        linkFragmentPages(pageId, tailPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void trackTailFragmentPage(long tailPageId, long 
headPageId) {
+        // Store link to head fragment page in tail fragment page.
+        linkFragmentPages(tailPageId, headPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void forgetFragmentPage(long pageId) throws 
IgniteCheckedException {

Review Comment:
   Why do we need dedicated method for fragment pages? These pages can be 
forgotten by the same forgotPage method, and currently you just update ts for 
these pages, not forgot them.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -177,4 +205,85 @@ public Random2LruPageEvictionTracker(
 
         GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
0L);
     }
+
+    /** {@inheritDoc} */
+    @Override public void trackFragmentPage(long pageId, long tailPageId) {
+        // Store link to tail fragment page in each fragment page.
+        linkFragmentPages(pageId, tailPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void trackTailFragmentPage(long tailPageId, long 
headPageId) {
+        // Store link to head fragment page in tail fragment page.
+        linkFragmentPages(tailPageId, headPageId);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void forgetFragmentPage(long pageId) throws 
IgniteCheckedException {
+        int pageIdx = PageIdUtils.pageIndex(pageId);
+
+        int trackingIdx = trackingIdx(pageIdx);
+
+        long latestTs = compactTimestamp(U.currentTimeMillis());
+
+        assert latestTs >= 0 && latestTs < Integer.MAX_VALUE;
+
+        GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
asLong((int)latestTs, 0));
+    }
+
+    /**
+     * Link two pages containing fragments of row.
+     * Link is stored as a page index in the second integer in the tracking 
data.
+     * First integer in the tracking data is set to -1.
+     *
+     * @param pageId Page id.
+     * @param nextPageId Id of page to link to.
+     */
+    private void linkFragmentPages(long pageId, long nextPageId) {
+        int pageIdx = PageIdUtils.pageIndex(pageId);
+
+        int nextPageIdx = PageIdUtils.pageIndex(nextPageId);
+
+        boolean success;
+
+        do {
+            int trackingIdx = trackingIdx(pageIdx);
+
+            long trackinData = GridUnsafe.getLongVolatile(null, trackingArrPtr 
+ trackingIdx * 8L);
+
+            int firstTs = first(trackinData);
+
+            if (firstTs == -1)

Review Comment:
   Looks like it's not possible? 
   If it's possible, then why don't we replace the link with the new one?



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -177,4 +205,85 @@ public Random2LruPageEvictionTracker(
 
         GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
0L);
     }
+
+    /** {@inheritDoc} */
+    @Override public void trackFragmentPage(long pageId, long tailPageId) {

Review Comment:
   API looks inconsistent. Method called "track page" but gets two pages as 
parameter. Maybe just "linkPages(long srcPageId, long destPageId)"?



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/freelist/AbstractFreeList.java:
##########
@@ -655,24 +678,62 @@ private long allocateDataPage(int part) throws 
IgniteCheckedException {
         }
     }
 
+    /**
+     * Grid cursor for rows to be written.
+     * <p>
+     * If current row is fragmented allows to maintain ID of page containg 
tail fragment.
+     */
+    private static class WriteRowsGridCursor<T> extends 
GridCursorIteratorWrapper<T> {
+        /** Tail page id. */
+        private long tailPageId = -1;
+
+        /**
+         * @param iter Iterator.
+         */
+        public WriteRowsGridCursor(Iterator<T> iter) {
+            super(iter);
+        }
+
+        /** {@inheritDoc} */
+        @Override public boolean next() throws IgniteCheckedException {
+            tailPageId(-1);
+
+            return super.next();
+        }
+
+        /** Get tail page id. */
+        public void tailPageId(long tailPageId) {
+            this.tailPageId = tailPageId;
+        }
+
+        /** Set tail page id. */
+        public long tailPageId() {
+            return tailPageId;
+        }
+    }
+
     /**
      * Write fragments of the row, which occupy the whole memory page. A data 
row is ignored if it is less than the max
      * payload of an empty data page.
      *
      * @param row Row to process.
      * @param statHolder Statistics holder to track IO operations.
+     * @param cur Rows cursor.
      * @return Number of bytes written, {@link #COMPLETE} if the row was fully 
written, {@code 0} if data row was
      * ignored because it is less than the max payload of an empty data page.
      * @throws IgniteCheckedException If failed.
      */
-    private int writeWholePages(T row, IoStatisticsHolder statHolder) throws 
IgniteCheckedException {
+    private int writeWholePages(T row, IoStatisticsHolder statHolder, 
WriteRowsGridCursor<T> cur) throws IgniteCheckedException {
         assert row.link() == 0 : row.link();
 
         int written = 0;
         int rowSize = row.size();
 
         while (rowSize - written >= MIN_SIZE_FOR_DATA_PAGE) {
-            written = writeSinglePage(row, written, statHolder);
+            written = writeSinglePage(row, written, statHolder, 
cur.tailPageId());
+
+            if (written != COMPLETE && cur.tailPageId() == -1)
+                cur.tailPageId(PageIdUtils.pageId(row.link()));

Review Comment:
   One tailId for all rows in cursor looks incorrect.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -123,9 +130,30 @@ public Random2LruPageEvictionTracker(
             while (dataPagesCnt < SAMPLE_SIZE) {
                 int trackingIdx = rnd.nextInt(trackingSize);
 
-                int firstTs = GridUnsafe.getIntVolatile(null, trackingArrPtr + 
trackingIdx * 8L);
+                long trackingData = GridUnsafe.getLongVolatile(null, 
trackingArrPtr + trackingIdx * 8L);
+
+                int firstTs = first(trackingData);
+
+                if (firstTs == -1) {
+                    // For page containing fragmented row data timestamps 
stored in the row's head page are used.
+                    // Fragment page (other than the tail one) contains link 
to tail page. Tail page contains link to
+                    // head page. So head page is found no more than in two 
hops.
+                    trackingIdx = trackingIdx(second(trackingData));
+
+                    trackingData = GridUnsafe.getLongVolatile(null, 
trackingArrPtr + trackingIdx * 8L);
 
-                int secondTs = GridUnsafe.getIntVolatile(null, trackingArrPtr 
+ trackingIdx * 8L + 4);
+                    firstTs = first(trackingData);
+
+                    if (firstTs == -1) {
+                        trackingIdx = trackingIdx(second(trackingData));
+
+                        trackingData = GridUnsafe.getLongVolatile(null, 
trackingArrPtr + trackingIdx * 8L);
+
+                        firstTs = first(trackingData);

Review Comment:
   `assert firstTs >= 0`?



-- 
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-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to