MDEV-35475 Assertion `!rec_offs_nth_extern(offsets1, n)' failed in cmp_rec_rec_simple_field #3656
Conversation
|
|
Thirunarayanan
force-pushed
the
10.11-MDEV-35475
branch
3 times, most recently
from
5f9333a
to
124ae9b
Compare
Thirunarayanan
force-pushed
the
10.11-MDEV-35475
branch
2 times, most recently
from
d93934d
to
4e02b10
Compare
…p_rec_rec_simple_field Problem: ======= InnoDB wrongly stores the primary key field in externally stored off page during bulk insert operation. This leads to assert failure. Solution: ======== row_merge_buf_blob(): Should store the primary key fields inline. Store the variable length field data externally based on the row format of the table. row_merge_buf_write(): check whether the record size exceeds the maximum record size. row_merge_copy_blob_from_file(): Construct the tuple based on the variable length field
Thirunarayanan
force-pushed
the
10.11-MDEV-35475
branch
from
4e02b10
to
1278381
Compare
| dberr_t err= os_file_write( | ||
| IORequestWrite, "(bulk insert)", blob_file->fd, | ||
| field->data, blob_file->offset, len); | ||
| (byte*)field->data + local_len, blob_file->offset, |
There was a problem hiding this comment.
static_cast<const byte*>(field->data) + local_len would be safer.
| memcpy(data, field->data, local_len); | ||
| /* Write zeroes for first 8 bytes */ | ||
| memset(data, 0, 8); | ||
| memset(data + local_len, 0, 8); | ||
| /* Write offset for next 8 bytes */ | ||
| mach_write_to_8(data + 8, val); | ||
| mach_write_to_8(data + local_len + 8, val); | ||
| /* Write length of the blob in 4 bytes */ | ||
| mach_write_to_4(data + 16, len); | ||
| blob_file->offset+= field->len; | ||
| mach_write_to_4(data + local_len + 16, field->len - local_len); | ||
| blob_file->offset+= (field->len - local_len); | ||
| blob_file->n_rec++; | ||
| dfield_set_data(field, data, BTR_EXTERN_FIELD_REF_SIZE); | ||
| dfield_set_data(field, data, | ||
| local_len + BTR_EXTERN_FIELD_REF_SIZE); | ||
| dfield_set_ext(field); |
There was a problem hiding this comment.
We could invoke dfield_set_data() and dfield_set_ext() at the start and then do data += local_len to reduce the diff and to possibly simplify the code, in case the common subexpression elimination (CSE) phase of the compiler might miss the repeated data + local_len.
| for (ulint i= 0; i < n_fields; i++) | ||
| { | ||
| dfield_t *field= &entry->fields[i]; | ||
| if (dfield_is_null(field) || field->len <= 2000) | ||
| dfield_t *field= &fields[i]; | ||
| if (i < index->n_uniq || dfield_is_null(field)) | ||
| continue; |
There was a problem hiding this comment.
Could we just adjust the initial value of the loop variable?
for (auto i= index->first_user_field(); i < n_fields; i++)| if (!blob_heap) { | ||
| blob_heap = mem_heap_create(100); | ||
| } |
There was a problem hiding this comment.
How was the initial size 100 bytes determined? If it is too small, a subsequent mem_heap_alloc() might just waste that block and allocate a new one. If it is too large, the end of the block would go wasted.
| { | ||
| const uint blob_prefix= dict_table_has_atomic_blobs(index->table) | ||
| ? 0 | ||
| : REC_ANTELOPE_MAX_INDEX_COL_LEN; | ||
| for (ulint i = 0; i < dtuple_get_n_fields(tuple); i++) |
There was a problem hiding this comment.
Could the loop start as follows?
for (auto i= index->first_user_field(); i < tuple->n_fields; i++)No clustered index field before that may be stored externally.
Description
Problem:
InnoDB wrongly stores the primary key field in externally stored off page during bulk insert operation. This leads to assert failure.
Solution:
row_merge_buf_blob(): Should store the primary key fields inline. Store the variable length field data externally based on the row format of the table.
g.
How can this PR be tested?
./mtr innodb.alter_copy_bulk innodb.insert_into_empty
Basing the PR against the correct MariaDB version
mainbranch.PR quality check