On Fri, Mar 15, 2024 at 12:22 PM Kristian Nielsen <kniel...@knielsen-hq.org> wrote:
> Brandon Nesterenko <brandon.nestere...@mariadb.com> writes: > > >> 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. > I see your point. Though likely not too important, it adds complexity to the code, so I've reverted that part (originally added by MDEV-11853). The PR (PR 3089 / branch 10.6-MDEV-33551) has been updated per your latest review. Brandon
_______________________________________________ developers mailing list -- developers@lists.mariadb.org To unsubscribe send an email to developers-le...@lists.mariadb.org
