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.
--
Yours truly,
Nikita Malyavin