Bug#12637786 was fixed with rb:692 by marko. But that fix has a remaining
bug. It added this assert;
ut_ad(ind_field->prefix_len);
before a section of code that assumes there is a prefix_len.
The patch replaced code that explicitly avoided this with a check for
prefix_len. It turns out that the purge thread can get to that assert
without a prefix_len because it does not use a row_ext_t* .
When UNIV_DEBUG is not defined, the affect of this is that the purge thread
sets the dfield->len to zero and then cannot find the entry in the index to
purge. So secondary index entries remain unpurged.
This patch does not do the assert. Instead, it uses
'if (ind_field->prefix_len) {...}'
around the section of code that assumes a prefix_len. This is the way the
patch I provided to Marko did it.
The test case is simply modified to do a sleep(10) in order to give the
purge thread a chance to run. Without the code change to row0row.c, this
modified testcase will assert if InnoDB was compiled with UNIV_DEBUG.
I tried to sleep(5), but it did not always assert.
bug. It added this assert;
ut_ad(ind_field->prefix_len);
before a section of code that assumes there is a prefix_len.
The patch replaced code that explicitly avoided this with a check for
prefix_len. It turns out that the purge thread can get to that assert
without a prefix_len because it does not use a row_ext_t* .
When UNIV_DEBUG is not defined, the affect of this is that the purge thread
sets the dfield->len to zero and then cannot find the entry in the index to
purge. So secondary index entries remain unpurged.
This patch does not do the assert. Instead, it uses
'if (ind_field->prefix_len) {...}'
around the section of code that assumes a prefix_len. This is the way the
patch I provided to Marko did it.
The test case is simply modified to do a sleep(10) in order to give the
purge thread a chance to run. Without the code change to row0row.c, this
modified testcase will assert if InnoDB was compiled with UNIV_DEBUG.
I tried to sleep(5), but it did not always assert.
GCC 4.6 has new -Wunused-but-set-variable flag, which is enabled
by -Wall, that causes GCC to emit a warning whenever a local variable
is assigned to, but otherwise unused (aside from its declaration).
Since the maintainer mode uses -Wall and -Werror, source code which
triggers these warnings will be rejected. That is, these warnings
become hard errors.
The solution is to fix the code which triggers these specific warnings.
In most of the cases, this is a welcome cleanup as code which triggers
this warning is probably dead anyway.
row_build_index_entry(): In innodb_file_format=Barracuda
(ROW_FORMAT=DYNAMIC or ROW_FORMAT=COMPRESSED), a secondary index on a
full column can refer to a field that is stored off-page in the
clustered index record. Take that into account.
rb:692 approved by Jimmy Yang
Refactor the !rec_offs_any_extern relaxation in row_build().
trx_assert_active(trx_id): Assert that the given transaction is active.
(In the 5.1 built-in InnoDB, there is no trx->is_recovered field.)
trx_assert_recovered(trx_id): Assert that the given transaction is
active and has been recovered after a crash.
row_build(): Replace a bunch of code with an assertion that invokes
trx_assert_active() or trx_assert_recovered() and row_get_rec_trx_id().
row_get_trx_id_offset(): Make the function inlined. Remove the unused
parameter rec, and make all parameters const.
row_get_rec_trx_id(), row_get_rec_roll_ptr(): Make all parameters const.
rb:691 approved by Jimmy Yang
page_zip_dir_elems(): New function, refactored from page_zip_dir_size().
page_zip_dir_size(): Use page_zip_dir_elems()
page_zip_dir_start_offs(): New function: Gets an offset to the
compressed page trailer (the dense page directory), including deleted
records (the free list)
page_zip_dir_start_low(page_zip, n_dense): Constness-preserving
wrapper macro for page_zip_dir_start_offs().
page_zip_dir_start(page_zip): Constness-preserving
wrapper macro for page_zip_dir_start_offs().
page_zip_decompress_node_ptrs(), page_zip_decompress_clust(): Replace
a formula with a fully equivalent page_zip_dir_start_low() call.
page_zip_write_rec(), page_zip_parse_write_node_ptr(),
page_zip_write_node_ptr(), page_zip_write_trx_id_and_roll_ptr(),
page_zip_clear_rec(): Replace a formula with an almost equivalent
page_zip_dir_start() call.
It is OK to replace page_dir_get_n_heap(page) with
page_dir_get_n_heap(page_zip->data), because
ut_ad(page_zip_header_cmp(page_zip, page)) or
page_zip_validate(page_zip, page) asserts that the
page headers are identical.
rb:687 approved by Jimmy Yang
approved by: Marko
rb://681
Coalescing of free buf_page_t descriptors can prove to be one severe
bottleneck in performance of compression. One such workload where it
hurts badly is DROP TABLE. This patch removes buf_page_t allocations
from buf_buddy and uses ut_malloc instead.
In order to further reduce overhead of colaescing we no longer attempt
to coalesce a block if the corresponding free_list is less than 16 in
size.
The buf_pool->zip_clean list is only needed for debugging, or for
recomputing buf_pool->page_hash when resizing the buffer pool. Buffer
pool resizing was never fully implemented. Remove the resizing code,
and define buf_pool->zip_clean only in debug builds.
buf_pool->zip_clean, buf_LRU_insert_zip_clean(): Enclose in
#if defined UNIV_DEBUG || UNIV_BUF_DEBUG.
buf_chunk_free(), buf_chunk_all_free(), buf_pool_shrink(),
buf_pool_page_hash_rebuild(), buf_pool_resize(): Remove (unreachable code).
rb:671 approved by Inaam Rana
There is an apparent problem with page_zip_clear_rec().
In btr_cur_optimistic_update() we do this:
page_cur_delete_rec(page_cursor, index, offsets, mtr);
...
rec = btr_cur_insert_if_possible(cursor, new_entry, 0/*n_ext*/, mtr);
ut_a(rec); /* <- We calculated above the insert would fit */
The problem is that page_cur_delete_rec() could fill the modification
log while doing page_zip_clear_rec(), requiring recompression for the
btr_cur_insert_if_possible(). In a pathological case, the data could
fail to recompress.
page_zip_clear_rec(): Leave the page modification log alone. Only
clear the necessary fields.
rb:673 approved by Jimmy Yang
Replace UNIV_BLOB_NULL_DEBUG with UNIV_DEBUG||UNIV_BLOB_LIGHT_DEBUG.
Fix known bogus failures.
btr_cur_optimistic_update(): If rec_offs_any_null_extern(), assert that
the current transaction is an incomplete transaction that is being
rolled back in crash recovery.
row_build(): If rec_offs_any_null_extern(), assert that the transaction
that last updated the record was recovered during crash recovery
(and will soon be rolled back).
btr_cur_compress_if_useful(), btr_compress(): Add the parameter ibool
adjust. If adjust=TRUE, adjust the cursor position after compressing
the page.
btr_lift_page_up(): Return a pointer to the father page.
BTR_KEEP_POS_FLAG: A new flag for btr_cur_pessimistic_update().
btr_cur_pessimistic_update(): If *big_rec != NULL and flags &
BTR_KEEP_POS_FLAG, keep the cursor positioned on the updated record.
Also, do not release the index tree x-lock if *big_rec != NULL.
btr_cur_mtr_commit_and_start(): Commits and restarts a
mini-transaction so that it will retain an x-lock on index->lock and
the page of the cursor. This is invoked when
btr_cur_pessimistic_update() returns *big_rec != NULL.
In all callers of btr_cur_pessimistic_update() that do not pass
BTR_KEEP_POS_FLAG, assert that *big_rec == NULL.
btr_cur_compress(): Unused function [in the built-in MySQL 5.1], remove.
page_rec_get_nth(): Return the nth record on the page (an inverse
function of page_rec_get_n_recs_before()). Refactored from
page_get_middle_rec().
page_get_middle_rec(): Invoke page_rec_get_nth().
page_cur_insert_rec_zip_reorg(): Make use of the page directory
shortcuts in page_rec_get_nth() instead of scanning the whole list of
records.
row_ins_clust_index_entry_by_modify(): Pass BTR_KEEP_POS_FLAG to
btr_cur_pessimistic_update().
row_ins_index_entry_low(): If row_ins_clust_index_entry_by_modify()
returns a big_rec, invoke btr_cur_mtr_commit_and_start() in order to
commit and start the mini-transaction without releasing the x-locks on
index->lock and the cursor page, and write the big_rec. Releasing the
page latch in mtr_commit() caused a race condition.
row_upd_clust_rec(): Pass BTR_KEEP_POS_FLAG to
btr_cur_pessimistic_update(). If it returns a big_rec, invoke
btr_cur_mtr_commit_and_start() in order to commit and start the
mini-transaction without releasing the x-locks on index->lock and the
cursor page, and write the big_rec. Releasing the page latch in
mtr_commit() caused a race condition.
sync_thread_add_level(): Add the parameter ibool relock. When TRUE,
bypass the latching order rules.
rw_lock_add_debug_info(): For nested X-lock requests, pass relock=TRUE
to sync_thread_add_level().
rb:678 approved by Jimmy Yang
Some ut_a(!rec_offs_any_null_extern()) assertion failures are indicating
genuine BLOB bugs, others are bogus failures when rolling back incomplete
transactions at crash recovery. This needs more work, and until I get a
chance to work on it, other testing must not be disrupted by this.
If UNIV_DEBUG or UNIV_BLOB_LIGHT_DEBUG is enabled, add
!rec_offs_any_null_extern() assertions, ensuring that records do not
contain null pointers to externally stored columns in inappropriate
places.
btr_cur_optimistic_update(): Assert !rec_offs_any_null_extern().
Incomplete records must never be updated or deleted. This assertion
will cover also the pessimistic route.
row_build(): Assert !rec_offs_any_null_extern(). Search tuples must
never be built from incomplete index entries.
row_rec_to_index_entry(): Assert !rec_offs_any_null_extern() unless
ROW_COPY_DATA is requested. ROW_COPY_DATA is used for
multi-versioning, and therefore it might be valid to copy the most
recent (uncommitted) version while it contains a null pointer to
off-page columns.
row_vers_build_for_consistent_read(),
row_vers_build_for_semi_consistent_read(): Assert !rec_offs_any_null_extern()
on all versions except the most recent one.
trx_undo_prev_version_build(): Assert !rec_offs_any_null_extern() on
the previous version.
rb:682 approved by Sunny Bains
Problem: in case of wrong data insert into indexed GEOMETRY fields
(e.g. NULL value for a not NULL field) MyISAM reported
"ERROR 126 (HY000): Incorrect key file for table; try to repair it"
due to misuse of the key deletion function.
Fix: always use R-tree key functions for R-tree based indexes
and B-tree key functions for B-tree based indexes.
SECONDARY INDEX IN INNODB
The patches for Bug#11751388 and Bug#11784056 enabled concurrent
reads while creating secondary indexes in InnoDB. However, they
introduced a regression. This regression occured if ALTER TABLE
failed after the index had been added, for example during the
lock upgrade needed to update .FRM. If this happened, InnoDB
and the server got out of sync with regards to which indexes
actually existed. Therefore the patch for Bug#11815600 again
disabled concurrent reads.
This patch re-enables concurrent reads. The original regression
is fixed by splitting the ADD INDEX operation into two parts.
First the new index is created but not made active. This is
done while concurrent reads are allowed. The second part of
the operation makes the index active (or reverts the change).
This is done after lock upgrade, which prevents the original
regression.
In order to implement this change, the patch changes the storage
API for in-place index creation. handler::add_index() is split
into two functions, handler_add_index() and
handler::final_add_index(). The former for creating indexes without
making them visible and the latter for commiting (i.e. making
visible) new indexes or reverting the changes.
Large parts of this patch were written by Marko Mäkelä.
Test case added to innodb_mysql_lock.test.
With this change, the index prefix column length lifted from 767 bytes
to 3072 bytes if "innodb_large_prefix" is set to "true".
rb://603 approved by Marko
mtr_start(): Declare the mtr memory area uninitialized in Valgrind
before initializing the fields.
mtr_commit(): Declare everything uninitialized except
mtr->start_lsn, mtr->end_lsn and mtr->state.
HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK".
Attempt to update an InnoDB temporary table under LOCK TABLES
led to assertion failure in both debug and production builds
if this temporary table was explicitly locked for READ. The
same scenario works fine for MyISAM temporary tables.
The assertion failure was caused by discrepancy between lock
that was requested on the rows of temporary table at LOCK TABLES
time and by update operation. Since SQL-layer requested a
read-lock at LOCK TABLES time InnoDB engine assumed that upcoming
statements which are going to be executed under LOCK TABLES will
only read table and therefore should acquire only S-lock.
An update operation broken this assumption by requesting X-lock.
Possible approaches to fixing this problem are:
1) Skip locking of temporary tables as locking doesn't make any
sense for connection-local objects.
2) Prohibit changing of temporary table locked by LOCK TABLES ...
READ.
Unfortunately both of these approaches have drawbacks which make
them unviable for stable versions of server.
So this patch takes another approach and changes code in such way
that LOCK TABLES for a temporary table will always request write
lock. In 5.1 version of this patch switch from read lock to write
lock is done inside of InnoDBs handler methods as doing it on
SQL-layer causes compatibility troubles with FLUSH TABLES WITH
READ LOCK.
Fix a deadlock in the initial patch. lock_validate() must not hold the
lock system mutex while s-latching a block, because some functions,
such as lock_rec_convert_impl_to_expl(), may be already holding an x-latch
on the block that lock_validate() is interested in while attempting to
acquire the lock system mutex.
This deadlock was not caught by UNIV_SYNC_DEBUG because of
buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK).
lock_clust_rec_some_has_impl(), row_get_rec_trx_id(),
lock_rec_queue_validate(), lock_table_other_has_incompatible(),
lock_table_has_to_wait_in_queue(), lock_table_queue_validate():
Add const qualifiers.
row_get_trx_id_offset(): Add const qualifiers. Keep the parameter rec
only in UNIV_DEBUG builds. Inline the function.
lock_rec_validate_page(): Take the buffer block as a parameter, to
avoid a buf_page_get_gen() call in most cases.
lock_rec_validate_page_low(): A version of lock_rec_validate_page()
that assumes that the lock system mutexes are already being held.
lock_rec_get_next_on_page_const(): A const variant of
lock_rec_get_next_on_page().
lock_validate(): Do not release the lock system mutex while
buffer-fixing the block for the lock_rec_validate_page() call.
Releasing the mutex apparently caused the assertion failure.
rb:665 approved by Sunny Bains
------------------------------------------------------------
revno 2876.244.305
revision id marko.makela@oracle.com-20110413082211-e6ouhjz5rmqxcqap
parent marko.makela@oracle.com-20110413075948-kvytmc37ye1nt7d9
committer Marko Mäkelä <marko.makela@oracle.com>
branch nick 5.6-innodb
timestamp Wed 2011-04-13 11:22:11 +0300
message:
Suppress the Bug #58815 (Bug #11765812) assertion failure.
buf_page_get_gen(): Introduce BUF_GET_POSSIBLY_FREED for suppressing the
check that the file page must not have been freed.
btr_estimate_n_rows_in_range_on_level(): Pass BUF_GET_POSSIBLY_FREED and
explain in the comments why this is needed and why it should be mostly
harmless to ignore the problem. If InnoDB had always initialized all
unused fields in data files, no problem would exist.
This change does not fix the bug, it just "shoots the messenger".
rb:647 approved by Jimmy Yang
or Oracle Bug #11766513.
trx_undo_free_prepared(): Do not acquire or release trx->rseg->mutex.
This code is invoked in the single-threaded part of shutdown, therefore
a mutex is not needed.
LEAK WITH PARTITIONED ARCHIVE TABLES
CHECK TABLE against archive table, when file descriptors
are exhausted, caused server crash.
Archive didn't handle errors when opening data file for
CHECK TABLE.
node->modification_counter == node->flush_counter
This bug (an assertion failure during a crash-like shutdown)
was introduced by the fix of
Bug#12323643 Clean up the InnoDB thread shutdown and assertions (WL#5136).
fil_node_close_file(): Relax the failing assertion
when innodb_fast_shutdown=2.
DB_COL_APPEARS_TWICE_IN_INDEX: Remove. This condition is already
checked and reported by MySQL before passing the index definition to
the storage engine.
row_create_index_for_mysql(): Remove the redundant check for
DB_COL_APPEARS_TWICE_IN_INDEX. When enforcing the column prefix index
limit, invoke dict_mem_index_free(index) to plug the memory leak. In
the loop, use index->n_def instead of dict_index_get_n_fields(index),
because the latter would be 0 for indexes that have not been copied to
the data dictionary cache.
innodb-use-sys-malloc.test:
Add test cases for attempting to trigger the error checks in
row_create_index_for_mysql(). Before MySQL 5.5 and WL#5743, the leak
is only reproducible if ha_innobase::max_supported_key_part_length()
returned a higher limit than the one used in
row_create_index_for_mysql().
In MySQL 5.5 and later, the leak is reproducible with
innodb_large_prefix=true.
rb:688 approved by Jimmy Yang
