Re: Proposal for Signal Detection Refactoring

On Tue, Oct 9, 2018 at 4:04 PM Peter Eisentraut <
peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:

> On 01/10/2018 14:00, Chris Travers wrote:
> >
> >
> > On Wed, Sep 26, 2018 at 9:54 AM Chris Travers <chris(dot)travers(at)adjust(dot)com
> > <mailto:chris(dot)travers(at)adjust(dot)com>> wrote:
> >
> >
> >
> > On Tue, Sep 25, 2018 at 3:23 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us
> > <mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us>> wrote:
> >
> > Chris Travers <chris(dot)travers(at)adjust(dot)com
> > <mailto:chris(dot)travers(at)adjust(dot)com>> writes:
> > > However, what I think one could do is use a struct of volatile
> > > sig_atomic_t members and macros for checking/setting. Simply
> > writing a
> > > value is safe in C89 and higher.
> >
> > Yeah, we could group those flags in a struct, but what's the
> point?

>
> >
> > This was one of two things I noticed in my previous patch on
> > interrupts and loops where I wasn't sure what the best practice in
> > our code is.
> >
> > If we don't want to make this change, then would there be any
> > objection to me writing up a README describing the flags, and best
> > practices in terms of checking them in our code based on the current
> > places we use them? If the current approach will be unlikely to
> > change in the future, then at least we can document that the way I
> > went about things is consistent with current best practices so next
> > time someone doesn't really wonder.
> >
> >
> > Attaching a first draft of a readme. Feedback welcome. I noticed
> > further that we used to document signals and what they did with carious
> > processes but that this was removed after 7.0, probably due to
> > complexity reasons.
>
> diff --git a/src/backend/utils/init/README b/src/backend/utils/init/README
> new file mode 100644
> index 0000000000..0472687f53
> --- /dev/null
> +++ b/src/backend/utils/init/README
> @@ -0,0 +1,96 @@
> +src/backend/utils/misc/README
>
> Not only are the directory names mismatched here, but I wonder whether
> this file would be long in either of these directories.
>

I am open to suggestions here. The file was put where the signal stuff
was. Maybe moving it up and calling it signals.README?

>
> +Implementation Notes on Globals and Signal/Event Handling
> +=========================================================
> +
> +The approch to signal handling in PostgreSQL is designed to strictly
> conform
> +with the C89 standard and designed to run with as few platform
> assumptions as
> +possible.
>
> We use C99 now, so why would be design this to be conforming to C89?
>

Can or worms alert: C89 and C99 are functionally the same here. C99
changed the spec in part because every known C89 implementation was
compliant in this area with the C99 spec.

I am fine with bumping that up to C99.

>
>
> More generally, I'd like this material to be code comments. It's the
> kind of stuff that gets outdated before long if it's kept separate.
>

The problem is that code comments are not going to be good places to
document "how do I check for pending actions?" That could be moved to the
main SGML I guess.....

>
> --
> Peter Eisentraut http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

--
Best Regards,
Chris Travers
Head of Database

Tel: +49 162 9037 210 | Skype: einhverfr | www.adjust.com
Saarbrücker Straße 37a, 10405 Berlin