It is `ulint` on 10.6 and `uint32_t` on 10.11+, but I included its
format specifier change in 10.6 (MDEV-35430, merged #3493) rather
than 10.11. This commit reverts that change so 10.11 can reapply it.
A race condition was observed between two buf_page_get_zip() for a page.
One of them had proceeded to buf_read_page(), allocating and x-latching
a buf_block_t that initially comprises only an uncompressed page frame.
While that thread was waiting inside buf_block_alloc(), another thread
would try to access the same page. Without acquiring a page latch, it
would wrongly conclude that there is corruption because no compressed
page frame exists for the block.
buf_page_get_zip(): Simplify the logic and correct the documentation.
Always acquire a shared latch to prevent any race condition with a
concurrent read operation. No longer increment a buffer-fix; the latch
is sufficient for preventing page relocation or eviction.
buf_read_page(): Add the parameter bool unzip=true. In buf_page_get_zip()
there is no need to allocate an uncompressed page frame for reading a
compressed BLOB page. We only need that for other ROW_FORMAT=COMPRESSED
pages, or for writing compressed BLOB pages.
btr_copy_zblob_prefix(): Remove the message "Cannot load compressed BLOB"
because buf_page_get_zip() will already have reported a more specific
error whenever it returns nullptr.
row_merge_buf_add(): Do not crash on BLOB corruption, but return an
error instead. (In debug builds, an assertion will fail if this
corruption is noticed.)
Reviewed by: Debarun Banerjee
MTR test rpl_semi_sync_no_missed_ack_after_add_slave fails on
buildbot after the preparatory commit for MDEV-35109 (5290fa043b)
which changed a sleep to a debug_sync point. The problem is that the
debug_sync point would time-out on a slave while waiting to enter
the logic to send an ACK reply. More specifically, where the test
config is a primary with two replicas, and the test waits on one of
the replicas to start sending an ACK, if the other replica was able
to receive the event and respond with an ACK before the binlog dump
thread of the timing-out server would prepare to send event, it
wouldn't set the SEMI_SYNC_NEED_ACK flag, and the replica wouldn't
even try to respond with an ACK.
Fix is to use debug_sync for both replicas such that both replicas
are held before sending their ack, so one can’t temporarily disable
semi-sync for the other before it receives the transaction.
When srv_page_size and innodb_page_size were introduced,
the functions page_align() and page_offset() got more expensive.
Let us try to replace such calls with simpler pointer arithmetics
with respect to the buffer page frame.
page_rec_get_next_non_del_marked(): Add a page frame as a parameter,
and template<bool comp>.
page_rec_next_get(): A more efficient variant of page_rec_get_next(),
with template<bool comp> and const page_t* parameters.
lock_get_heap_no(): Replaces page_rec_get_heap_no() outside debug checks.
fseg_free_step(), fseg_free_step_not_header(): Take the header block
as a parameter.
Reviewed by: Vladislav Lesin
ut_hash_ulint(): Remove. The exclusive OR before a modulus operation
does not serve any useful purpose; it is only obfuscating code and
wasting some CPU cycles.
Reviewed by: Debarun Banerjee
ut_fold_ull(): For SIZEOF_SIZE_T < 8, we simulate universal hashing
(Carter and Wegman, 1977) by pretending that SIZE_T_MAX + 1
is a prime. In other words, we implement a Rabin–Karp rolling
hash algorithm similar to java.lang.String.hashCode().
This is used for representing 64-bit dict_index_t::id or
dict_table_t::id in the native word size.
For SIZEOF_SIZE_T >= 8, we just use an identity mapping.
Reviewed by: Debarun Banerjee
The HASH_ macros are unnecessarily obfuscating the logic,
so we had better replace them.
hash_cell_t::search(): Implement most of the HASH_DELETE logic,
for a subsequent insert or remove().
hash_cell_t::remove(): Remove an element.
hash_cell_t::find(): Implement the HASH_SEARCH logic.
xb_filter_hash_free(): Avoid any hash table lookup;
just traverse the hash bucket chains and free each element.
xb_register_filter_entry(): Search databases_hash only once.
rm_if_not_found(): Make use of find_filter_in_hashtable().
dict_sys_t::acquire_temporary_table(), dict_sys_t::find_table():
Define non-inline to avoid unnecessary code duplication.
dict_sys_t::add(dict_table_t *table), dict_table_rename_in_cache():
Look for duplicate while finding the insert position.
dict_table_change_id_in_cache(): Merged to the only caller
row_discard_tablespace().
hash_insert(): Helper function of dict_sys_t::resize().
fil_space_t::create(): Look for a duplicate (and crash if found)
when searching for the insert position.
lock_rec_discard(): Take the hash array cell as a parameter
to avoid a duplicated lookup.
lock_rec_free_all_from_discard_page(): Remove a parameter.
Reviewed by: Debarun Banerjee
buf_pool_t::LRU_warn(): Also clear the try_LRU_scan flag, to ensure
that need_LRU_eviction() will hold. This should ensure progress when
buf_LRU_get_free_block() is expecting buf_flush_page_cleaner() to
make some room, even when buf_pool.LRU.count is small.
This hang was observed in trx_lists_init_at_db_start() while the last
batch of crash recovery was in progress, but it could theoretically
be possible also when a large part of the buffer pool is occupied by
record locks or the adaptive hash index.
Reviewed by: Debarun Banerjee
Item_func_conv::fix_length_and_dec() incorrectly set maximum
length as 64 character. But for negative numbers it can
return up to 65 charcters (including the sign).
Also fixes
MDEV-35392 Assertion `!__asan_region_is_poisoned((vo id*) dest,templ->mysql_col_len)' failed in void row_sel_field_store_in_mysql_format_func(byte *, const mysql_row_templ_t *, const byte *, ulint)
Conversion from CHAR to VARCHAR must be done before the call
for create_length_to_internal_length_string().
Moving the conversion code from Column_definition::prepare_blob_field()
to Column_definition::prepare_stage1_string().
- While saving the InnoDB table statistics, InnoDB does clone
the table object and its statistics to protect the metadata
in case if table is being dropped. From 10.6 onwards, any
background task inside InnoDB on the table takes MDL.
So metadata is protected by MDL of the table. Avoid the
cloning of the table and its associated index statistics.
ha_storage_put_memlim(): Invoke my_crc32c() to "fold", and traverse
the hash table only once.
fold_lock(): Remove some redundant conditions and use my_crc32c()
instead of ut_fold_ulint_pair().
trx_i_s_cache_t::add(): Replaces add_lock_to_cache(),
search_innodb_locks(), and locks_row_eq_lock(). Avoid duplicated
traversal of the hash table.
Reviewed by: Debarun Banerjee
- Replace statement fails with duplicate key error when multiple
unique key conflict happens. Reason is that Server expects the
InnoDB engine to store the confliciting keys in ascending order.
But the InnoDB doesn't store the conflicting keys
in ascending order.
Fix:
===
- Enable HA_DUPLICATE_KEY_NOT_IN_ORDER for InnoDB storage engine
only when unique index order is different in .frm and innodb dictionary.
The merge f00711bba2 included a change
of the test innodb.log_file_name, which would try to ensure that
in the presence of the code fix decdd4bf49
we would get an error on Linux when invoking lseek() on a directory.
It turns out that this is not the case in at least one Linux based
cloud environment.
Problem:
========
- dict_stats_table_clone_create() does not initialize the
flag stats_error_printed in either dict_table_t or dict_index_t.
Because dict_stats_save_index_stat() is operating on a copy
of a dict_index_t object, it appears that
dict_index_t::stats_error_printed will always be false
for actual metadata objects, and uninitialized in
dict_stats_save_index_stat().
Solution:
=========
dict_stats_table_clone_create(): Assign stats_error_printed
for table and index while copying the statistics
Replace wait_for_pattern_in_file.inc and all of its uses
to use search_pattern_in_file.inc with SEARCH_WAIT.
Reviewed By:
============
Kristian Nielsen <knielsen@knielsen-hq.org>
Sergei Golubchik <serg@mariadb.org>
For easier diff comparison, this commit takes
search_pattern_in_file.inc from 10.11 as-is for the
SEARCH_WAIT functionality added by Kristian Nielsen.
The changes to make it replace
wait_for_pattern_in_file.inc are in the following
commit.
Note that this commit breaks existing
wait_for_pattern_in_file.inc usage, so any tests which
use it will fail if building here.
The code in the can_eval_in_optimize() branch in
Item_func_pad::fix_length_and_dec() did not take into account
that the constant can be negative. So the function will return NULL.
This later crashed on DBUG_ASSERT() because a NOT NULL function returned NULL.
Adding set_maybe_null() into this branch if the constant is negative.
Problem was missing thd->set_time() before binlog event
execution in wsrep_apply_events.
Removed part of earlier commit 1363580 because it had
nothing to do with VERSIONED tables.
Note that this commit does not contain mtr-testcase
because actual timestamps on binlog file depends the
actual time when events are executed and how long
their execution takes.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
Ignoring configured server_id should not be a warning because
correct configuration is documented. Changed message to info
level with more detailed message what was configured and
what will be actually used.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
While applying CTAS log event, we peek the relay log to see if CTAS
contains inserted rows or if it's empty.
The peek function didn't check for end-of-file condition when tried to
get the next event from the log, and thus it hanged.
The fix includes checking for end-of-file while peeking for log events
and considering returned XID_EVENT value as a sign of an empty CTAS.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
Cache results of expensive my_get_stack_bounds(), instead of calling
it for every query in thread_attach()
See MDEV-27943 for effort to reduce thread_attach() overhead
Item_func_insert::fix_length_and_dec() incorrectly calculated max_length
when its collation.collation evaluated to my_charset_bin.
Fixing the code to calculate max_length in terms of octets rather
than in terms of characters when collation.collation is my_charset_bin.
For a primary configured with wait_point=AFTER_SYNC, if two threads
T1 (binlogging through MYSQL_BIN_LOG::write()) and T2 were
binlogging at the same time, T1 could accidentally wait for its
semi-sync ACK using the binlog coordinates of T2. Prior to
MDEV-33551, this only resulted in delayed transactions, because all
transactions shared the same condition variable for ACK signaling.
However, with the MDEV-33551 changes, each thread has its own
condition variable to signal. So T1 could wait indefinitely when
either:
1) T1's ACK is received but not T2's when T1 goes into
wait_after_sync(), because the ACK receiver thread has already
notified about the T1 ACK, but T1 was _actually_ waiting on T2's
ACK, and therefore tries to wait (in vain).
2) T1 goes to wait_after_sync() before any ACKs have arrived. When
T1's ACK comes in, T1 is woken up; however, sees it needs to wait
more (because it was actually waiting on T2's ACK), and goes to wait
again (this time, in vain).
Note that the actual cause of T1 waiting on T2's binlog coordinates
is when MYSQL_BIN_LOG::write() would call
Repl_semisync_master::wait_after_sync(), the binlog offset parameter
was read as the end of MYSQL_BIN_LOG::log_file, which is shared
among transactions. So if T2 had updated the binary log _after_ T1
had released LOCK_log, but not yet invoked wait_after_sync(), it
would use the end of the binary log file as the binlog offset, which
was that of T2 (or any future transaction).
The fix in this patch ensures consistency between the binary log
coordinates a transaction uses between report_binlog_update() and
wait_after_sync().
Reviewed By
============
Kristian Nielsen <knielsen@knielsen-hq.org>
Andrei Elkin <andrei.elkin@mariadb.com>
This is a preparatory commit for MDEV-35109 to make its
testing code cleaner (and harden other tests too).
The DEBUG_DBUG point simulate_delay_semisync_slave_reply
up to this patch used my_sleep() to delay an ACK response,
but sleeps are prone to test failures on machines that
run tests when already having a heavy load (e.g. on
buildbot).
This patch changes this DEBUG_DBUG sleep to use DEBUG_SYNC
to coordinate exactly when a slave should send its reply,
which is safer and faster.
As DEBUG_SYNC can't be used while a server is shutting
down, to synchronize threads with SHUTDOWN WAIT FOR SLAVES
logic, we use and extend wait_for_pattern_in_file.inc to
wait for an informational error message in the logic to
indicate that the shutdown process has reached the
intended state (i.e. indicating that the shutdown has
been delayed to await semi-sync ACKs). Specifically, the
extensions are as follows:
1. wait_for_pattern_in_file.inc is extended with parameter
wait_for_pattern_count as a number that indicates the
number of times a pattern should occur in the file before
return control back to the calling script.
2. search_for_pattern_in_file.inc is extended with parameter
SEARCH_ABORT_IS_SUCCESS to inverse the error/success
logic, so the SEARCH_ABORT condition can be used to
indicate success, rather than error.
The code tried to avoid String::copy() but did it in a wrong way,
so asan detected heap-use-after-free errors.
Removing the wrong optimization, using copy() instead.
- Moving the check_cols(1) test from fix_fields() to fix_length_and_dec().
So the test is now done before the code calling val_decimal()
in fix_length_and_dec().
- Removing Item_func_interval::fix_fields(), as it become equal
to the inherited one.
Post-push fix: row_vers_impl_x_locked() must be invoked under unlatched
lock_sys, the corresponding assertion was removed in MDEV-34466 and
was not restored in MDEV-34690. This fix restores it.
For Windows, the method of finding stack limit is reportedly flaky,
and might not work as desired, as documented in
https://joeduffyblog.com/2006/07/15/checking-for-sufficient-stack-space
"Unfortunately, the StackLimit is only updated as you actually touch pages on
the stack, and thus it’s not a reliable way to find out how much
uncommitted stack is left."
Thus, Windows specific code was removed. It might be added, if we find out
that we need it, so far there was no need.
Also AIX, the code based on HAVE_PTHREAD_GETATTR_NP was found not to work,
(produce false positives of stack overrun), thus the traditional
fallback code is used.
Also
- removed repetitive fallback code
- fixed non-portable void pointer arithmethics (GCC-ism)
- took into account that pthread_attr_getstack() can fail,
- fixed the code for (less common) STACK_DIRECTION > 0.
- removed confusing/wrong comments about what "stack base address" means
Single Unix Spec, AIX documentation make it clear what that is.
It was found that unnecessary work of building Ordered_key structures
is being done when processing NULL-aware materialization for IN predicates
having only one column. In fact, the logic for that simplified case can be
expressed as follows.
Say we have predicate left_expr IN (SELECT <subq1>), where left_expr is
scalar(not a tuple).
Then
if (left_expr is NULL) {
if (subq1 produced any rows) {
// note that we don't care if subq1 has produced
// NULLs or not.
NULL IN (<some values>) -> UNKNOWN, i.e. NULL.
} else {
NULL IN ({empty-set}) -> FALSE.
}
} else {
// left_expr is a non-NULL value
if (subq1 output has a match for left_expr) {
left_expr IN (..., left_expr ...) -> TRUE
} else {
// no "known" matches.
if (subq1 output has a NULL) {
left_expr IN ( ... NULL ...) ->
(NULL could have been a match or not)
-> NULL.
} else {
// subq1 didn't produce any "UNKNOWNs" so
// we're positive there weren't any matches
-> FALSE.
}
}
}
This commit introduces subselect_single_column_partial_engine class
implementing the logic described.
Reviewer: Sergei Petrunia <sergey@mariadb.com>
Two problem solved:
1) Item_default_value makes a shallow copy so the copy
should not delete field belong to the Item
2) Item_default_value should not inherit
derived_field_transformer_for_having and
derived_field_transformer_for_where (in this variant
pushing DEFAULT(f) is prohibited (return NULL) but
if return "this" it will be allowed (should go with
a lot of tests))
