wernerdv commented on code in PR #13554:
URL: https://github.com/apache/ignite/pull/13554#discussion_r4129823055
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java:
##########
@@ -1213,27 +1267,159 @@ public void ensureFreeSpaceForInsert(DataRegion
region, int dataRowSize) throws
// Note that not the whole page can be used to storing links,
// see PagesListNodeIO and PagesListMetaIO#getCapacity(), so we
pessimistically multiply the result on 1.5,
// in any way, the number of required pages is less than 1 percent.
- boolean oomThreshold = (memorySize / pageMem.systemPageSize()) <
+ boolean oomThreshold = (regCfg.getMaxSize() /
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();
+
+ // Maximum payload bytes that a single data page can hold for a
fragmented row.
+ long pagePayload = pageMem.pageSize() -
AbstractDataPageIO.MIN_DATA_PAGE_OVERHEAD;
- if (cctx.kernalContext() != null)
- cctx.kernalContext().failure().process(new
FailureContext(FailureType.CRITICAL_ERROR, oom));
+ // 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).
+ if (dataRowSize <= regCfg.getEmptyPagesPoolSize() * pagePayload)
+ return;
- throw oom;
+ CacheFreeList freeList = freeListMap.get(regCfg.getName());
+
+ if (freeList == null)
+ return;
+
+ long totalPages = regCfg.getMaxSize() / pageMem.systemPageSize();
+
+ // 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;
+
+ // The row fundamentally cannot fit into the whole region.
+ if (requiredPages > totalPages)
+ throw outOfMemory(regCfg);
+
+ // The reserve must guarantee `requiredPages` REAL empty pages, not
just apparent headroom. Both are shared and
+ // non-exclusive (emptyDataPages() is a snapshot; any writer can
consume them), but once the region is full
+ // (loadedPages == totalPages) headroom can no longer grow it (fresh
allocateDataPage -> raw OOM), while empty
+ // pages in the reuse bucket stay reachable via takePage(). So empty
pages are the only resource the fragmented
+ // write can consume on a full region. The TOCTOU between this reserve
and the actual write is closed by the
+ // lazy re-reserve in AbstractFreeList#writeSinglePage.
+ long emptyPages = freeList.emptyDataPages();
+
+ // The gate reuses evictionThreshold as a regime boundary, not as
"when to start eviction" (evictionRequired()
+ // does that, stopping on emptyPages >= poolSize; no last 10% of page
memory is left unusable). Below the
+ // threshold the region has real slack, so a row fitting into the
combined spare space is satisfied without
+ // eviction (live, e.g. short-TTL, entries are not evicted just to
accumulate empty pages). At/above it headroom
+ // is no longer trustworthy (concurrent writers could commit the same
headroom - TOCTOU), so only real empty
+ // pages are counted and eviction is driven below.
+ boolean evictionRegime = pageMem.loadedPages() >= (long)(totalPages *
regCfg.getEvictionThreshold());
Review Comment:
Fixed in the latest commits:
1. `ensureFreeSpaceForEviction` — removed `evictionRegime` entirely.
The fast path and the eviction loop now always use
`effectiveHeadroom(totalPages, loadedPages)`, so headroom and empty pages are
treated symmetrically in both branches. There is no longer any difference
between them.
2. `ensureFreeSpace` (called from `GridCacheMapEntry.ensureFreeSpace()`
before `lockEntry()`) — added a headroom gate.
If `effectiveHeadroom > emptyPagesPoolSize`, the method returns early
without entering the `evictionRequired()` loop, because new pages can be
allocated from headroom instead of evicting. Previously, this method evicted as
soon as `evictionRequired()` returned `true` (i.e. `loadedPages` crossed the
0.9 threshold), regardless of remaining headroom, capping the region at ~90%.
3. `AbstractFreeList.allocateDataPage` — added OOM recovery.
If headroom was consumed between the pre-reserve and the actual
`allocatePage` call (TOCTOU), the catch block calls `ensureFreeSpaceForInsert`
(which evicts) and retries `takePage`. This eliminates the OOM that previously
occurred when the `evictionRegime` gate was removed.
4. `testRegionFillsBeyondEvictionThreshold` — new test.
Fills the region to ≥95% of `totalPages` via both `cache.put` and
`cache.putAll`, asserts `isEvictionsStarted() == false`.
--
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]
