1. 15 Sep, 2023 3 commits
    • Yuchen Pei's avatar
      MDEV-32157 MDEV-28856 Spider: Tests, documentation, small fixes and cleanups · fb4434ee
      Yuchen Pei authored
      Removed some redundant hint related string literals from
      spd_db_conn.cc
      
      Clean up SPIDER_PARAM_*_[CHAR]LEN[S]
      
      Adding tests covering monitoring_kind=2. What it does is that it reads
      from mysql.spider_link_mon_servers with matching db_name, table_name,
      link_id, and does not do anything about that...
      
      How monitoring_* can be useful: in the deprecated spider high
      availability feature, when one remote fails, spider will try another
      remote, which apparently makes use of these table parameters.
      
      A test covering the query_cache_sync table param. Some further tests
      on some spider table params.
      
      Wrapper should be case insensitive.
      
      Code documentation on spider priority binary tree.
      
      Add an assertion that static_key_cardinality is always -1. All tests
      pass still
      fb4434ee
    • Yuchen Pei's avatar
      MDEV-32157 MDEV-28856 Spider: drop server in tests · 4d2fff63
      Yuchen Pei authored
      This helps eliminate "server exists" failures
      
      Also, spider/bugfix.mdev_29676, when enabled after MDEV-29525 is
      pushed will fail because we have not --recorded the result. But the
      failure will only emerge when working on MDEV-31138 where we manually
      re-enable this test, so let's worry about that then.
      4d2fff63
    • Yuchen Pei's avatar
      MDEV-31117 Fix spider connection info parsing · d0cdb966
      Yuchen Pei authored
      Spider connection string is a comma-separated parameter definitions,
      where each definition is of the form "<param_title> <param_value>",
      where <param_value> is quote delimited on both ends, with backslashes
      acting as an escaping prefix.
      
      Despite the simple syntax, the existing spider connection string
      parser was poorly-written, complex, hard to reason and error-prone,
      causing issues like the one described in MDEV-31117. For example it
      treated param title the same way as param value when assigning, and
      have nonsensical fields like delim_title_len and delim_title.
      
      Thus as part of the bugfix, we clean up the spider comment connection
      string parsing, including:
      
      - Factoring out some code from the parsing function
      - Simplify the struct `st_spider_param_string_parse`
      - And any necessary changes caused by the above changes
      d0cdb966
  2. 11 Sep, 2023 1 commit
  3. 08 Sep, 2023 1 commit
  4. 24 Aug, 2023 2 commits
  5. 21 Aug, 2023 1 commit
  6. 17 Aug, 2023 2 commits
  7. 15 Aug, 2023 30 commits
    • Nikita Malyavin's avatar
      MDEV-31812 Add switch to old_mode to disable non-locking ALTER · 8aa1a9e6
      Nikita Malyavin authored
      Add LOCK_ALTER_TABE_COPY bit to old_mode. Disables online copy by default,
      but still allows to force it with explicit lock=none
      8aa1a9e6
    • Nikita Malyavin's avatar
      MDEV-31804 Assertion `thd->m_transaction_psi == __null' fails · a1af5255
      Nikita Malyavin authored
      ... upon replicating online ALTER
      
      When an online event is applied and slave_exec_mode is idempotent,
      Write_rows_log_event::do_before_row_operations had reset
      thd->lex->sql_command to SQLCOM_REPLACE.
      
      This led to that a statement was detected as a row-type during binlogging,
      and was logged as not standalone.
      
      So the corresponding Gtid_log_event, when applied on replica, did not exit
      early and created a new PSI transaction. Hence the difference with
      non-online ALTER.
      a1af5255
    • Nikita Malyavin's avatar
      Cleanup: make slave_exec_mode of its enum type and pack Log_event better · c373e6c3
      Nikita Malyavin authored
      Pack these fields together:
      event_owns_temp_buf
      cache_type
      slave_exec_mode
      checksum_alg
      
      Make them bitfields to fit a single 2-byte hole.
      
      This saves 24 bytes per event.
      
      SLAVE_EXEC_MODE_LAST_BIT is rewritten as
      
      > SLAVE_EXEC_MODE_LAST= SLAVE_EXEC_MODE_IDEMPOTENT
      
      to avoid a false-positive -Wbitfield-enum-conversion warning:
      Bit-field 'slave_exec_mode' is not wide enough to store all enumerators of
      'enum_slave_exec_mode'.
      c373e6c3
    • Nikita Malyavin's avatar
      MDEV-31838 Assertion fails upon replication online alter with MINIMAL row · 982b6895
      Nikita Malyavin authored
      Replica honors its own binlog_row_image value when it sets up read_set.
      When a slave thread is applying replicated row events in parallel with the
      running online alter, we need all columns to be read from the table
      (for the online alter logged row event) not only those that were present in
      the pre-image for the replicated row event.
      
      Avoid shrinking the set when online alter is running, i.e. leave it all set
      982b6895
    • Nikita Malyavin's avatar
      MDEV-31781 ALTER TABLE ENGINE=s3 fails · c4adaed0
      Nikita Malyavin authored
      s3 is read-only. No need to update to a read-only engine with LOCK=NONE.
      c4adaed0
    • Nikita Malyavin's avatar
      MDEV-31777 ER_GET_ERRNO upon online alter on CONNECT table · 30c965f8
      Nikita Malyavin authored
      Forbid Online for CONNECT.
      30c965f8
    • Nikita Malyavin's avatar
      MDEV-31631 Adding auto-increment to table with history online misbehaves · 44ca37ef
      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
      44ca37ef
    • Nikita Malyavin's avatar
      MDEV-31776 Online ALTER reports the number of affected rows incorrectly · e026a366
      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.
      e026a366
    • Nikita Malyavin's avatar
      MDEV-31775 Server crash upon online alter on sequence · 9c855425
      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.
      9c855425
    • Nikita Malyavin's avatar
    • Nikita Malyavin's avatar
    • Andrei's avatar
      MDEV-31755 Replica's DML event deadlocks wit online alter table · bac8f189
      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)
      bac8f189
    • Nikita Malyavin's avatar
      e1b9ab19
    • Nikita Malyavin's avatar
      MDEV-31677 Assertion failed upon online ALTER with binlog_row_image=NOBLOB · 70491fb0
      Nikita Malyavin authored
      Make binlog_prepare_row_images accept image type as an argument.
      70491fb0
    • Nikita Malyavin's avatar
    • Nikita Malyavin's avatar
      MDEV-31646 Online alter applies binlog cache limit to cache writes · d5e59c98
      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.
      d5e59c98
    • Nikita Malyavin's avatar
      MDEV-31601 Some ALTER TABLEs fail ... with a wrong error message · 2cecb5a6
      Nikita Malyavin authored
      Report correct algorithm in the error message.
      2cecb5a6
    • Nikita Malyavin's avatar
      follow-up MDEV-30430: fix versioning.rpl · 8a165d7c
      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.
      8a165d7c
    • Nikita Malyavin's avatar
      fix main.mysql57_virtual, main.alter_table, innodb.alter_algorithm · 43cb98b4
      Nikita Malyavin authored
      The correct (best) algorithm is now chosen for ALGORITHM=DEFAULT
      and alter_algorithm=DEFAULT
      
      See also MDEV-30906
      43cb98b4
    • Nikita Malyavin's avatar
      MDEV-30984 Online ALTER table is denied with non-informative error messages · c382de72
      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.
      c382de72
    • Nikita Malyavin's avatar
    • Nikita Malyavin's avatar
      Add const to alloc-related thd methods · 500787c7
      Nikita Malyavin authored
      Also update abi declarations. The abi itself is unchanged, since const
      doesn't affect C-style exported name.
      500787c7
    • Nikita Malyavin's avatar
      MDEV-30987 main.alter_table_online times out with view-protocol · d7b0c6d8
      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
      d7b0c6d8
    • Nikita Malyavin's avatar
      MDEV-31059 "Slave SQL" errors upon concurrent DML and erroneous ALTER · 55d1645d
      Nikita Malyavin authored
      Skip more rpl-related error handling.
      Also move the error check inside if (table) -- otherwise the error
      should be handled already.
      55d1645d
    • Nikita Malyavin's avatar
      refactor unpack_row · 3a42f286
      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.
      3a42f286
    • Nikita Malyavin's avatar
      500379cf
    • Nikita Malyavin's avatar
      MDEV-31058 ER_KEY_NOT_FOUND upon concurrent CHANGE column autoinc and DML · da277396
      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.
      da277396
    • Nikita Malyavin's avatar
    • Nikita Malyavin's avatar
      MDEV-30949 Direct leak in binlog_online_alter_end_trans · ecb9db4c
      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.
      ecb9db4c
    • Nikita Malyavin's avatar
      MDEV-31043 ER_KEY_NOT_FOUND upon concurrent ALTER and transaction · 0695f7dd
      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.
      0695f7dd