Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CBRD-21100] Do not deregister "avoid deallocation" on victimized bcb #611

Merged
merged 2 commits into from Mar 29, 2017
Merged

[CBRD-21100] Do not deregister "avoid deallocation" on victimized bcb #611

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66c2b63
page unfixed by pgbuf_ordered_fix can be victimized; update code to r…
ribram Mar 29, 2017
b82de19
do not crash server when avoid_deallocation is positive
ribram Mar 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
113 changes: 70 additions & 43 deletions src/storage/page_buffer.c
View file
Open in desktop
Expand Up @@ -1180,6 +1180,8 @@ STATIC_INLINE void pgbuf_bcb_mark_was_not_flushed (THREAD_ENTRY * thread_p, PGBU
STATIC_INLINE void pgbuf_bcb_register_avoid_deallocation (PGBUF_BCB * bcb) __attribute__ ((ALWAYS_INLINE));
STATIC_INLINE void pgbuf_bcb_unregister_avoid_deallocation (PGBUF_BCB * bcb) __attribute__ ((ALWAYS_INLINE));
STATIC_INLINE bool pgbuf_bcb_should_avoid_deallocation (const PGBUF_BCB * bcb) __attribute__ ((ALWAYS_INLINE));
STATIC_INLINE void pgbuf_bcb_check_and_reset_fix_and_avoid_dealloc (PGBUF_BCB * bcb, const char *file, int line)
__attribute__ ((ALWAYS_INLINE));
STATIC_INLINE void pgbuf_bcb_register_fix (PGBUF_BCB * bcb) __attribute__ ((ALWAYS_INLINE));
STATIC_INLINE bool pgbuf_bcb_is_hot (const PGBUF_BCB * bcb) __attribute__ ((ALWAYS_INLINE));

Expand Down Expand Up @@ -7096,8 +7098,7 @@ pgbuf_delete_from_hash_chain (THREAD_ENTRY * thread_p, PGBUF_BCB * bufptr)
curr_bufptr->hash_next = NULL;
pthread_mutex_unlock (&hash_anchor->hash_mutex);
VPID_SET_NULL (&(bufptr->vpid));
assert ((bufptr->count_fix_and_avoid_dealloc & PGBUF_BCB_AVOID_DEALLOC_MASK) == 0);
bufptr->count_fix_and_avoid_dealloc = 0;
pgbuf_bcb_check_and_reset_fix_and_avoid_dealloc (bufptr, ARG_FILE_LINE);

return NO_ERROR;
}
Expand Down Expand Up @@ -7593,8 +7594,7 @@ pgbuf_claim_bcb_for_fix (THREAD_ENTRY * thread_p, const VPID * vpid, PAGE_FETCH_
assert (!pgbuf_bcb_avoid_victim (bufptr));
bufptr->latch_mode = PGBUF_NO_LATCH;
pgbuf_bcb_update_flags (thread_p, bufptr, 0, PGBUF_BCB_ASYNC_FLUSH_REQ); /* todo: why this?? */
assert ((bufptr->count_fix_and_avoid_dealloc & PGBUF_BCB_AVOID_DEALLOC_MASK) == 0);
bufptr->count_fix_and_avoid_dealloc = 0;
pgbuf_bcb_check_and_reset_fix_and_avoid_dealloc (bufptr, ARG_FILE_LINE);
LSA_SET_NULL (&bufptr->oldest_unflush_lsa);

if (fetch_mode != NEW_PAGE)
Expand Down Expand Up @@ -8025,8 +8025,7 @@ pgbuf_put_bcb_into_invalid_list (THREAD_ENTRY * thread_p, PGBUF_BCB * bufptr)
bufptr->latch_mode = PGBUF_LATCH_INVALID;
assert ((bufptr->flags & PGBUF_BCB_FLAGS_MASK) == 0);
pgbuf_bcb_change_zone (thread_p, bufptr, 0, PGBUF_INVALID_ZONE);
assert ((bufptr->count_fix_and_avoid_dealloc & PGBUF_BCB_AVOID_DEALLOC_MASK) == 0);
bufptr->count_fix_and_avoid_dealloc = 0;
pgbuf_bcb_check_and_reset_fix_and_avoid_dealloc (bufptr, ARG_FILE_LINE);

rv = pthread_mutex_lock (&pgbuf_Pool.buf_invalid_list.invalid_mutex);
bufptr->next_BCB = pgbuf_Pool.buf_invalid_list.invalid_top;
Expand Down Expand Up @@ -12118,38 +12117,24 @@ pgbuf_ordered_fix_release (THREAD_ENTRY * thread_p, const VPID * req_vpid, PAGE_
er_status = er_errid ();
if (er_status == ER_INTERRUPTED)
{
/* this is expected */
goto exit;
}

valid =
pgbuf_is_valid_page (thread_p, &(ordered_holders_info[i].vpid),
VPID_EQ (req_vpid, &(ordered_holders_info[i].vpid)), NULL, NULL);
#if defined(PGBUF_ORDERED_DEBUG)
_er_log_debug (__FILE__, __LINE__,
"ORDERED_FIX(%u): restore_pages_ERR: cannot fix VPID:(%d,%d), "
"GROUP:%d,%d, rank:%d, page_fetch_mode:%d , latch_req: %d, valid:%d", ordered_fix_id,
ordered_holders_info[i].vpid.volid, ordered_holders_info[i].vpid.pageid,
ordered_holders_info[i].group_id.volid, ordered_holders_info[i].group_id.pageid,
ordered_holders_info[i].rank, curr_fetch_mode, curr_request_mode, valid);
#endif
if (valid == DISK_INVALID)
if (er_status == ER_PB_BAD_PAGEID)
{
er_set (ER_FATAL_ERROR_SEVERITY, ARG_FILE_LINE, ER_PB_BAD_PAGEID, 2, ordered_holders_info[i].vpid.pageid,
fileio_get_volume_label (ordered_holders_info[i].vpid.volid, PEEK));
/* page was probably deallocated? so has the impossible indeed happen?? */
assert (false);
er_log_debug (ARG_FILE_LINE, "pgbuf_ordered_fix: page %d|%d was deallocated an we told it not to!\n",
VPID_AS_ARGS (&ordered_holders_info[i].vpid));
}

er_status = er_errid ();

if (!VPID_EQ (req_vpid, &(ordered_holders_info[i].vpid)))
{
int prev_er_status = er_status;
er_status = ER_PB_ORDERED_REFIX_FAILED;
er_set (ER_ERROR_SEVERITY, ARG_FILE_LINE, er_status, 3, ordered_holders_info[i].vpid.volid,
ordered_holders_info[i].vpid.pageid, prev_er_status);
goto exit;
}
/* try to restore fixes on all remaining pages */
continue;
goto exit;
}

/* get holder of last fix: last fixed pages is in top of holder list, we use parse code just for safety */
Expand Down Expand Up @@ -15072,14 +15057,8 @@ pgbuf_bcb_get_pool_index (const PGBUF_BCB * bcb)
STATIC_INLINE void
pgbuf_bcb_register_avoid_deallocation (PGBUF_BCB * bcb)
{
#if !defined (NDEBUG)
int newval =
#endif /* !NDEBUG */
ATOMIC_INC_32 (&bcb->count_fix_and_avoid_dealloc, 1);
assert (newval > 0);
assert ((newval & PGBUF_BCB_AVOID_DEALLOC_MASK) > 0);
assert (bcb->count_fix_and_avoid_dealloc > 0);
assert ((bcb->count_fix_and_avoid_dealloc & PGBUF_BCB_AVOID_DEALLOC_MASK) > 0);
assert ((bcb->count_fix_and_avoid_dealloc & 0x00008000) == 0);
(void) ATOMIC_INC_32 (&bcb->count_fix_and_avoid_dealloc, 1);
}

/*
Expand All @@ -15091,14 +15070,39 @@ pgbuf_bcb_register_avoid_deallocation (PGBUF_BCB * bcb)
STATIC_INLINE void
pgbuf_bcb_unregister_avoid_deallocation (PGBUF_BCB * bcb)
{
#if !defined (NDEBUG)
int newval =
#endif /* !NDEBUG */
ATOMIC_INC_32 (&bcb->count_fix_and_avoid_dealloc, -1);
assert (newval >= 0);
assert ((newval & PGBUF_BCB_AVOID_DEALLOC_MASK) != 0xFFFF);
assert (bcb->count_fix_and_avoid_dealloc >= 0);
assert ((bcb->count_fix_and_avoid_dealloc & PGBUF_BCB_AVOID_DEALLOC_MASK) != 0xFFFF);
int count_crt;
do
{
/* get bcb->count_fix_and_avoid_dealloc (volatile) */
count_crt = bcb->count_fix_and_avoid_dealloc;
assert ((count_crt & 0x00008000) == 0);
if ((count_crt & PGBUF_BCB_AVOID_DEALLOC_MASK) > 0)
{
/* we can decrement counter */
}
else
{
/* interestingly enough, this case can happen. how?
*
* well, pgbuf_ordered_fix may be forced to unfix all pages currently held by transaction to fix a new page.
* all pages that are "less" than new page are marked to avoid deallocation and unfixed. then transaction is
* blocked on latching new page, which may take a while, pages previously unfixed can be victimized.
* when pgbuf_ordered_fix tries to fix back these pages, it will load them from disk and tadaa, the avoid
* deallocation count is 0. so we expect the case.
*
* note: avoid deallocation count is supposed to prevent vacuum workers from deallocating these pages.
* so, victimizing a bcb marked to avoid deallocation is not perfectly safe. however, the likelihood of
* page really getting deallocated is ... almost zero. the alternative of avoiding victimization when
* bcb's are marked for deallocation is much more complicated and poses serious risks (what if we leak
* the counter and prevent bcb from being victimized indefinitely?). so, we prefer the existing risks.
*/
er_log_debug (ARG_FILE_LINE,
"pgbuf_bcb_unregister_avoid_deallocation: bcb %p, vpid = %d|%d was probably victimized.\n",
bcb, VPID_AS_ARGS (&bcb->vpid));
break;
}
}
while (!ATOMIC_CAS_32 (&bcb->count_fix_and_avoid_dealloc, count_crt, count_crt - 1));
}

/*
Expand All @@ -15111,9 +15115,32 @@ STATIC_INLINE bool
pgbuf_bcb_should_avoid_deallocation (const PGBUF_BCB * bcb)
{
assert (bcb->count_fix_and_avoid_dealloc >= 0);
assert ((bcb->count_fix_and_avoid_dealloc & 0x00008000) == 0);
return (bcb->count_fix_and_avoid_dealloc & PGBUF_BCB_AVOID_DEALLOC_MASK) != 0;
}

/*
* pgbuf_bcb_check_and_reset_fix_and_avoid_dealloc () - check avoid deallocation is 0 and reset the whole bcb field.
*
* return : void
* bcb (in) : bcb
* file (in) : caller file
* line (in) : caller line
*
* note: avoid deallocation is allowed to be non-zero due to pgbuf_ordered_fix and the possibility of victimizing its
* bcb. avoid crashing the server and just issue a warning.
*/
STATIC_INLINE void
pgbuf_bcb_check_and_reset_fix_and_avoid_dealloc (PGBUF_BCB * bcb, const char *file, int line)
{
if (pgbuf_bcb_should_avoid_deallocation (bcb))
{
er_log_debug (file, line, "warning: bcb %p, vpid = %d|%d, should not have avoid deallocation marker.\n",
bcb, VPID_AS_ARGS (&bcb->vpid));
}
bcb->count_fix_and_avoid_dealloc = 0;
}

/*
* pgbuf_bcb_register_fix () - register page fix
*
Expand Down