- 15 Aug, 2023 40 commits
-
-
Nikita Malyavin authored
s3 is read-only. No need to update to a read-only engine with LOCK=NONE.
-
Nikita Malyavin authored
Forbid Online for CONNECT.
-
Nikita Malyavin authored
Adding an auto_increment column online leads to an undefined behavior. Basically any DEFAULTs that depend on a row order in the table, or on the non-deterministic (in scope of the ALTER TABLE statement) function is UB. For example, NOW() is considered generally non-deterministic (Item_func_now_utc is marked with VCOL_NON_DETERMINISTIC), but it's fixed in scope of a single statement. Same for any other function that depends only on the session/status vars apart from its arguments. Only two UB cases are known: * adding new AUTO_INCREMENT column. Modifying the existing column may be fine under certain circumstances, see MDEV-31058. * adding new column with DEFAULT(nextval(...)). Modifying the existing column is possible, since its value will be always present in the online event, except for the NULL -> NOT NULL modification
-
Nikita Malyavin authored
Add a new virtual function that will increase the inserted rows count for the insert log event and decrease it for the delete event. Reuses Rows_log_event::m_row_count on the replication side, which was only set on the logging side.
-
Nikita Malyavin authored
Forbid online for sequences. It doesn't work now and sequence tables are always only one row, so not worth the extra complexity.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
-
Andrei authored
The deadlock was caused by too strong MDL acquired by the start ALTER. Replica's ALTER TABLE replication consists of two phases: 1. Start ALTER (SA) -- the event is emittd in the very beginning, allowing replication start ALTER in parallel 2. Commit ALTER (CA) -- ensures that master finishes successfully CA is normally received by wait_for_master call. If parallel DML was run, the following sequence will take place: |- SA |- DML |- CA If CA is handled after MDL upgrade, it'll will deadlock with DML. While MDL is shared by the start ALTER wait for its 2nd part to allow concurrent DMLs to grab the lock. The fix uses wait_for_master reentrancy -- no need to avoid a second call in the end of mysql_alter_table. Since SA and CA are marked with FL_DDL, the DML issued in-between cannot be rescheduled before or after them. However, SA "commits" (by he call of write_bin_log_start_alter and, subsequently, thd->wakeup_subsequent_commits) before the copy stage begins, unlocking the DMLs to run on this table. That is, these DMLs will be executed concurrently with the copy stage, making Online alter effective on replicas as well Co-authored-by: Nikita Malyavin (nikitamalyavin@gmail.com)
-
Nikita Malyavin authored
-
Nikita Malyavin authored
Make binlog_prepare_row_images accept image type as an argument.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
1. Make online disk writes unlimited, same as filesort does. 2. Make proper error handling -- in 32-bit build IO_CACHE capacity limit is 4GB, so it is quite possible to overfill there. 3. Event_log::write_cache complicated with event reparsing, and as it was proven by QA, contains some mistakes. Rewrite introbuce a simpler and much faster version, not featuring reparsing and therefore copying a whole buffer at once. This also disables checksums and crypto. 4. Handle read_log_event errors correctly: error returned is -1 (eof signal for alter table), and my_error is not called. Call my_error and always return 1. There's no test for this, since it shouldn't happen, see the next bullet. 5. An event could be written partially in case of error, if it's bigger than the IO_CACHE buffer. Restore the position where it was before the error was emitted. As a result, online alter is untied of several binlog variables, which was a second aim of this patch.
-
Nikita Malyavin authored
Report correct algorithm in the error message.
-
Nikita Malyavin authored
Don't skip row_end if it wasn't set explicitly. Also another segfault was caused by accessing rpl_write_set on slave during the row update/delete. The reason was a default_column_bitmaps() call, which also sets rpl_write_set to NULL. Previously, the related behavior was changed in commit afd3ee97ad, where one such call was removed from Update_rows_log_event::do_exec_row, but the same one was mistakenly left in Delete_rows_log_event. Now it's also removed.
-
Nikita Malyavin authored
The correct (best) algorithm is now chosen for ALGORITHM=DEFAULT and alter_algorithm=DEFAULT See also MDEV-30906
-
Nikita Malyavin authored
Group all the checks in online_alter_check_supported(). There is now two groups of checks: 1. A technical availability of online, that is checked before open_tables, and affects table_list->lock_type. It's supposed to be safe to make it TL_READ even if COPY algorithm will fall back to not-online, since MDL is SHARED_UPGRADEABLE anyway. 2. An 'online' availability for a COPY algorithm. It can be done as late as just before the copy_data_between_tables call. The lock_type influence is disclosed above, so the only other place it affects is Alter_info::supports_lock, where `online` flag is only used to decide whether to report the error at the inplace preparation stage. We'd want to make that at the last resort, which is COPY preparation, if no algorithm is chosen by the user. So it's event better now. Some changes are required to the autoinc support detection, as the check now happens after mysql_prepare_alter_table: * alter_info->drop_list is empty * instead, dropped columns are in tmp_set * alter_info->create_list now has every field that's in the new table. * the column definition's change.str will be nonnull whether the column remains in the new table (vs whether it was changed, as before). But it also has `field` field set. * IF EXISTS doesn't have to be dealt anymore This infers that the changes are now checked in more detail: a field's definition shouldn't be changed, vs a field shouldn't be mentioned in the CHANGE list, as it was before. This is reflected by the line 193 test.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
Also update abi declarations. The abi itself is unchanged, since const doesn't affect C-style exported name.
-
Nikita Malyavin authored
disabie view-protocol for certain places: 1. While a transaction is in progress: a view gets locked for a transaction duration, so DROP VIEW from a servicing conneciton deadlocks. We can't just disable servicing connection, as CREATE VIEW and DROP VIEW commit implicitly. 2. For querying Opened_table_definitions: the actual query executed is `SELECT * FROM mysqltest_tmp_v`, then the view is dropped
-
Nikita Malyavin authored
Skip more rpl-related error handling. Also move the error check inside if (table) -- otherwise the error should be handled already.
-
Nikita Malyavin authored
Online alter and replication paths differ quite noticeably. It's time to fix the mess. Online alter has to unpack the old table's data (here conv_table), and then convert, while replication requires more sophisticated track: handle possible conversions, treat extra replica fields, and carefully deduce master's record length. * Extract unpack_field. * handle the unpacking progress through a state machine presented by Unpack_record_state struct. * Stick most of the online alter unpacking together under a single conditional branch.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
When column is changed to autoinc, ALTER TABLE may update zero/NULL values, if NO_AUTO_VALUE_ON_ZERO mode is not enabled. Forbid this for LOCK=NONE for the unreliable cases. The cases are described in online_alter_check_autoinc.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
when committing a big transaction, online_alter_cache_log creates a cache file. It wasn't properly closed, which was spotted by a memory leak from my_register_filename. A temporary file also remained open. Binlog wasn't affected by this, since it features its own file management. A proper closing is calling close_cached_file. It deinits io_cache and closes the underlying file. After closing, the file is expected to be deleted automagically.
-
Nikita Malyavin authored
Non-transactional engines changes are visible immediately the row operation succeeds, so we should apply the changes to the online buffer immediately after the statement ends.
-
Nikita Malyavin authored
The row events were applied "twice": once for the ha_partition, and one more time for the underlying storage engine. There's no such problem in binlog/rpl, because ha_partiton::row_logging is normally set to false. The fix makes the events replicate only when the handler is a root handler. We will try to *guess* this by comparing it to table->file. The same approach is used in the MDEV-21540 fix, 231feabd. The assumption is made, that the row methods are only called for table->file (and never for a cloned handler), hence the assertions are added in ha_innobase and ha_myisam to make sure that this is true at least for those engines Also closes MDEV-31040, however the test is not included, since we have no convenient way to construct a deterministic version.
-
Nikita Malyavin authored
...in bitmap_intersect m_cols_ai was accessed during the Delete event, however this field is only related to Updates. Moving it to Update_rows_event would require too much effort. So instead: * Only access m_cols_ai in Update events (conditional branch is added in Rows_log_event::do_add_row_data) * Clean up m_cols_ai operations in Rows_log_event constructor. m_cols_ai.bitmap is first set to NULL, indicating invalid event. Then it is initialized: -> For Update events, a new bitmap is created. -> For other events, debug mode, m_cols_ai.bitmap is set to 1, indicating that the value is correct, but it shouldn't be accessed. To make sure we'll have a failure, n_bits is also set to 1. -> In release mode, m_cols_ai mirrors m_cols, providing extra safety in production.
-
Nikita Malyavin authored
Modernize declaration to C++11. Add missing const. Convert TYPE_CODE to constexpr.
-
Nikita Malyavin authored
Assertion `!table->versioned(VERS_TRX_ID)' failed in Write_rows_log_event::binlog_row_logging_function during ONLINE ALTER. trxid-versioned tables can't be replicated. ONLINE ALTER will also be forbidden for these tables.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
The bug is inherent for row-based replication as well. To reproduce, a virtual (not stored) field of a blob type computed from another field of a different blob type is required. The following happens during an update or delete row event: 1. A row is unpacked. 2. Virtual fields are updated. Field b1 stores the pointer in Field_blob::value and references it in table->record[0]. 3. record[0] is stored to record[1] in Rows_log_event::find_row. 4. A new record is fetched from handler. (e.g. ha_rnd_next) 5. Virtual columns are updated (only non-stored). 6. Field b1 receives new value. Old value is deallocated (Field_blob::val_str). 7. record_compare is called. record[0] and record[1] are compared. 8. record[1] contains a reference to a freed value. record_compare is used in replication to find a matching record for update or delete. Virtual columns that are not stored should be definitely skipped both for correctness, and for this bug fix. STORED virtual columns, on the other hand, may be required and shouldn't be skipped. Stored columns are not affected, since they are not updated after handler's fetch.
-
Nikita Malyavin authored
ASAN showed use-after-free in binlog_online_alter_end_trans, during running through thd->online_alter_cache_list. In online_alter_binlog_get_cache_data, new_cache_data was allocated on thd->mem_root, in case of autocommit=1, but this mem_root could be freed in sp_head::execute, upon using stored functions. It appears that thd->transaction->mem_root exists even in single-stmt transaction mode (i.e autocommit=1), so it can be used in all cases. This mem_root will remain valid till the end of transaction, including commit phase.
-
Nikita Malyavin authored
This is the corner case of ONLINE ALTER vs ha_maria vs App-time Periods. When a Delete_rows_event (or update) is executed, a lookup handler may be created, normally to serve long unique index needs, by a call of handler::prepare_for_insert. This function also creates a lookup handler if an application-time period exists in a table. A difference with a usual call of prepare_for_insert is that transactions are disabled for this table during ALTER TABLE. See mysql_trans_prepare_alter_copy_data call in copy_data_between_tables. Then, ha_maria calls _ma_tmp_disable_logging_for_table during ha_maria::external_lock. It never happened so before, that two handlers would be created for write to a single ha_maria table under transactions disabled. Hence, the fix handles this scenario. It could be done otherwise, by not creating this lookup handler (since it's not used anyway during ONLINE ALTER), but architecturally, two handlers should be supported. Avoiding the creation of lookup handler could be done here additionally, but with a cost of slowing down other more generic cases, with an additional check of online alter table active.
-
Nikita Malyavin authored
ONLINE ALTER TABLE uses binlog events like the replication does. Before it was never used outside of replication, so significant change was required. For example, a single event had a statement-like befavior: it locked the tables, opened it, and closed them in the end. But for ONLINE ALTER we use preopened table. A crash scenario is following: lex->query_tables was set to NULL in restore_empty_query_table_list when alter event is applied. Then lex->query_tables->prev_global was write-accessed in LEX::first_lists_tables_same, leading to a segfault. In replication restore_empty_query_table_list would mean resetting lex before next query or event. In ONLINE ALTER TABLE we reuse a locked table between the events, so we should avoid it. Here the need to reset lex state (or close the tables) can be determined by nonzero rgi->tables_to_lock_count. If no table is locked, then event doesn't own the tables. The same was already done before for rgi->slave_close_thread_tables call.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
-
Nikita Malyavin authored
previously, fields with DEFAULTs were allowed just when expression is deterministic. In case of online alter, we should recursively check that underlying fields of expression also either have explicit values, or have DEFAULT following this validity rule.
-
Nikita Malyavin authored
-
Nikita Malyavin authored
-