alex-plekhanov commented on code in PR #13554:
URL: https://github.com/apache/ignite/pull/13554#discussion_r3980303153
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java:
##########
@@ -1172,32 +1196,62 @@ public WALPointer
latestWalPointerReservedForPreloading() {
}
/**
- * Checks that the given {@code region} has enough space for putting a new
entry.
- *
- * This method makes sense then and only then
- * the data region is not persisted {@link
DataRegionConfiguration#isPersistenceEnabled()}
- * and page eviction is disabled {@link DataPageEvictionMode#DISABLED}.
- *
- * The non-persistent region should reserve a number of pages to support a
free list {@link AbstractFreeList}.
- * For example, removing a row from underlying store may require
allocating a new data page
- * in order to move a tracked page from one bucket to another one which
does not have a free space for a new stripe.
- * See {@link AbstractFreeList#removeDataRowByLink}.
- * Therefore, inserting a new entry should be prevented in case of some
threshold is exceeded.
+ * Checks that the given {@code region} has enough space for putting a new
entry of {@code dataRowSize} bytes.
+ * <p>
+ * For a non-persistent region with page eviction disabled, verifies that
the region reserves enough pages to
+ * support a free list {@link AbstractFreeList}. For example, removing a
row from underlying store may require
+ * allocating a new data page in order to move a tracked page from one
bucket to another one which does not have
+ * a free space for a new stripe. See {@link
AbstractFreeList#removeDataRowByLink}. Therefore, inserting a new
+ * entry should be prevented in case of some threshold is exceeded.
+ * <p>
+ * For a non-persistent region with page eviction enabled, additionally
performs size-aware eviction: when the
+ * row does not fit into the currently available page space, data pages
are evicted until either enough space is
+ * freed or it becomes clear that the goal is unreachable (in which case an
+ * {@link IgniteOutOfMemoryException} is thrown).
+ * <p>
+ * The size-aware reserve is required because page eviction by itself only
keeps a steady-state pool of empty pages
+ * ({@link DataRegionConfiguration#getEmptyPagesPoolSize()}) and does not
guarantee enough space for a single row
+ * larger than this pool.
+ * <p>
+ * Worst case: when called while the entry being written is locked
(single-row insertion), the eviction loop can
+ * hold that lock for up to {@link #EVICTION_NO_PROGRESS_TIMEOUT_MILLIS} —
only if no evictable entry releases
+ * its lock within that time (e.g. a long-running transaction holding all
evictable entries), after which the
+ * call fails with {@link IgniteOutOfMemoryException} (reported as a
critical failure to the configured failure
+ * handler).
*
* @param region Data region to be checked.
* @param dataRowSize Size of data row to be inserted.
- * @throws IgniteOutOfMemoryException In case of the given data region
does not have enough free space
- * for putting a new entry.
+ * @throws IgniteOutOfMemoryException In case the given data region does
not have enough free space
+ * for putting a new entry, even after eviction.
+ * @throws IgniteCheckedException If failed to evict data pages.
*/
- public void ensureFreeSpaceForInsert(DataRegion region, int dataRowSize)
throws IgniteOutOfMemoryException {
+ public void ensureFreeSpaceForInsert(DataRegion region, int dataRowSize)
+ throws IgniteOutOfMemoryException, IgniteCheckedException {
if (region == null)
return;
DataRegionConfiguration regCfg = region.config();
- if (regCfg.getPageEvictionMode() != DataPageEvictionMode.DISABLED ||
regCfg.isPersistenceEnabled())
+ if (regCfg.isPersistenceEnabled())
return;
+ if (regCfg.getPageEvictionMode() == DataPageEvictionMode.DISABLED)
+ checkOomThreshold(region, regCfg, dataRowSize);
+ else
+ ensureFreeSpaceForEviction(region, regCfg, dataRowSize);
+ }
+
+ /**
+ * Checks that a non-persistent region with disabled page eviction has
enough pages for a new row, taking into
+ * account the pages required to support the free list.
+ *
+ * @param region Data region.
+ * @param regCfg Data region configuration.
+ * @param dataRowSize Size of data row to be inserted.
+ * @throws IgniteOutOfMemoryException If the region does not have enough
free space for the new entry.
+ */
+ private void checkOomThreshold(DataRegion region, DataRegionConfiguration
regCfg, int dataRowSize)
Review Comment:
Codestyle violation
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/IgniteCacheDatabaseSharedManager.java:
##########
@@ -1172,32 +1196,62 @@ public WALPointer
latestWalPointerReservedForPreloading() {
}
/**
- * Checks that the given {@code region} has enough space for putting a new
entry.
- *
- * This method makes sense then and only then
- * the data region is not persisted {@link
DataRegionConfiguration#isPersistenceEnabled()}
- * and page eviction is disabled {@link DataPageEvictionMode#DISABLED}.
- *
- * The non-persistent region should reserve a number of pages to support a
free list {@link AbstractFreeList}.
- * For example, removing a row from underlying store may require
allocating a new data page
- * in order to move a tracked page from one bucket to another one which
does not have a free space for a new stripe.
- * See {@link AbstractFreeList#removeDataRowByLink}.
- * Therefore, inserting a new entry should be prevented in case of some
threshold is exceeded.
+ * Checks that the given {@code region} has enough space for putting a new
entry of {@code dataRowSize} bytes.
+ * <p>
+ * For a non-persistent region with page eviction disabled, verifies that
the region reserves enough pages to
+ * support a free list {@link AbstractFreeList}. For example, removing a
row from underlying store may require
+ * allocating a new data page in order to move a tracked page from one
bucket to another one which does not have
+ * a free space for a new stripe. See {@link
AbstractFreeList#removeDataRowByLink}. Therefore, inserting a new
+ * entry should be prevented in case of some threshold is exceeded.
+ * <p>
+ * For a non-persistent region with page eviction enabled, additionally
performs size-aware eviction: when the
+ * row does not fit into the currently available page space, data pages
are evicted until either enough space is
+ * freed or it becomes clear that the goal is unreachable (in which case an
+ * {@link IgniteOutOfMemoryException} is thrown).
+ * <p>
+ * The size-aware reserve is required because page eviction by itself only
keeps a steady-state pool of empty pages
+ * ({@link DataRegionConfiguration#getEmptyPagesPoolSize()}) and does not
guarantee enough space for a single row
+ * larger than this pool.
+ * <p>
+ * Worst case: when called while the entry being written is locked
(single-row insertion), the eviction loop can
+ * hold that lock for up to {@link #EVICTION_NO_PROGRESS_TIMEOUT_MILLIS} —
only if no evictable entry releases
+ * its lock within that time (e.g. a long-running transaction holding all
evictable entries), after which the
+ * call fails with {@link IgniteOutOfMemoryException} (reported as a
critical failure to the configured failure
+ * handler).
*
* @param region Data region to be checked.
* @param dataRowSize Size of data row to be inserted.
- * @throws IgniteOutOfMemoryException In case of the given data region
does not have enough free space
- * for putting a new entry.
+ * @throws IgniteOutOfMemoryException In case the given data region does
not have enough free space
+ * for putting a new entry, even after eviction.
+ * @throws IgniteCheckedException If failed to evict data pages.
*/
- public void ensureFreeSpaceForInsert(DataRegion region, int dataRowSize)
throws IgniteOutOfMemoryException {
+ public void ensureFreeSpaceForInsert(DataRegion region, int dataRowSize)
Review Comment:
Codestyle violation: In case of multi-line method declaration, each
parameter should be on it's own line.
##########
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)
Review Comment:
Codestyle violation
##########
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())
Review Comment:
Braces required for multi-line statements
##########
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:
> Apparent headroom (totalPages - loadedPages) is shared and non-exclusive
But empty pages are also shared and non-exclusive
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/RowStore.java:
##########
@@ -132,8 +133,25 @@ public void addRow(CacheDataRow row, IoStatisticsHolder
statHolder) throws Ignit
* @param statHolder Statistics holder to track IO operations.
* @throws IgniteCheckedException If failed.
*/
- public void addRows(Collection<? extends CacheDataRow> rows,
- IoStatisticsHolder statHolder) throws IgniteCheckedException {
+ public void addRows(Collection<? extends CacheDataRow> rows,
IoStatisticsHolder statHolder) throws IgniteCheckedException {
+ if (!persistenceEnabled &&
grp.dataRegion().config().getPageEvictionMode() !=
DataPageEvictionMode.DISABLED) {
+ // Size-aware reserve for each row in the batch (reserving only
the largest is insufficient: a later large
+ // row can still exhaust page memory mid-write). The
reserve/consume TOCTOU race and the "second large row
+ // in a batch" case are both closed by the lazy re-reserve in
AbstractFreeList#writeSinglePage, which
+ // re-runs the reserve on the row remainder when a fragmented
write cannot take a page (a raw OOM there
+ // would otherwise be wrapped by insertDataRows into
CorruptedFreeListException and reported as corruption).
+ //
+ // The reserve evicts non-blockingly even though the batch path
holds no entry locks (so blocking would be
+ // deadlock-safe and more effective here): the same reserve path
is shared with single-row insertion,
+ // which runs under an entry lock and must not block.
+ for (CacheDataRow row : rows) {
+ int rowSize = row.size();
+
+ if (rowSize > 0)
Review Comment:
How it can be <= 0?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/freelist/AbstractFreeList.java:
##########
@@ -701,10 +713,29 @@ private int writeWholePages(T row, IoStatisticsHolder
statHolder) throws IgniteC
* @throws IgniteCheckedException If failed.
*/
private int writeSinglePage(T row, int written, IoStatisticsHolder
statHolder) throws IgniteCheckedException {
+ // TOCTOU closure: the size-aware reserve (ensureFreeSpaceForInsert,
invoked from RowStore.addRow/addRows
+ // before this write) accumulates enough real empty pages but does not
pin them to this thread - a concurrent
+ // writer can consume them between the reserve and this allocation.
When the free list cannot hand out a page,
+ // re-reserve on the remaining size and retry before allocating a
brand-new page; otherwise the race surfaces
+ // as a raw IgniteOutOfMemoryException (wrapped into
CorruptedFreeListException in the batch path).
+ //
+ // The re-reserve is an inline demand-eviction: reached from the
BPlusTree.invoke row-creation closure, it may
+ // re-entrantly remove other entries from the same data tree. That is
safe because the closure runs with no
+ // data-tree page locks held (page read lock released before it runs,
leaf write lock taken after), and the
+ // outer operation revalidates via the page tag / triangle / removeId
protocols. The key being written is
+ // skipped (its entry lock is held, so tryLock fails for it), so there
is no self-eviction or lock-ordering
+ // deadlock; like the initial reserve, the re-reserve throws OOM if
the row genuinely cannot fit.
AbstractDataPageIO initIo = null;
long pageId = takePage(row.size() - written, row, statHolder);
+ if (pageId == 0L) {
+ if (dbMgr != null)
+ dbMgr.ensureFreeSpaceForInsert(dataRegion, row.size() -
written);
+
+ pageId = takePage(row.size() - written, row, statHolder);
+ }
+
if (pageId == 0L) {
pageId = allocateDataPage(row.partition());
Review Comment:
There is also insertDataRows method where page can be requested on rows
insert and oom can be fired.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/RowStore.java:
##########
@@ -132,8 +133,25 @@ public void addRow(CacheDataRow row, IoStatisticsHolder
statHolder) throws Ignit
* @param statHolder Statistics holder to track IO operations.
* @throws IgniteCheckedException If failed.
*/
- public void addRows(Collection<? extends CacheDataRow> rows,
- IoStatisticsHolder statHolder) throws IgniteCheckedException {
+ public void addRows(Collection<? extends CacheDataRow> rows,
IoStatisticsHolder statHolder) throws IgniteCheckedException {
Review Comment:
addRows is only executed on rebalance. Why do we need this new code for
rebalance but not for regular put?
> reserving only the largest is insufficient
But you do exactly the same. Ensure free space for each row before any row
is inserted, so only free space for largest row will be ensured. Other rows
reservation will go through writeSinglePage.
##########
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:
Why this method not in PageEvictionTracker?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/freelist/AbstractFreeList.java:
##########
@@ -701,10 +713,29 @@ private int writeWholePages(T row, IoStatisticsHolder
statHolder) throws IgniteC
* @throws IgniteCheckedException If failed.
*/
private int writeSinglePage(T row, int written, IoStatisticsHolder
statHolder) throws IgniteCheckedException {
+ // TOCTOU closure: the size-aware reserve (ensureFreeSpaceForInsert,
invoked from RowStore.addRow/addRows
+ // before this write) accumulates enough real empty pages but does not
pin them to this thread - a concurrent
+ // writer can consume them between the reserve and this allocation.
When the free list cannot hand out a page,
+ // re-reserve on the remaining size and retry before allocating a
brand-new page; otherwise the race surfaces
+ // as a raw IgniteOutOfMemoryException (wrapped into
CorruptedFreeListException in the batch path).
+ //
+ // The re-reserve is an inline demand-eviction: reached from the
BPlusTree.invoke row-creation closure, it may
+ // re-entrantly remove other entries from the same data tree. That is
safe because the closure runs with no
Review Comment:
> That is safe because the closure runs with no data-tree page locks held.
Are you sure about this statement?
As far as I know BPlusTree.invoke holds the write lock on leaf page while
execution the closure, so it's not deadlock safe. Deadlock is possible between
data-tree leaf pages since page can contain more than one entry (acquired entry
lock is not enough protection).
Also, as far as I understand there can be deadlock on expiration: Expiration
thread holds write lock on pending tree leaf page and reads data tree (with
read lock). While addRow thread holds write lock on data tree leaf page and can
concurrently evict data (remove rows) which remove ttl entries and requires
write lock on pending tree leaf page.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/evict/PageAbstractEvictionTracker.java:
##########
@@ -41,6 +41,13 @@ public abstract class PageAbstractEvictionTracker implements
PageEvictionTracker
/** Millis in day. */
private static final int DAY = 24 * 60 * 60 * 1000;
+ /**
+ * Thread-local marker that the current eviction is requested by
size-aware eviction, which may run
+ * while the calling thread already holds entry locks. When set, entries
whose locks are contended are skipped
+ * (via a non-blocking {@code evictInternal}) instead of blocking,
avoiding a lock-ordering deadlock.
+ */
+ private static final ThreadLocal<Boolean> EVICT_NON_BLOCKING = new
ThreadLocal<>();
Review Comment:
It's strange to use thread-local to pass parameter to next stack level.
Let's use
evictDataPage(boolean blocking) -> evictDataPage(int pageIdx, boolean
blocking)
Instead of
evictDataPageNonBlocking -> set thread-local -> evictDataPage() ->
evictDataPage(int pageIdx) -> get thread local.
##########
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))
Review Comment:
Eviction always started when pagesThreshold is exceeded and there is not
enough empty pages. So page memory will never exceeds pagesThreshold (by
default 10% of page memory will be unusable). Looks strange.
--
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]
