diff --git a/mysql-test/suite/binlog_in_engine/rpl_oob.result b/mysql-test/suite/binlog_in_engine/rpl_oob.result index 598d2ff47ec..4556349812a 100644 --- a/mysql-test/suite/binlog_in_engine/rpl_oob.result +++ b/mysql-test/suite/binlog_in_engine/rpl_oob.result @@ -15,5 +15,6 @@ SELECT COUNT(*), SUM(a), SUM(b), SUM(LENGTH(c)) FROM t1; COUNT(*) SUM(a) SUM(b) SUM(LENGTH(c)) 2552 33150 128776 5000383 connection master; +connection master; DROP TABLE t1; include/rpl_end.inc diff --git a/mysql-test/suite/binlog_in_engine/rpl_oob.test b/mysql-test/suite/binlog_in_engine/rpl_oob.test index bf4591e171a..563eafdd7f1 100644 --- a/mysql-test/suite/binlog_in_engine/rpl_oob.test +++ b/mysql-test/suite/binlog_in_engine/rpl_oob.test @@ -62,6 +62,20 @@ SELECT COUNT(*), SUM(a), SUM(b), SUM(LENGTH(c)) FROM t1; SELECT COUNT(*), SUM(a), SUM(b), SUM(LENGTH(c)) FROM t1; +--connection master +# Test various event group sizes close to the binlog cache size. +# There was a bug where if the event group fit in the cache without the GTID +# event, but not with the GTID, then the code would incorrectly attempt to +# spill part of the GTID event as oob data and the server would hang on +# incorrect double locking of LOCK_commit_ordered. +--disable_query_log +--let $i= 50 +while ($i > 0) { + eval INSERT INTO t1 VALUES (1000000, $i, REPEAT('#', @@binlog_cache_size - $i*20)); + dec $i; +} +--enable_query_log + # Cleanup. --connection master DROP TABLE t1; diff --git a/sql/log.cc b/sql/log.cc index f1ac0219add..0175ace4d85 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -7224,6 +7224,22 @@ MYSQL_BIN_LOG::write_gtid_event(THD *thd, IO_CACHE *dest, bool standalone, gtid_event.cache_type= is_transactional ? Log_event::EVENT_TRANSACTIONAL_CACHE : Log_event::EVENT_STMT_CACHE; + if (opt_binlog_engine_hton) + { + DBUG_ASSERT(!direct_write); + uint32_t avail= (uint32_t)(dest->write_end - dest->write_pos); + if (unlikely(avail < Gtid_log_event::max_size) && + avail < gtid_event.get_size()) + { + /* + The GTID event doesn't fit in the cache, so we have to spill the + contents as oob event data. + */ + if (my_b_flush_io_cache(dest, 0)) + DBUG_RETURN(true); + } + } + /* Write the event to the binary log. */ DBUG_ASSERT(this == &mysql_bin_log); diff --git a/sql/log_event.h b/sql/log_event.h index d6e757bd36a..ebce81a6064 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -3262,6 +3262,40 @@ public: }; +#ifdef MYSQL_SERVER +/* + This is used to compute a compile-time constant max for the size (in bytes) + of a GTID event (Gtid_log_event::max_size). + + It is carefully written to take boolean parameters corresponding directly + to each conditional in Gtid_log_event::write(), so that the calculation here + will match the actual length computed by write(). + + Please ensure that that any new conditionals added in write() that affect + the event length are similarly extended with a boolean parameter for this + function so future code changes do not introduce incorrect result of this + function. +*/ +static constexpr uint32_t +cap_gtid_event_size(uint32_t proposed_size) +{ + /* This just because std::min is not constexpr in c++11. */ + return LOG_EVENT_HEADER_LEN + + (proposed_size < GTID_HEADER_LEN ? GTID_HEADER_LEN : proposed_size); +} +static constexpr uint32_t +get_gtid_event_size(bool fl_commit_id, bool fl_xa, bool fl_extra, + bool fl_multi_engine, bool fl_alter, + int bq_size, int gt_size) +{ + return cap_gtid_event_size((fl_commit_id ? GTID_HEADER_LEN + 2 : 13) + + (fl_xa ? 6 + bq_size + gt_size : 0) + + (fl_extra ? 1 : 0) + + (fl_multi_engine ? 1 : 0) + + (fl_alter ? 8 : 0)); +} +#endif + /** @class Gtid_log_event @@ -3392,12 +3426,19 @@ public: involving multiple storage engines. No flag and extra data are added to the event when the transaction involves only one engine. */ - static const uchar FL_EXTRA_MULTI_ENGINE_E1= 1; - static const uchar FL_START_ALTER_E1= 2; - static const uchar FL_COMMIT_ALTER_E1= 4; - static const uchar FL_ROLLBACK_ALTER_E1= 8; + static constexpr uchar FL_EXTRA_MULTI_ENGINE_E1= 1; + static constexpr uchar FL_START_ALTER_E1= 2; + static constexpr uchar FL_COMMIT_ALTER_E1= 4; + static constexpr uchar FL_ROLLBACK_ALTER_E1= 8; #ifdef MYSQL_SERVER + static constexpr uint32_t max_size= + get_gtid_event_size(FL_GROUP_COMMIT_ID, + (bool)(FL_PREPARED_XA|FL_COMPLETED_XA), + true, FL_EXTRA_MULTI_ENGINE_E1, + (bool)(FL_COMMIT_ALTER_E1|FL_ROLLBACK_ALTER_E1), + MAXBQUALSIZE, MAXGTRIDSIZE); + Gtid_log_event(THD *thd_arg, uint64 seq_no, uint32 domain_id, bool standalone, uint16 flags, bool is_transactional, uint64 commit_id, bool has_xid= false, bool is_ro_1pc= false); @@ -3430,6 +3471,7 @@ public: } #ifdef MYSQL_SERVER + uint32_t get_size() const noexcept; bool write(Log_event_writer *writer) override; static int make_compatible_event(String *packet, bool *need_dummy_event, ulong ev_offset, enum_binlog_checksum_alg checksum_alg); diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 71affc59040..7981d19b237 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -2973,6 +2973,18 @@ Gtid_log_event::peek(const uchar *event_start, size_t event_len, } +uint32_t +Gtid_log_event::get_size() const noexcept +{ + return get_gtid_event_size(flags2 & FL_GROUP_COMMIT_ID, + flags2 & (FL_PREPARED_XA | FL_COMPLETED_XA), + flags_extra > 0, + flags_extra & FL_EXTRA_MULTI_ENGINE_E1, + flags_extra & (FL_COMMIT_ALTER_E1 | FL_ROLLBACK_ALTER_E1), + xid.bqual_length, xid.gtrid_length); +} + + bool Gtid_log_event::write(Log_event_writer *writer) { @@ -3042,6 +3054,17 @@ Gtid_log_event::write(Log_event_writer *writer) bzero(buf+write_len, GTID_HEADER_LEN-write_len); write_len= GTID_HEADER_LEN; } + /* + Whenever updating this function, make sure that Gtid_log_event::get_size() + still computes the same consistent event length! Do not just rely on this + assertion, in case test coverage is not 100%. + */ + DBUG_ASSERT(DBUG_IF("negate_xid_from_gtid") || + DBUG_IF("negate_xid_data_from_gtid") || + DBUG_IF("negate_alter_fl_from_gtid") || + DBUG_IF("inject_fl_extra_multi_engine_into_gtid") || + write_len + LOG_EVENT_HEADER_LEN == get_size()); + return write_header(writer, write_len) || write_data(writer, buf, write_len) || write_footer(writer); diff --git a/storage/innobase/handler/innodb_binlog.cc b/storage/innobase/handler/innodb_binlog.cc index 84e8c721cf8..1f4872cbd9b 100644 --- a/storage/innobase/handler/innodb_binlog.cc +++ b/storage/innobase/handler/innodb_binlog.cc @@ -302,22 +302,7 @@ struct chunk_data_cache : public chunk_data_base { header_remain= (uint32_t)(p - header_buf); ut_ad((size_t)(p - header_buf) <= sizeof(header_buf)); - if (cache->pos_in_file > binlog_info->out_of_band_offset) { - /* - ToDo: A limitation in mysys IO_CACHE. If I change (reinit_io_cache()) - the cache from WRITE_CACHE to READ_CACHE without seeking out of the - current buffer, then the cache will not be flushed to disk (which is - good for small cache that fits completely in buffer). But then if I - later my_b_seek() or reinit_io_cache() it again and seek out of the - current buffer, the buffered data will not be flushed to the file - because the cache is now a READ_CACHE! The result is that the end of the - cache will be lost if the cache doesn't fit in memory. - - So for now, have to do this somewhat in-elegant conditional flush - myself. - */ - flush_io_cache(cache); - } + ut_ad (cache->pos_in_file <= binlog_info->out_of_band_offset); /* Start with the GTID event, which is put at the end of the IO_CACHE. */ my_bool res= reinit_io_cache(cache, READ_CACHE, binlog_info->gtid_offset, 0, 0);