Обсуждение: Re: [HACKERS] logical replication and SIGHUP
On Thu, Apr 06, 2017 at 02:21:29AM +0900, Fujii Masao wrote: > Both launcher and worker don't handle SIGHUP signal and cannot > reload the configuration. I think that this is a bug. Will add this as > an open item barring objection. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
On Mon, Apr 10, 2017 at 11:41 AM, Noah Misch <noah@leadboat.com> wrote: > On Thu, Apr 06, 2017 at 02:21:29AM +0900, Fujii Masao wrote: >> Both launcher and worker don't handle SIGHUP signal and cannot >> reload the configuration. I think that this is a bug. Will add this as >> an open item barring objection. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks. > > [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com After more review, I think that got_SIGTERM should be of type volatile sig_atomic_t in launcher.c or that's not signal-safe. I think as well that for correctness errno should be saved as SetLatch() is called and restored afterwards. Please find attached a patch to address all that. -- Michael VMware vCenter Server www.vmware.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
On 10/04/17 05:20, Michael Paquier wrote: > On Mon, Apr 10, 2017 at 11:41 AM, Noah Misch <noah@leadboat.com> wrote: >> On Thu, Apr 06, 2017 at 02:21:29AM +0900, Fujii Masao wrote: >>> Both launcher and worker don't handle SIGHUP signal and cannot >>> reload the configuration. I think that this is a bug. Will add this as >>> an open item barring objection. >> >> [Action required within three days. This is a generic notification.] >> >> The above-described topic is currently a PostgreSQL 10 open item. Peter, >> since you committed the patch believed to have created it, you own this open >> item. If some other commit is more relevant or if this does not belong as a >> v10 open item, please let us know. Otherwise, please observe the policy on >> open item ownership[1] and send a status update within three calendar days of >> this message. Include a date for your subsequent status update. Testers may >> discover new open items at any time, and I want to plan to get them all fixed >> well in advance of shipping v10. Consequently, I will appreciate your efforts >> toward speedy resolution. Thanks. >> >> [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > After more review, I think that got_SIGTERM should be of type volatile > sig_atomic_t in launcher.c or that's not signal-safe. I think as well > that for correctness errno should be saved as SetLatch() is called and > restored afterwards. Please find attached a patch to address all that. > Looks good to me. Just as a note, we'll have to handle this newly supported config rereads in the async commit patch where we override synchronous_commit GUC, but the config reread will change it back. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
> Looks good to me. Just as a note, we'll have to handle this newly
> supported config rereads in the async commit patch where we override
> synchronous_commit GUC, but the config reread will change it back.
Umm ... you're doing what?
There are mechanisms for making local changes to a GUC. Just stomping
on the variable is not an approved way to do it.
regards, tom lane
On 10/04/17 14:40, Tom Lane wrote:
> Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
>> Looks good to me. Just as a note, we'll have to handle this newly
>> supported config rereads in the async commit patch where we override
>> synchronous_commit GUC, but the config reread will change it back.
>
> Umm ... you're doing what?
>
We are doing:
+ SetConfigOption("synchronous_commit",
+ MySubscription->synccommit ?
"local" : "off",
+ PGC_BACKEND, PGC_S_OVERRIDE);
> There are mechanisms for making local changes to a GUC. Just stomping
> on the variable is not an approved way to do it.
>
I don't remember from top of my head if above is safe enough against
config reread or not, hence the note.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
> On 10/04/17 14:40, Tom Lane wrote:
>> Umm ... you're doing what?
> We are doing:
> + SetConfigOption("synchronous_commit",
> + MySubscription->synccommit ?
> "local" : "off",
> + PGC_BACKEND, PGC_S_OVERRIDE);
That looks fine.
> I don't remember from top of my head if above is safe enough against
> config reread or not, hence the note.
Yes, the override will take care of it ...
regards, tom lane
On 4/9/17 23:20, Michael Paquier wrote: > After more review, I think that got_SIGTERM should be of type volatile > sig_atomic_t in launcher.c or that's not signal-safe. I think as well > that for correctness errno should be saved as SetLatch() is called and > restored afterwards. Please find attached a patch to address all that. committed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services