Brandon Nesterenko <brandon.nesterenko@mariadb.com> writes:
Sure, I'll keep COND_binlog around and use that (the main thread doesn't have a THD, and creating one just to enqueue into the list would be more overhead than keeping COND_binlog).
Agree, no need to create another THD.
But once we fix the await_all_slave_replies(), do we even need the is_awaiting_semi_sync_ack flag any more? If so, why? If not, then this point becomes moot anyway.
Ah, right, this is actually a bug as is, as when we are in kill_threads_phase_1 we test for is_awaiting_semi_sync_ack, which would only be set on the leader binlog thread.. So then in kill_threads_phase_1, we'd just check if the THD is in Active_tranx to see if we should skip the kill.
I still don't understand why we want to skip the kill? I think the point of SHUTDOWN WAIT FOR ALL SLAVES is to ensure that slaves have time to get all transactions that were binlogged on the master, before it shuts down. This will be ensured by await_all_slave_replies(), which I think can be done without relying on any other session/THD waiting for specific ACKs. In fact, any session could in principle be killed anyway (by the user), and we would still need await_all_slave_replies() to work, right? Or a session could be just before or after waiting for its ACK, and then it will be killed anyway. So I don't understand why the shutdown should skip killing these specific sessions in this specific case? On the other hand, I'm not sure this is that important, can't really think of any harm in skipping the kill either. I'm mostly trying to understand the reason for doing this. - Kristian.