Re: [HACKERS] kqueue

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Mateusz Guzik <mjguzik(at)gmail(dot)com>, Keith Fiske <keith(at)omniti(dot)com>, Matteo Beccati <php(at)beccati(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Marko Tiikkaja <marko(at)joh(dot)to>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Noah Misch <noah(at)leadboat(dot)com>
Subject: Re: [HACKERS] kqueue
Date: 2018-09-28 12:19:58
Message-ID: CAEepm=0fCRRkaDombHrMXH4NXTeRVfBtkO8rbuh0xza0ovhzxQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Sep 28, 2018 at 11:09 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2018-09-28 10:55:13 +1200, Thomas Munro wrote:
> > Matteo Beccati reported a 5-10% performance drop on a
> > low-end Celeron NetBSD box which we have no explanation for, and we
> > have no reports from server-class machines on that OS -- so perhaps we
> > (or the NetBSD port?) should consider building with WAIT_USE_POLL on
> > NetBSD until someone can figure out what needs to be fixed there
> > (possibly on the NetBSD side)?
>
> Yea, I'm not too worried about that. It'd be great to test that, but
> otherwise I'm also ok to just plonk that into the template.

Thanks for the review! Ok, if we don't get a better idea I'll put
this in src/template/netbsd:

CPPFLAGS="$CPPFLAGS -DWAIT_USE_POLL"

> > @@ -576,6 +592,10 @@ CreateWaitEventSet(MemoryContext context, int nevents)
> > if (fcntl(set->epoll_fd, F_SETFD, FD_CLOEXEC) == -1)
> > elog(ERROR, "fcntl(F_SETFD) failed on epoll descriptor: %m");
> > #endif /* EPOLL_CLOEXEC */
> > +#elif defined(WAIT_USE_KQUEUE)
> > + set->kqueue_fd = kqueue();
> > + if (set->kqueue_fd < 0)
> > + elog(ERROR, "kqueue failed: %m");
> > #elif defined(WAIT_USE_WIN32)
>
> Is this automatically opened with some FD_CLOEXEC equivalent?

No. Hmm, I thought it wasn't necessary because kqueue descriptors are
not inherited and backends don't execve() directly without forking,
but I guess it can't hurt to add a fcntl() call. Done.

> > + *((WaitEvent **)(&k_ev->udata)) = event;
>
> I'm mildly inclined to hide that behind a macro, so the other places
> have a reference, via the macro definition, to this too.

Done.

> > + if (rc < 0 && event->events == WL_POSTMASTER_DEATH && errno == ESRCH)
> > + {
> > + /*
> > + * The postmaster is already dead. Defer reporting this to the caller
> > + * until wait time, for compatibility with the other implementations.
> > + * To do that we will now add the regular alive pipe.
> > + */
> > + WaitEventAdjustKqueueAdd(&k_ev[0], EVFILT_READ, EV_ADD, event);
> > + rc = kevent(set->kqueue_fd, &k_ev[0], count, NULL, 0, NULL);
> > + }
>
> That's, ... not particulary pretty. Kinda wonder if we shouldn't instead
> just add a 'pending_events' field, that we can check at wait time.

Done.

> > +/* Define to 1 if you have the `kqueue' function. */
> > +#undef HAVE_KQUEUE
> > +

> Should adjust pg_config.win32.h too.

Done.

--
Thomas Munro
http://www.enterprisedb.com

Attachment Content-Type Size
0001-Add-kqueue-2-support-for-WaitEventSet-v11.patch application/octet-stream 17.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Banck 2018-09-28 13:06:41 Re: Progress reporting for pg_verify_checksums
Previous Message Amit Kapila 2018-09-28 11:59:24 Re: clarify documentation of BGW_NEVER_RESTART ?