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