It's a micro optimization. On most platforms CPUs has instructions to
compare with 0 fast. DB_SUCCESS is the most popular outcome of functions
and this patch optimized code like (err == DB_SUCCESS)
BtrBulk::finish(): bogus assertion fixed
fil_node_t::read_page0(): corrected usage of os_file_read()
que_eval_sql(): bugus assertion removed. Apparently it checked that
the field was assigned after having been zero-initialized at
object creation.
It turns out that the return type of os_file_read_func() was changed
in mysql/mysql-server@98909cefbc (MySQL 5.7)
from ibool to dberr_t. The reviewer (if there was any) failed to
point out that because of future merges, it could be a bad idea to
change the return type of a function without changing the function name.
This change was applied to MariaDB 10.2.2 in
commit 2e814d4702 but the
MariaDB-specific code was not fully adjusted accordingly,
e.g. in fil_node_open_file(). Essentially, code like
!os_file_read(...) became dead code in MariaDB and later
in Mariabackup 10.2, and we could be dealing with an uninitialized
buffer after a failed page read.
os_mem_alloc_large(): Invoke the macro ut_2pow_round() with the
correct argument type.
innobase_large_page_size, innobase_use_large_pages,
os_use_large_pages, os_large_page_size: Remove.
Simply refer to opt_large_page_size, my_use_large_pages.
xtrabackup_backup_func(): If the log checkpoint header changed
since we last read it, search for the most recent checkpoint again.
Otherwise, we could corrupt the backup of the redo log, because the
least significant bits of checkpoint_lsn_start would not match
log_sys->log.lsn.
The recv_sys data structures are accessed not only from the thread
that executes InnoDB plugin initialization, but also from the
InnoDB I/O threads, which can invoke recv_recover_page().
Assert that sufficient concurrency control is in place.
Some code was accessing recv_sys data structures without
holding recv_sys->mutex.
recv_recover_page(bpage): Refactor the call from buf_page_io_complete()
into a separate function that performs necessary steps. The
main thread was unnecessarily releasing and reacquiring recv_sys->mutex.
recv_recover_page(block,mtr,recv_addr): Pass more parameters from
the caller. Avoid redundant lookups and computations. Eliminate some
redundant variables.
recv_get_fil_addr_struct(): Assert that recv_sys->mutex is being held.
That was not always the case!
recv_scan_log_recs(): Acquire recv_sys->mutex for the whole duration
of the function. (While we are scanning and buffering redo log records,
no pages can be read in.)
recv_read_in_area(): Properly protect access with recv_sys->mutex.
recv_apply_hashed_log_recs(): Check recv_addr->state only once,
and continuously hold recv_sys->mutex. The mutex will be released
and reacquired inside recv_recover_page() and recv_read_in_area(),
allowing concurrent processing by buf_page_io_complete() in I/O threads.
- Fetch innodb_compression_level from the running server.Add the value
of innodb_compression_level in backup-my.cnf file during backup phase.
So that prepare can use the innodb_compression_level variable from
backup-my.cnf
Fix the warnings issued by GCC 8 -Wstringop-truncation
and -Wstringop-overflow in InnoDB and XtraDB.
This work is motivated by Jan Lindström. The patch mainly differs
from his original one as follows:
(1) We remove explicit initialization of stack-allocated string buffers.
The minimum amount of initialization that is needed is a terminating
NUL character.
(2) GCC issues a warning for invoking strncpy(dest, src, sizeof dest)
because if strlen(src) >= sizeof dest, there would be no terminating
NUL byte in dest. We avoid this problem by invoking strncpy() with
a limit that is 1 less than the buffer size, and by always writing
NUL to the last byte of the buffer.
(3) We replace strncpy() with memcpy() or strcpy() in those cases
when the result is functionally equivalent.
Note: fts_fetch_index_words() never deals with len==UNIV_SQL_NULL.
This was enforced by an assertion that limits the maximum length
to FTS_MAX_WORD_LEN. Also, the encoding that InnoDB uses for
the compressed fulltext index is not byte-order agnostic, that is,
InnoDB data files that use FULLTEXT INDEX are not portable between
big-endian and little-endian systems.
log_group_read_log_seg(): Always return false when returning
before reading end_lsn.
xtrabackup_copy_logfile(): On error, indicate whether
a corrupt log record was encountered.
Only xtrabackup_copy_logfile() in Mariabackup cared about the
return value of the function. InnoDB crash recovery was not
affected by this bug.
The variable is obsolete.
In mariabackup --backup, encryption plugin loading code sets the value
this parameter to the same as in server.
In mariabackup --prepare, no new redo log is generated,
and xtrabackup_logfile is removed after it anyway.
Fix one more bug in "DDL redo" phase in prepare
If table was renamed, and then new table was created with the old name,
prepare can be confused, and .ibd can end up with wrong name.
Fix the order of how DDL fixup is applied , once again - ".new" files
should be processed after renames.
If, during backup
1) Innodb table is dropped (after being copied to backup) and then
2) Before backup finished, another Innodb table is renamed, and new name
is the name of the dropped table in 1)
then, --prepare fails with assertion, as DDL fixup code in prepare
did not handle this specific case.
The fix is to process drops before renames, in prepare DDL-"redo" phase.
If an encrypted table is created during backup, then
mariabackup --backup could wrongly fail.
This caused a failure of the test mariabackup.huge_lsn once on buildbot.
This is due to the way how InnoDB creates .ibd files. It would first
write a dummy page 0 with no encryption information. Due to this,
xb_fil_cur_open() could wrongly interpret that the table is not encrypted.
Subsequently, page_is_corrupted() would compare the computed page
checksum to the wrong checksum. (There are both "before" and "after"
checksums for encrypted pages.)
To work around this problem, we introduce a Boolean option
--backup-encrypted that is enabled by default. With this option,
Mariabackup will assume that a nonzero key_version implies that the
page is encrypted. We need this option in order to be able to copy
encrypted tables from MariaDB 10.1 or 10.2, because unencrypted pages
that were originally created before MySQL 5.1.48 could contain nonzero
garbage in the fields that were repurposed for encryption.
Later, MDEV-18128 would clean up the way how .ibd files are created,
to remove the need for this option.
page_is_corrupted(): Add missing const qualifiers, and do not check
space->crypt_data unless --skip-backup-encrypted has been specified.
xb_fil_cur_read(): After a failed page read, output a page dump.
would not hide more interesting information, like invalid memory accesses.
some "leaks" are expected
- partly this is due to weird options parsing, that runs twice, and
does not free memory after the first run.
- also we do not mind to exit() whenever it makes sense, without full
cleanup.
- Refactor code to isolate page validation in page_is_corrupted() function.
- Introduce --extended-validation parameter(default OFF) for mariabackup
--backup to enable decryption of encrypted uncompressed pages during
backup.
- mariabackup would still always check checksum on encrypted data,
it is needed to detect partially written pages.
