wernerdv commented on code in PR #13554:
URL: https://github.com/apache/ignite/pull/13554#discussion_r4004738919


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java:
##########
@@ -1216,24 +1270,170 @@ public void ensureFreeSpaceForInsert(DataRegion 
region, int dataRowSize) throws
         boolean oomThreshold = (memorySize / pageMem.systemPageSize()) <
             ((double)dataRowSize / pageMem.pageSize() + nonEmptyPages * (8.0 * 
1.5 / pageMem.pageSize() + 1) + 256 /*one page per bucket*/);
 
-        if (oomThreshold) {
-            IgniteOutOfMemoryException oom = new 
IgniteOutOfMemoryException("Out of memory in data region [" +
-                "name=" + regCfg.getName() +
-                ", initSize=" + U.readableSize(regCfg.getInitialSize(), false) 
+
-                ", maxSize=" + U.readableSize(regCfg.getMaxSize(), false) +
-                ", persistenceEnabled=" + regCfg.isPersistenceEnabled() + "] 
Try the following:" + U.nl() +
-                "  ^-- Increase maximum off-heap memory size 
(DataRegionConfiguration.maxSize)" + U.nl() +
-                "  ^-- Enable Ignite persistence 
(DataRegionConfiguration.persistenceEnabled)" + U.nl() +
-                "  ^-- Enable eviction or expiration policies"
-            );
+        if (oomThreshold)
+            throw outOfMemory(regCfg);
+    }
+
+    /**
+     * Size-aware reserve for an eviction-enabled non-persistent region. Runs 
eviction until the free list holds
+     * enough real empty pages to accommodate the row, or throws {@link 
IgniteOutOfMemoryException} if the goal is
+     * unreachable / no progress can be made. Progress is measured against the 
number of empty pages in the free list
+     * (the only resource a subsequent fragmented write can reliably consume 
once the region is effectively full); the
+     * region's spare capacity (headroom) is only trusted in the fast path 
while the region is below the eviction
+     * threshold.
+     *
+     * @param region Data region.
+     * @param regCfg Data region configuration.
+     * @param dataRowSize Size of data row to be inserted.
+     * @throws IgniteOutOfMemoryException If the target cannot be reached (row 
too large for the region or eviction
+     * makes no progress).
+     * @throws IgniteCheckedException If failed to evict data pages.
+     */
+    private void ensureFreeSpaceForEviction(DataRegion region, 
DataRegionConfiguration regCfg, int dataRowSize)
+        throws IgniteOutOfMemoryException, IgniteCheckedException {
+        PageMemory pageMem = region.pageMemory();
+
+        long pageSize = pageMem.pageSize();
+
+        // Maximum payload bytes that a single data page can hold for a 
fragmented row.
+        long pagePayload = pageSize - 
AbstractDataPageIO.MIN_DATA_PAGE_OVERHEAD;
+
+        // A row that fits into the steady-state empty-pages pool is satisfied 
by normal threshold eviction, so the
+        // fast path is a single comparison (no page computation, free-list 
lookup or page-memory reads on the hot
+        // small-put path).
+        long maxFastRowBytes = regCfg.getEmptyPagesPoolSize() * pagePayload;
+
+        if (dataRowSize <= maxFastRowBytes)
+            return;
+
+        CacheFreeList freeList = freeListMap.get(regCfg.getName());
+
+        if (freeList == null)
+            return;
+
+        long totalPages = regCfg.getMaxSize() / pageMem.systemPageSize();
 
-            if (cctx.kernalContext() != null)
-                cctx.kernalContext().failure().process(new 
FailureContext(FailureType.CRITICAL_ERROR, oom));
+        // Pages the row will actually occupy once written, and which the free 
list must hand out on demand during
+        // the fragmented write.
+        long requiredPages = (dataRowSize + pagePayload - 1) / pagePayload;
 
-            throw oom;
+        // The row fundamentally cannot fit into the whole region.
+        if (requiredPages > totalPages)
+            throw outOfMemory(regCfg);
+
+        // The reserve must guarantee that the free list holds `requiredPages` 
REAL empty pages, not merely that the
+        // region has apparent headroom. Apparent headroom (totalPages - 
loadedPages) is shared and non-exclusive:
+        // concurrent inserts can both count on it and then both run out of 
pages mid-write (TOCTOU / raw OOM), since
+        // a fresh allocation cannot grow the region beyond capacity. Once the 
region is effectively full, real empty
+        // pages already in the free list are the only resource the fragmented 
write can reliably consume, so the loop
+        // below accumulates them. (Headroom is trusted in the fast path only 
while the region is below the eviction
+        // threshold, i.e. where contention cannot exhaust the slack.)
+        long emptyPages = freeList.emptyDataPages();
+
+        long headroom = totalPages - pageMem.loadedPages();
+
+        // The region is "under pressure" once loaded pages reach the eviction 
threshold; below it a fresh allocation
+        // can safely grow the region, so a row that fits into the combined 
spare space is satisfied without eviction
+        // (which would otherwise destroy evictable, e.g. short-TTL, entries 
just to accumulate empty pages the slack
+        // could have absorbed).
+        long pagesThreshold = (long)(totalPages * 
regCfg.getEvictionThreshold());
+
+        boolean underPressure = pageMem.loadedPages() >= pagesThreshold;
+
+        // Fast path: the row is satisfiable without eviction when (a) the 
free list already holds enough real empty
+        // pages, or (b) the region is not under pressure and has enough spare 
space to grow into.
+        if (emptyPages >= requiredPages || (!underPressure && emptyPages + 
headroom >= requiredPages))
+            return;
+
+        PageEvictionTracker evictionTracker = region.evictionTracker();
+
+        // Evict data pages until the free list holds enough real empty pages. 
Progress is measured against the count
+        // of empty pages, so pages freed concurrently (e.g. by TTL cleanup) 
also count. Eviction may run while the
+        // current thread already holds entry locks (single-row insertion), so 
contended entries are skipped
+        // (non-blocking) rather than blocked on, avoiding a lock-ordering 
deadlock.
+        //
+        // The guard is time-based (no progress for 
EVICTION_NO_PROGRESS_TIMEOUT_MILLIS) rather than attempt-count:
+        // a fixed budget could exhaust in milliseconds under 
contention/lock-holders and turn a slow-but-progressing
+        // eviction into a premature OOM. On each stalled iteration the thread 
backs off (rather than busy-spinning)
+        // both to save CPU and to let a lock holder run and release it.
+        long bestEmptyPages = emptyPages;
+
+        long lastProgressNanos = System.nanoTime();
+
+        long backoffNanos = EVICTION_BACKOFF_START_NANOS;
+
+        while (bestEmptyPages < requiredPages) {
+            if (region.metrics().onPageEvictionsStarted())
+                U.warn(log, "Page-based evictions started." +
+                    " Consider increasing 'maxSize' on Data Region 
configuration: " + regCfg.getName());
+
+            evictDataPageNonBlocking(evictionTracker);
+
+            region.metrics().updateEvictionRate();
+
+            long curEmptyPages = freeList.emptyDataPages();
+
+            // Only an iteration that establishes a new highest empty-pages 
count counts as progress (drops caused by
+            // concurrent inserts consuming pages do not). As long as there is 
progress the loop continues; on a
+            // stalled iteration it backs off rather than busy-spinning.
+            if (curEmptyPages > bestEmptyPages) {
+                bestEmptyPages = curEmptyPages;
+
+                lastProgressNanos = System.nanoTime();
+
+                backoffNanos = EVICTION_BACKOFF_START_NANOS;
+            }
+            else {
+                LockSupport.parkNanos(backoffNanos);
+
+                backoffNanos = Math.min(backoffNanos << 1, 
EVICTION_BACKOFF_MAX_NANOS);
+            }
+
+            // Fail with OOM only after a sustained period without any 
progress: this bounds a genuinely stuck eviction
+            // (nothing evictable, or contenders never releasing their locks) 
without tearing down a slow-but-
+            // progressing one. The region is already under pressure (fast 
path failed), so OOM is correct here.
+            if (System.nanoTime() - lastProgressNanos > 
TimeUnit.MILLISECONDS.toNanos(EVICTION_NO_PROGRESS_TIMEOUT_MILLIS))
+                throw outOfMemory(regCfg);
         }
     }
 
+    /**
+     * Invokes a single page eviction, acquiring entry locks non-blockingly so 
that contended entries are skipped.
+     * This is required when eviction runs while the current thread already 
holds entry locks (size-aware eviction
+     * from a single-row insertion) to avoid a lock-ordering deadlock. {@link 
NoOpPageEvictionTracker}
+     * (disabled eviction, never reaching this path) falls back to the plain 
{@code evictDataPage()}.
+     *
+     * @param evictionTracker Page eviction tracker.
+     * @throws IgniteCheckedException If failed to evict a data page.
+     */
+    private void evictDataPageNonBlocking(PageEvictionTracker evictionTracker) 
throws IgniteCheckedException {
+        if (evictionTracker instanceof PageAbstractEvictionTracker)

Review Comment:
   After the latest changes, the method is no longer needed.



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