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


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -164,9 +194,16 @@ public Random2LruPageEvictionTracker(
     @Override protected boolean checkTouch(long pageId) {
         int trackingIdx = trackingIdx(PageIdUtils.pageIndex(pageId));
 
-        int firstTs = GridUnsafe.getIntVolatile(null, trackingArrPtr + 
trackingIdx * 8L);
+        int firstTs = first(GridUnsafe.getLongVolatile(null, trackingArrPtr + 
trackingIdx * 8L));
 
-        return firstTs != 0;
+        return firstTs > 0;
+    }
+
+    /** */
+    private long trackingData(long pageId) {

Review Comment:
   Can be inlined



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -177,4 +214,79 @@ public Random2LruPageEvictionTracker(
 
         GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
0L);
     }
+
+    /** {@inheritDoc} */
+    @Override public void trackFragmentPage(long pageId, long prevPageId, 
boolean isHeadPage) {
+        // Do nothing if called for tail page.
+        if (prevPageId == 0)
+            return;
+
+        if (isHeadPage) {
+            // Store link to head fragment page in tail fragment page.
+            linkFragmentPages(tailPageIdx(prevPageId), 
PageIdUtils.pageIndex(pageId));
+        }
+        else {
+            // Store link to tail fragment page in each fragment page.
+            linkFragmentPages(PageIdUtils.pageIndex(pageId), 
tailPageIdx(prevPageId));
+        }
+    }
+
+    /**
+     * 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 pageIdx Page index.
+     * @param nextPageIdx Index of page to link to.
+     */
+    private void linkFragmentPages(int pageIdx, int nextPageIdx) {
+        int trackingIdx = trackingIdx(pageIdx);
+
+        GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
toLong(-1, nextPageIdx));
+    }
+
+    /**
+     * Helper to encode a pair of integer values into a single long.
+     */
+    private long toLong(int first, int second) {
+        return U.toLong(first, second);

Review Comment:
   Can be inlined



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/Random2LruPageEvictionTracker.java:
##########
@@ -177,4 +214,79 @@ public Random2LruPageEvictionTracker(
 
         GridUnsafe.putLongVolatile(null, trackingArrPtr + trackingIdx * 8L, 
0L);
     }
+
+    /** {@inheritDoc} */
+    @Override public void trackFragmentPage(long pageId, long prevPageId, 
boolean isHeadPage) {
+        // Do nothing if called for tail page.
+        if (prevPageId == 0)
+            return;
+
+        if (isHeadPage) {
+            // Store link to head fragment page in tail fragment page.
+            linkFragmentPages(tailPageIdx(prevPageId), 
PageIdUtils.pageIndex(pageId));
+        }
+        else {
+            // Store link to tail fragment page in each fragment page.
+            linkFragmentPages(PageIdUtils.pageIndex(pageId), 
tailPageIdx(prevPageId));
+        }
+    }
+
+    /**
+     * 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 pageIdx Page index.
+     * @param nextPageIdx Index of page to link to.
+     */
+    private void linkFragmentPages(int pageIdx, int nextPageIdx) {

Review Comment:
   Let's store trackingIdx, not pageIdx. In this case:
   1. `trackFragmentPage` will require 2 remapping from pageIdx to trackingIdx 
(currently the same amount), but we don't need extra remapping in 
`evictDataPage`
   2. We can implement the same approach in both implementation of trackings. 
`RandomLruPageEvictionTracker` uses 1 int to store the data, we can use first 
bit to indicate page reference. `pageIdx` can't be used, since first bit of 
`pageIdx` is used by segment and can be 1. But trackingIdx - is int and always 
> 0, so trackingIdx can be used here. 



##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/eviction/paged/Random2LruPageEvictionPutLargeObjectsTest.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.internal.processors.cache.eviction.paged;
+
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.affinity.Affinity;
+import org.apache.ignite.configuration.DataPageEvictionMode;
+import org.apache.ignite.configuration.DataRegionConfiguration;
+import org.apache.ignite.configuration.DataStorageConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.testframework.junits.WithSystemProperty;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static 
org.apache.ignite.configuration.DataStorageConfiguration.DFLT_PAGE_SIZE;
+import static 
org.apache.ignite.internal.processors.cache.eviction.paged.PageEvictionAbstractTest.setEvictionMode;
+
+/** */
+public class Random2LruPageEvictionPutLargeObjectsTest extends 
GridCommonAbstractTest {
+    /** Offheap size for memory policy. */
+    private static final int SIZE = 1024 * 1024 * 1024;
+
+    /** Record size. */
+    private static final int RECORD_SIZE = 80 * 1024 * 1024;
+
+    /** Number of entries. */
+    static final int ENTRIES = 100;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String gridName) 
throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(gridName)
+            .setDataStorageConfiguration(new DataStorageConfiguration()
+                .setDefaultDataRegionConfiguration(new 
DataRegionConfiguration()
+                    .setInitialSize(SIZE)
+                    .setMaxSize(SIZE)
+                )
+                .setPageSize(DFLT_PAGE_SIZE)
+            );
+
+        return setEvictionMode(DataPageEvictionMode.RANDOM_2_LRU, cfg);
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids();
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    @WithSystemProperty(key = "IGNITE_DUMP_THREADS_ON_FAILURE", value = 
"false")
+    public void testPutLargeObjects() throws Exception {
+        IgniteEx ignite = startGrids(2);
+
+        IgniteCache<Integer, Object> cache = 
ignite.createCache(DEFAULT_CACHE_NAME);
+
+        Affinity<Integer> aff = ignite.affinity(DEFAULT_CACHE_NAME);
+
+        Object val = new byte[RECORD_SIZE];
+
+        int counter = 0;
+
+        for (int key = 1; key <= ENTRIES; key++) {
+            // Skip keys local node is primary for to force async processing 
in system striped pool in the non-local node.
+            if (!aff.isPrimary(ignite.localNode(), key)) {

Review Comment:
   ```
   for (Integer key : primaryKeys(grid(1).cache(DEFAULT_CACHE_NAME), 50))
   ```
   Why only striped pool is interested?
   
   Any checks? For example size < 50 to check that pages are evicted?



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/freelist/AbstractFreeList.java:
##########
@@ -202,9 +204,15 @@ protected Integer addRow(
             written = (written == 0 && oldFreeSpace >= rowSize) ? 
addRowFull(pageId, page, pageAddr, io, row, rowSize) :
                 addRowFragment(pageId, page, pageAddr, io, row, written, 
rowSize);
 
-            if (written == rowSize)
+            if (written == rowSize) {
                 evictionTracker.touchPage(pageId);
 
+                if (lastLink != 0)

Review Comment:
   Already checked in `trackFragmentPage`



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