You forgot to address the following:
if (do_commit) { - // do not set STMT_END for last event to leave table open in
altering thd
- error= binlog_flush_pending_rows_event(thd, false, true, binlog, &cache); - if (is_ending_transaction) + /* + If the cache wasn't reinited to write, then it remains empty after + the last write. + */ + if (cache.cache_log.type != READ_CACHE && !error)
this is a confusing new condition. are you trying to avoid locking a mutex for an empty cache? If yes, you can check my_b_bytes_in_cache(), that'd be more clear.
Pardon. My subconscious ignores uncomfortable questions:) No, i didn't. reinit_io_cache is called inside
error= binlog->write_cache(thd, &cache.cache_log);
This reinit function behaves differently when it reinits WRITE_CACHE -> READ_CACHE vs READ_CACHE -> READ_CACHE. The latter happens when we issue COMMIT when non-trans trable was involved. This simply leads to a crash. I didn't investigate for something smarter, so yes, checking my_b_bytes_in_cache() may also work, I suppose. Let's see... Anyway, calling Event_log::write_cache with cache.cache_log.type == READ_CACHE is error-prone, so I'll leave it as an assertion. Sergei, once I reiterated to this MDEV, found out that I was wrong about this:
then we'll have to clear the pending event.
The pending event is deleted during truncate() (called from restore_prev_position()). Also I noticed another reinit_io_cache there. I think it's not vulnerable to the problem above, but it may end up in a real OS ftell. Even though our file is temporary, I will add at least an assertion to the zero reinit_io_cache result. The commit to see is 4747e9cd584ee <https://github.com/MariaDB/server/commit/4747e9cd584ee982b0033061957d76eb09b13554> . -- Yours truly, Nikita Malyavin