On 22 November 2016 at 11:35, Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote: > Hello, > > At Mon, 21 Nov 2016 21:39:07 -0500, Robert Haas <robertmh...@gmail.com> wrote > in <ca+tgmozh6m5btmtzzm1vbpfghmengese4urj3-owknu56sk...@mail.gmail.com> >> On Mon, Nov 21, 2016 at 3:21 AM, Craig Ringer <cr...@2ndquadrant.com> wrote: >> > I'm still interested in hearing comments from experienced folks about >> > whether it's sane to do this at all, rather than have similar >> > duplicate signal handling for the walsender. >> >> Well, I mean, if it's reasonable to share code in a given situation, >> that is generally better than NOT sharing code... > > Walsender handles SIGUSR1 completely different way from normal > backends. The procsignal_sigusr1_handler is designed to work as > the peer of SendProcSignal (not ProcSendSignal:). Walsender is > just using a latch to be woken up. It has nothing to do with > SendProcSignal.
Indeed, at the moment it doesn't. I'm proposing to change that, since walsenders doing logical decoding on a standby need to be notified of conflicts with recovery, many of which are the same as for normal user backends and bgworkers. The main exception is snapshot conflicts, where logical decoding walsenders don't care about conflicts with specific tables, they want to be terminated if the effective catalog_xmin on the upstream increases past their needed catalog_xmin. They don't care about non-catalog (or non-heap-catalog) tables. So I expect we'd just want to ignore PROCSIG_RECOVERY_CONFLICT_SNAPSHOT when running a walsender and handle conflict with catalog_xmin increases separately. > IMHO, I don't think walsender is allowed to just share the > backends' handler for a practical reason that pss_signalFlags can > harm. I'm not sure I see the problem. The ProcSignalReason argument to RecoveryConflictInterrupt states that: * Also, because of race conditions, it's important that all the signals be * defined so that no harm is done if a process mistakenly receives one. > If you need to expand the function of SIGUSR1 of walsender, more > thought would be needed. Yeah, I definitely don't think it's as simple as just using procsignal_sigusr1_handler as-is. I expect we'd likely create a new global IsWalSender and ignore some RecoveryConflictInterrupt cases when in a walsender, at least PROCSIG_RECOVERY_CONFLICT_SNAPSHOT, and probably add a new case for catalog_xmin conflicts that's only acted on when IsWalSender. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
