On Thu, Apr 5, 2018 at 7:30 PM, Magnus Hagander <mag...@hagander.net> wrote:
> On Thu, Apr 5, 2018 at 5:08 PM, Andrey Borodin <x4...@yandex-team.ru> > wrote: > >> >> >> > 5 апр. 2018 г., в 19:58, Magnus Hagander <mag...@hagander.net> >> написал(а): >> > >> > >> > >> > On Thu, Apr 5, 2018 at 4:55 PM, Andrey Borodin <x4...@yandex-team.ru> >> wrote: >> > >> > >> > > 5 апр. 2018 г., в 14:33, Tomas Vondra <tomas.von...@2ndquadrant.com> >> написал(а): >> > > >> > > This patch version seems fine to me. I'm inclined to mark it RFC. >> > +1 >> > The patch works fine for me. I've tried different combinations of >> backend cancelation and the only suspicious thing I found is that you can >> start multiple workers by cancelling launcher and not cancelling worker. Is >> it problematic behavior? If we run pg_enable_data_checksums() it checks for >> existing launcher for a reason, m.b. it should check for worker too? >> > >> > I don't think it's a problem in itself -- it will cause pointless work, >> but not actually cause any poroblems I think (whereas duplicate launchers >> could cause interesting things to happen). >> > >> > How did you actually cancel the launcher to end up in this situation? >> select pg_enable_data_checksums(10000,1); >> select pg_sleep(0.1); >> select pg_cancel_backend(pid),backend_type from pg_stat_activity where >> backend_type ~ 'checksumhelper launcher' ; >> select pg_enable_data_checksums(10000,1); >> select pg_sleep(0.1); >> select pg_cancel_backend(pid),backend_type from pg_stat_activity where >> backend_type ~ 'checksumhelper launcher' ; >> select pg_enable_data_checksums(10000,1); >> select pg_sleep(0.1); >> select pg_cancel_backend(pid),backend_type from pg_stat_activity where >> backend_type ~ 'checksumhelper launcher' ; >> >> select pid,backend_type from pg_stat_activity where backend_type >> ~'checks'; >> pid | backend_type >> -------+----------------------- >> 98587 | checksumhelper worker >> 98589 | checksumhelper worker >> 98591 | checksumhelper worker >> (3 rows) >> >> There is a way to shoot yourself in a leg then by calling >> pg_disable_data_checksums(), but this is extremely stupid for a user >> > > > Ah, didn't consider query cancel. I'm not sure how much we should > actually care about it, but it's easy enough to trap that signal and just > do a clean shutdown on it, so I've done that. > > PFA a patch that does that, and also rebased over the datallowconn patch > just landed (which also removes some docs). > > I have now pushed this latest version with some minor text adjustments and a catversion bump. Thanks for all the reviews! -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
