Re: Online enabling of checksums

On Thu, Apr 5, 2018 at 5:08 PM, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:

>
>
> > 5 апр. 2018 г., в 19:58, Magnus Hagander <magnus(at)hagander(dot)net>
> написал(а):
> >
> >
> >
> > On Thu, Apr 5, 2018 at 4:55 PM, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
> wrote:
> >
> >
> > > 5 апр. 2018 г., в 14:33, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)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).

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>