Details
-
Bug
-
Status: In Review (View Workflow)
-
Critical
-
Resolution: Unresolved
-
10.11
-
Related to performance
Description
buf_page_peek_if_young() does not properly compute and wrap the age of the page, which causes the function to determine some pages being "young" when in fact (considering the 31-bits accuracy of the page clock limit absolute correctness) are old.
The code of the function has this reported has FIXME:
/** Determine if a block is still close enough to the MRU end of the LRU list
|
meaning that it is not in danger of getting evicted and also implying
|
that it has been accessed recently.
|
The page must be either buffer-fixed, or its page hash must be locked.
|
@param[in] bpage buffer pool page
|
@return whether bpage is close to MRU end of LRU */
|
inline bool buf_page_peek_if_young(const buf_page_t *bpage)
|
{
|
/* FIXME: bpage->freed_page_clock is 31 bits */
|
return((buf_pool.freed_page_clock & ((1UL << 31) - 1))
|
< (bpage->freed_page_clock
|
+ (buf_pool.curr_size()
|
* (BUF_LRU_OLD_RATIO_DIV - buf_pool.LRU_old_ratio)
|
/ (BUF_LRU_OLD_RATIO_DIV * 4))));
|
}
|
I wrote a test program to simulate error conditions for the current version and a plausible fix:
#include <array>
|
#include <cstdio>
|
#include <cstdint>
|
|
|
/*
|
* inline bool buf_page_peek_if_young(const buf_page_t *bpage)
|
* {
|
* return((buf_pool.freed_page_clock & ((1UL << 31) - 1))
|
* < (bpage->freed_page_clock
|
* + (buf_pool.curr_size()
|
* (BUF_LRU_OLD_RATIO_DIV - buf_pool.LRU_old_ratio)
|
* / (BUF_LRU_OLD_RATIO_DIV * 4))));
|
* }
|
*/
|
|
|
constexpr uint32_t CLOCK_MASK= ((1UL << 31) - 1);
|
static_assert(CLOCK_MASK == 0x7FFFFFFF);
|
|
|
inline bool f_current_young(const uint32_t bp_fpc, const uint32_t stamped, const size_t window)
|
{
|
const uint32_t now= bp_fpc & CLOCK_MASK;
|
static_assert(sizeof((stamped + window)) == sizeof(size_t)); /* promotion */
|
return now < (stamped + window);
|
}
|
|
|
inline bool f_new_young(const uint32_t bp_fpc, const uint32_t stamped, const size_t window)
|
{
|
const uint32_t now= bp_fpc & CLOCK_MASK;
|
const uint32_t age= (now - stamped) & CLOCK_MASK;
|
return age < window;
|
}
|
|
|
int main()
|
{
|
constexpr size_t window= 10000;
|
const std::array<uint32_t, 7> stampeds= {0, 1, window - 1, window, window + 1, CLOCK_MASK / 2, CLOCK_MASK - 1};
|
|
|
for (const uint32_t stamped : stampeds)
|
{
|
printf("stamped=%u (0x%08X)\n", stamped, stamped);
|
bool disagree= false;
|
uint64_t disagree_count= 0;
|
|
|
for (uint32_t bp_fpc= 0; ; ++bp_fpc)
|
{
|
const bool current_young= f_current_young(bp_fpc, stamped, window);
|
const bool new_young= f_new_young(bp_fpc, stamped, window);
|
if (current_young != new_young)
|
{
|
++disagree_count;
|
if (!disagree)
|
{
|
disagree= true;
|
printf("Disagree started at bp_fpc=%u (0x%08X), stamped=%u (0x%08X), window=%zu: current_young=%d != new_young=%d\n",
|
bp_fpc, bp_fpc, stamped, stamped, window, current_young, new_young);
|
}
|
}
|
else if (current_young == new_young && disagree)
|
{
|
disagree= false;
|
printf("Disagree stopped at bp_fpc=%u (0x%08X), stamped=%u (0x%08X), window=%zu: current_young=%d == new_young=%d\n",
|
bp_fpc, bp_fpc, stamped, stamped, window, current_young, new_young);
|
}
|
|
|
if (bp_fpc == 0xFFFFFFFF)
|
{
|
if (disagree)
|
{
|
printf("Disagreed up to the end\n");
|
}
|
break;
|
}
|
}
|
printf("stamped=%u (0x%08X), disagree_count=%lu\n\n", stamped, stamped, disagree_count);
|
}
|
|
|
return 0;
|
}
|
The output is (considering the window size at 10000):
stamped=0 (0x00000000)
|
stamped=0 (0x00000000), disagree_count=0
|
|
|
stamped=1 (0x00000001)
|
Disagree started at bp_fpc=0 (0x00000000), stamped=1 (0x00000001), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=1 (0x00000001), stamped=1 (0x00000001), window=10000: current_young=1 == new_young=1
|
Disagree started at bp_fpc=2147483648 (0x80000000), stamped=1 (0x00000001), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=2147483649 (0x80000001), stamped=1 (0x00000001), window=10000: current_young=1 == new_young=1
|
stamped=1 (0x00000001), disagree_count=2
|
|
|
stamped=9999 (0x0000270F)
|
Disagree started at bp_fpc=0 (0x00000000), stamped=9999 (0x0000270F), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=9999 (0x0000270F), stamped=9999 (0x0000270F), window=10000: current_young=1 == new_young=1
|
Disagree started at bp_fpc=2147483648 (0x80000000), stamped=9999 (0x0000270F), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=2147493647 (0x8000270F), stamped=9999 (0x0000270F), window=10000: current_young=1 == new_young=1
|
stamped=9999 (0x0000270F), disagree_count=19998
|
|
|
stamped=10000 (0x00002710)
|
Disagree started at bp_fpc=0 (0x00000000), stamped=10000 (0x00002710), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=10000 (0x00002710), stamped=10000 (0x00002710), window=10000: current_young=1 == new_young=1
|
Disagree started at bp_fpc=2147483648 (0x80000000), stamped=10000 (0x00002710), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=2147493648 (0x80002710), stamped=10000 (0x00002710), window=10000: current_young=1 == new_young=1
|
stamped=10000 (0x00002710), disagree_count=20000
|
|
|
stamped=10001 (0x00002711)
|
Disagree started at bp_fpc=0 (0x00000000), stamped=10001 (0x00002711), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=10001 (0x00002711), stamped=10001 (0x00002711), window=10000: current_young=1 == new_young=1
|
Disagree started at bp_fpc=2147483648 (0x80000000), stamped=10001 (0x00002711), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=2147493649 (0x80002711), stamped=10001 (0x00002711), window=10000: current_young=1 == new_young=1
|
stamped=10001 (0x00002711), disagree_count=20002
|
|
|
stamped=1073741823 (0x3FFFFFFF)
|
Disagree started at bp_fpc=0 (0x00000000), stamped=1073741823 (0x3FFFFFFF), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=1073741823 (0x3FFFFFFF), stamped=1073741823 (0x3FFFFFFF), window=10000: current_young=1 == new_young=1
|
Disagree started at bp_fpc=2147483648 (0x80000000), stamped=1073741823 (0x3FFFFFFF), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=3221225471 (0xBFFFFFFF), stamped=1073741823 (0x3FFFFFFF), window=10000: current_young=1 == new_young=1
|
stamped=1073741823 (0x3FFFFFFF), disagree_count=2147483646
|
|
|
stamped=2147483646 (0x7FFFFFFE)
|
Disagree started at bp_fpc=9998 (0x0000270E), stamped=2147483646 (0x7FFFFFFE), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=2147483646 (0x7FFFFFFE), stamped=2147483646 (0x7FFFFFFE), window=10000: current_young=1 == new_young=1
|
Disagree started at bp_fpc=2147493646 (0x8000270E), stamped=2147483646 (0x7FFFFFFE), window=10000: current_young=1 != new_young=0
|
Disagree stopped at bp_fpc=4294967294 (0xFFFFFFFE), stamped=2147483646 (0x7FFFFFFE), window=10000: current_young=1 == new_young=1
|
stamped=2147483646 (0x7FFFFFFE), disagree_count=4294947296
|
We can see that the error is always current function telling "young" while the new one does not.
Sample error condition (discovered while investigating MDEV-40050, comment here):
stamped = 2**31 - 10
|
now (stamped + 2010, wrapped, masked to 31 bits) = 2000
|
window = 1000
|
true age = 2010 -> truly old
|
|
|
would make the current code tell:
|
|
|
2000 < (2**31 - 10) + 1000 => 2000 < some big number => true => young (wrong!)
|
|
|
fixed code:
|
|
|
(2000 - (2**31 - 10)) & CLOCK_MASK < 1000 => (2147485658 & CLOCK_MASK) < 1000 => 2010 < 1000 => false => old (correct)
|
Mistakenly marking pages as "young" can make hot pages slip into the old portion of the list.
Attachments
Issue Links
- relates to
-
PERF-552 Loading...
