Conversation
…p page age in young calculation buf_page_t::freed_page_clock is a 31-bit bitfield holding the low bits of buf_pool.freed_page_clock. buf_page_peek_if_young() evaluated the condition now < stamp + window in size_t arithmetic, which failed to account for wraparound within the 31-bit clock space. When the global clock wrapped past a stamp (particularly when stamp was high, near 2^31), the comparison evaluated to true for an extended period even though the page was older than the window. During these wraparound windows, pages were falsely reported as young. Because returning true causes InnoDB to skip moving pages to the MRU head (to reduce lock contention), these falsely "young" pages were not promoted. As a result, active/hot pages could sink into the old portion of the LRU list and be evicted prematurely. Additionally, buf_read_ahead_random() over-counted recently accessed pages during wraparound intervals. Compute age as (now - stamp) & clock_mask using 31-bit modular arithmetic and compare age < window. A page then cleanly leaves the young window after the intended number of evictions regardless of clock wraparound.
|
|
There was a problem hiding this comment.
🟡 Changes recommended
The wraparound and threshold boundaries need regression coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes InnoDB LRU youth detection across the 31-bit eviction-clock wraparound.
Changes:
- Computes page age using 31-bit modular subtraction.
- Preserves the existing young-window threshold.
File summaries
| File | Description |
|---|---|
storage/innobase/include/buf0buf.inl |
Corrects wraparound handling in buf_page_peek_if_young(). |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| buf_pool.freed_page_clock, so the age must be computed modulo 2^31. */ | ||
| constexpr uint32_t CLOCK_MASK= (1U << 31) - 1; | ||
| const uint32_t now= uint32_t(buf_pool.freed_page_clock) & CLOCK_MASK; | ||
| const uint32_t age= (now - uint32_t(bpage->freed_page_clock)) & CLOCK_MASK; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…p page age in young calculation
buf_page_t::freed_page_clock is a 31-bit bitfield holding the low bits of buf_pool.freed_page_clock. buf_page_peek_if_young() evaluated the condition now < stamp + window in size_t arithmetic, which failed to account for wraparound within the 31-bit clock space.
When the global clock wrapped past a stamp (particularly when stamp was high, near 2^31), the comparison evaluated to true for an extended period even though the page was older than the window. During these wraparound windows, pages were falsely reported as young.
Because returning true causes InnoDB to skip moving pages to the MRU head (to reduce lock contention), these falsely "young" pages were not promoted.
As a result, active/hot pages could sink into the old portion of the LRU list and be evicted prematurely. Additionally, buf_read_ahead_random() over-counted recently accessed pages during wraparound intervals.
Compute age as (now - stamp) & clock_mask using 31-bit modular arithmetic and compare age < window. A page then cleanly leaves the young window after the intended number of evictions regardless of clock wraparound.