Skip to content

Commit

Permalink
MDEV-20788: Bogus assertion failure for PAGE_FREE list
Browse files Browse the repository at this point in the history 
In MDEV-11369 (instant ADD COLUMN) in MariaDB Server 10.3,
we introduced the hidden metadata record that must be the
first record in the clustered index if and only if
index->is_instant() holds.

To catch MDEV-19783, in
commit ed0793e and
commit 99dc40d
we added some assertions to find cases where
the metadata record is missing while it should not be, or a
record exists when it should not. Those assertions were invalid
when traversing the PAGE_FREE list. That list can contain anything;
we must only be able to determine the successor and the size of
each garbage record in it.

page_validate(), page_simple_validate_old(), page_simple_validate_new():
Do not invoke page_rec_get_next_const() for traversing the PAGE_FREE
list, but instead use a lower-level accessor that does not attempt to
validate the REC_INFO_MIN_REC_FLAG.

page_copy_rec_list_end_no_locks(),
page_copy_rec_list_start(), page_delete_rec_list_start():
Add assertions.

btr_page_get_split_rec_to_left(): Remove a redundant return value,
and make the output parameter the return value.

btr_page_get_split_rec_to_right(), btr_page_split_and_insert(): Clean up.
  • Loading branch information
@dr-m
dr-m committed Oct 10, 2019
1 parent 726b199 commit 6d7a826
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 168 deletions.
194 changes: 68 additions & 126 deletions storage/innobase/btr/btr0btr.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2030,104 +2030,78 @@ btr_root_raise_and_insert(
}
}

/*************************************************************//**
Decides if the page should be split at the convergence point of inserts
/** Decide if the page should be split at the convergence point of inserts
converging to the left.
@return TRUE if split recommended */
ibool
btr_page_get_split_rec_to_left(
/*===========================*/
btr_cur_t* cursor, /*!< in: cursor at which to insert */
rec_t** split_rec) /*!< out: if split recommended,
the first record on upper half page,
or NULL if tuple to be inserted should
be first */
@param[in] cursor insert position
@return the first record to be moved to the right half page
@retval NULL if no split is recommended */
rec_t* btr_page_get_split_rec_to_left(const btr_cur_t* cursor)
{
page_t* page;
rec_t* insert_point;
rec_t* infimum;

page = btr_cur_get_page(cursor);
insert_point = btr_cur_get_rec(cursor);
rec_t* split_rec = btr_cur_get_rec(cursor);
const page_t* page = page_align(split_rec);

if (page_header_get_ptr(page, PAGE_LAST_INSERT)
== page_rec_get_next(insert_point)) {

infimum = page_get_infimum_rec(page);

/* If the convergence is in the middle of a page, include also
the record immediately before the new insert to the upper
page. Otherwise, we could repeatedly move from page to page
lots of records smaller than the convergence point. */
!= page_rec_get_next(split_rec)) {
return NULL;
}

if (infimum != insert_point
&& page_rec_get_next(infimum) != insert_point) {
const rec_t* infimum = page_get_infimum_rec(page);

*split_rec = insert_point;
} else {
*split_rec = page_rec_get_next(insert_point);
}
/* If the convergence is in the middle of a page, include also
the record immediately before the new insert to the upper
page. Otherwise, we could repeatedly move from page to page
lots of records smaller than the convergence point. */

return(TRUE);
if (split_rec == infimum
|| split_rec == page_rec_get_next_const(infimum)) {
split_rec = page_rec_get_next(split_rec);
}

return(FALSE);
return split_rec;
}

/*************************************************************//**
Decides if the page should be split at the convergence point of inserts
/** Decide if the page should be split at the convergence point of inserts
converging to the right.
@return TRUE if split recommended */
ibool
btr_page_get_split_rec_to_right(
/*============================*/
btr_cur_t* cursor, /*!< in: cursor at which to insert */
rec_t** split_rec) /*!< out: if split recommended,
the first record on upper half page,
or NULL if tuple to be inserted should
be first */
@param[in] cursor insert position
@param[out] split_rec if split recommended, the first record
on the right half page, or
NULL if the to-be-inserted record
should be first
@return whether split is recommended */
bool
btr_page_get_split_rec_to_right(const btr_cur_t* cursor, rec_t** split_rec)
{
page_t* page;
rec_t* insert_point;

page = btr_cur_get_page(cursor);
insert_point = btr_cur_get_rec(cursor);
rec_t* insert_point = btr_cur_get_rec(cursor);
const page_t* page = page_align(insert_point);

/* We use eager heuristics: if the new insert would be right after
the previous insert on the same page, we assume that there is a
pattern of sequential inserts here. */

if (page_header_get_ptr(page, PAGE_LAST_INSERT) == insert_point) {

rec_t* next_rec;

next_rec = page_rec_get_next(insert_point);

if (page_rec_is_supremum(next_rec)) {
split_at_new:
/* Split at the new record to insert */
*split_rec = NULL;
} else {
rec_t* next_next_rec = page_rec_get_next(next_rec);
if (page_rec_is_supremum(next_next_rec)) {

goto split_at_new;
}
if (page_header_get_ptr(page, PAGE_LAST_INSERT) != insert_point) {
return false;
}

/* If there are >= 2 user records up from the insert
point, split all but 1 off. We want to keep one because
then sequential inserts can use the adaptive hash
index, as they can do the necessary checks of the right
search position just by looking at the records on this
page. */
insert_point = page_rec_get_next(insert_point);

*split_rec = next_next_rec;
if (page_rec_is_supremum(insert_point)) {
insert_point = NULL;
} else {
insert_point = page_rec_get_next(insert_point);
if (page_rec_is_supremum(insert_point)) {
insert_point = NULL;
}

return(TRUE);
/* If there are >= 2 user records up from the insert
point, split all but 1 off. We want to keep one because
then sequential inserts can use the adaptive hash
index, as they can do the necessary checks of the right
search position just by looking at the records on this
page. */
}

return(FALSE);
*split_rec = insert_point;
return true;
}

/*************************************************************//**
Expand Down Expand Up @@ -2782,30 +2756,20 @@ btr_page_split_and_insert(
buf_block_t* block;
page_t* page;
page_zip_des_t* page_zip;
ulint page_no;
byte direction;
ulint hint_page_no;
buf_block_t* new_block;
page_t* new_page;
page_zip_des_t* new_page_zip;
rec_t* split_rec;
buf_block_t* left_block;
buf_block_t* right_block;
buf_block_t* insert_block;
page_cur_t* page_cursor;
rec_t* first_rec;
byte* buf = 0; /* remove warning */
rec_t* move_limit;
ibool insert_will_fit;
ibool insert_left;
ulint n_iterations = 0;
rec_t* rec;
ulint n_uniq;
dict_index_t* index;

index = btr_cur_get_index(cursor);

if (dict_index_is_spatial(index)) {
if (dict_index_is_spatial(cursor->index)) {
/* Split rtree page and update parent */
return(rtr_page_split_and_insert(flags, cursor, offsets, heap,
tuple, n_ext, mtr));
Expand Down Expand Up @@ -2837,41 +2801,30 @@ btr_page_split_and_insert(
ut_ad(!page_is_empty(page));

/* try to insert to the next page if possible before split */
rec = btr_insert_into_right_sibling(
flags, cursor, offsets, *heap, tuple, n_ext, mtr);

if (rec != NULL) {
if (rec_t* rec = btr_insert_into_right_sibling(
flags, cursor, offsets, *heap, tuple, n_ext, mtr)) {
return(rec);
}

page_no = block->page.id.page_no();

/* 1. Decide the split record; split_rec == NULL means that the
tuple to be inserted should be the first record on the upper
half-page */
insert_left = FALSE;
bool insert_left = false;
ulint hint_page_no = block->page.id.page_no() + 1;
byte direction = FSP_UP;

if (tuple != NULL && n_iterations > 0) {
direction = FSP_UP;
hint_page_no = page_no + 1;
if (tuple && n_iterations > 0) {
split_rec = btr_page_get_split_rec(cursor, tuple, n_ext);

if (split_rec == NULL) {
insert_left = btr_page_tuple_smaller(
cursor, tuple, offsets, n_uniq, heap);
}
} else if (btr_page_get_split_rec_to_right(cursor, &split_rec)) {
direction = FSP_UP;
hint_page_no = page_no + 1;

} else if (btr_page_get_split_rec_to_left(cursor, &split_rec)) {
} else if ((split_rec = btr_page_get_split_rec_to_left(cursor))) {
direction = FSP_DOWN;
hint_page_no = page_no - 1;
ut_ad(split_rec);
hint_page_no -= 2;
} else {
direction = FSP_UP;
hint_page_no = page_no + 1;

/* If there is only one record in the index page, we
can't split the node in the middle by default. We need
to determine whether the new record will be inserted
Expand All @@ -2896,7 +2849,7 @@ btr_page_split_and_insert(
new_block = btr_page_alloc(cursor->index, hint_page_no, direction,
btr_page_get_level(page, mtr), mtr, mtr);

if (new_block == NULL && os_has_said_disk_full) {
if (!new_block) {
return(NULL);
}

Expand All @@ -2921,12 +2874,8 @@ btr_page_split_and_insert(
*offsets = rec_get_offsets(split_rec, cursor->index, *offsets,
page_is_leaf(page), n_uniq, heap);

if (tuple != NULL) {
insert_left = cmp_dtuple_rec(
tuple, split_rec, *offsets) < 0;
} else {
insert_left = 1;
}
insert_left = !tuple
|| cmp_dtuple_rec(tuple, split_rec, *offsets) < 0;

if (!insert_left && new_page_zip && n_iterations > 0) {
/* If a compressed page has already been split,
Expand Down Expand Up @@ -2961,10 +2910,10 @@ btr_page_split_and_insert(
on the appropriate half-page, we may release the tree x-latch.
We can then move the records after releasing the tree latch,
thus reducing the tree latch contention. */
bool insert_will_fit;
if (tuple == NULL) {
insert_will_fit = 1;
}
else if (split_rec) {
insert_will_fit = true;
} else if (split_rec) {
insert_will_fit = !new_page_zip
&& btr_page_insert_fits(cursor, split_rec,
offsets, tuple, n_ext, heap);
Expand Down Expand Up @@ -3069,8 +3018,6 @@ btr_page_split_and_insert(
new_block, block, move_limit);
}

ut_ad(!dict_index_is_spatial(index));

btr_search_move_or_delete_hash_entries(
new_block, block, cursor->index);

Expand Down Expand Up @@ -3102,18 +3049,15 @@ btr_page_split_and_insert(

/* 6. The split and the tree modification is now completed. Decide the
page where the tuple should be inserted */
rec_t* rec;
buf_block_t* const insert_block = insert_left
? left_block : right_block;

if (tuple == NULL) {
if (UNIV_UNLIKELY(!tuple)) {
rec = NULL;
goto func_exit;
}

if (insert_left) {
insert_block = left_block;
} else {
insert_block = right_block;
}

/* 7. Reposition the cursor for insert and try insertion */
page_cursor = btr_cur_get_page_cur(cursor);

Expand Down Expand Up @@ -3190,9 +3134,7 @@ btr_page_split_and_insert(
ut_ad(page_validate(buf_block_get_frame(left_block), cursor->index));
ut_ad(page_validate(buf_block_get_frame(right_block), cursor->index));

if (tuple == NULL) {
ut_ad(rec == NULL);
}
ut_ad(tuple || !rec);
ut_ad(!rec || rec_offs_validate(rec, cursor->index, *offsets));
return(rec);
}
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/btr/btr0cur.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3138,7 +3138,7 @@ btr_cur_optimistic_insert(
&& page_get_n_recs(page) >= 2
&& dict_index_get_space_reserve() + rec_size > max_size
&& (btr_page_get_split_rec_to_right(cursor, &dummy)
|| btr_page_get_split_rec_to_left(cursor, &dummy))) {
|| btr_page_get_split_rec_to_left(cursor))) {
goto fail;
}

Expand Down
40 changes: 16 additions & 24 deletions storage/innobase/include/btr0btr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,30 +445,22 @@ btr_page_reorganize(
dict_index_t* index, /*!< in: the index tree of the page */
mtr_t* mtr) /*!< in/out: mini-transaction */
MY_ATTRIBUTE((nonnull));
/*************************************************************//**
Decides if the page should be split at the convergence point of
inserts converging to left.
@return TRUE if split recommended */
ibool
btr_page_get_split_rec_to_left(
/*===========================*/
btr_cur_t* cursor, /*!< in: cursor at which to insert */
rec_t** split_rec)/*!< out: if split recommended,
the first record on upper half page,
or NULL if tuple should be first */
MY_ATTRIBUTE((warn_unused_result));
/*************************************************************//**
Decides if the page should be split at the convergence point of
inserts converging to right.
@return TRUE if split recommended */
ibool
btr_page_get_split_rec_to_right(
/*============================*/
btr_cur_t* cursor, /*!< in: cursor at which to insert */
rec_t** split_rec)/*!< out: if split recommended,
the first record on upper half page,
or NULL if tuple should be first */
MY_ATTRIBUTE((warn_unused_result));
/** Decide if the page should be split at the convergence point of inserts
converging to the left.
@param[in] cursor insert position
@return the first record to be moved to the right half page
@retval NULL if no split is recommended */
rec_t* btr_page_get_split_rec_to_left(const btr_cur_t* cursor);
/** Decide if the page should be split at the convergence point of inserts
converging to the right.
@param[in] cursor insert position
@param[out] split_rec if split recommended, the first record
on the right half page, or
NULL if the to-be-inserted record
should be first
@return whether split is recommended */
bool
btr_page_get_split_rec_to_right(const btr_cur_t* cursor, rec_t** split_rec);

/*************************************************************//**
Splits an index page to halves and inserts the tuple. It is assumed
Expand Down
Loading

0 comments on commit 6d7a826

Please sign in to comment.