Re: [Maria-developers] 17548c8a8b6: MDEV-25477 Auto-create breaks replication when triggering event was not replicated
Hi, Aleksey, Basically, it's fine, but I still didn't like that there's a new flag in THD to mark a statement that has to be logged. So I tried to change to log_current_statement. I know you tried that and you told me what tests fails, rpl.create_or_replace_*. I debugged that a bit, and I think the problem is that select_create inherits from select_insert, so CREATE ... SELECT runs your code that is only supposed to be run for INSERT ODKU or REPLACE. This change makes all tests pass (after removing vers_created_partitions): @@ -4225,7 +4225,8 @@ bool select_insert::prepare_eof() thd->clear_error(); else errcode= query_error_code(thd, killed_status == NOT_KILLED); - StatementBinlog stmt_binlog(thd, thd->binlog_need_stmt_format(trans_table)); + StatementBinlog stmt_binlog(thd, !can_rollback_data() && + thd->binlog_need_stmt_format(trans_table)); res= thd->binlog_query(THD::ROW_QUERY_TYPE, thd->query(), thd->query_length(), trans_table, FALSE, FALSE, errcode); On Apr 10, Aleksey Midenkov wrote:
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi Sergei! On Tue, Apr 12, 2022 at 12:58 PM Sergei Golubchik <serg@mariadb.org> wrote:
I attached the patch and this still fails rpl.create_or_replace_row. Why don't you like vers_created_partitions? It might be useful info for debugging and for new features.
-- @midenok
Hi, Aleksey,
I attached the patch and this still fails rpl.create_or_replace_row.
small mistake in the patch:
if you put here log_current_statement && !binlog_get_pending_rows_event(is_transactional) then all tests pass.
}
bool binlog_for_noop_dml(bool transactional_table);
Why don't you like vers_created_partitions? It might be useful info for debugging and for new features.
I have a second patch that renames flags to have a very well-defined meaning and name. OPTION_KEEP_LOG -> OPTION_BINLOG_THIS_TRX log_current_statement -> OPTION_BINLOG_THIS_STMT the first flag means that when a transaction is rolled back, it's written to binlog with ROLLBACK at the end, not discarded. The second means that the statement is written to binlog (or binlog trx cache) even if it otherwise wouldn't be. So, two flags. Clear names and semantics. "vers_created_partitions" fits into the second use case, so one should use OPTION_BINLOG_THIS_STMT for it. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi, Aleksey, On Apr 18, Aleksey Midenkov wrote:
It's really just renaming, almost nothing else. But here it is, attached. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi Sergei! On Tue, Apr 19, 2022 at 11:13 AM Sergei Golubchik <serg@mariadb.org> wrote:
I'd keep log_current_statement() like this: --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3562,4 +3562,8 @@ class THD: public THD_count, /* this must be first */ /* set during loop of derived table processing */ bool derived_tables_processing; bool tablespace_op; /* This is TRUE in DISCARD/IMPORT TABLESPACE */ + bool log_current_statement() const + { + return variables.option_bits & OPTION_BINLOG_THIS_STMT; + } /**
-- @midenok
Hi, Aleksey, Okay, sure. I was going to push it after the release. And in 10.3 to simplify future merges. On Apr 20, Aleksey Midenkov wrote:
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Sergei, I updated the branch accordingly. There is your patch fa444975d0f where I added log_current_statement(). On Wed, Apr 20, 2022 at 1:12 PM Sergei Golubchik <serg@mariadb.org> wrote:
-- @midenok
participants (2)
-
Aleksey Midenkov -
Sergei Golubchik