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


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

Review Comment:
   Correct. `emptyDataPages()` is a snapshot of a shared AtomicLong counter; 
any writer can consume it. 
   The real distinction isn't exclusivity but reachability at the capacity 
limit: at loadedPages == totalPages headroom can't grow the region (fresh 
allocateDataPage → raw OOM) whereas reuse-bucket empty pages remain reachable 
via takePage(). 
   Correctness rests on the lazy re-reserve (now 
AbstractFreeList#takePageWithReserve), not on the reserve being exclusive.



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