Re: Tracking wait event for latches

On Wed, Sep 28, 2016 at 9:39 AM, Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> Ok, if they really are independent then shouldn't we take advantage of
> that at call sites where we might be idle but we might also be waiting
> for the network? For example we could do this:
>
> /* Sleep until something happens or we time out */
> WaitLatchOrSocket(MyLatch, wakeEvents,
> MyProcPort->sock, sleeptime,
> pq_is_send_pending() ? WAIT_CLIENT :
> WAIT_ACTIVITY,
> WE_WAL_SENDER_MAIN);

Yes, we can do fancy things with this API. Extensions could do the
same, the important thing being to have generic enough event
categories (take class).

> Isn't WE_WAL_SENDER_WAIT_WAL primarily WAIT_IPC, not WAIT_CLIENT? Or
> perhaps "pq_is_send_pending() ? WAIT_CLIENT : WAIT_IPC".
>
> Actually, I'm still not sold on "Activity" and "Client". I think
> "Idle" and "Network" would be better. Everybody knows intuitively
> what "Idle" means. "Network" is better than "Client" because it
> avoids confusion about user applications vs replication connections or
> clients vs servers.

Personally I am buying the argument of Robert instead. The class of an
event means what it is waiting for, not in which state in it waiting
for something. "Idle" means that the process *is* idle, not that it is
waiting for something.

> + <listitem>
> + <para>
> + <literal>Activity</>: The server process is waiting for some
> + activity to happen on a socket. This is mainly used system processes
> + in their main processing loop. <literal>wait_event</> will identify
> + the type of activity waited for.
> + </para>
> + </listitem>
>
> "The server process is waiting for some activity to happen on a
> socket." Not true: could be a latch, or both.
>
> I think what this category is really getting at is that the server
> process is idle. Everything in it could otherwise go in the IPC or
> Client categories on the basis that it's mainly waiting for a socket
> or a latch, but we're deciding to separate the wait points
> representing "idleness" and put them here.
>
> How about: "The server process is idle. This is used by system
> processes waiting for activity in their main processing loop.
> <literal>wait_event</a> will identify the specific wait point."

You have a better way to word things than I do.

> + <listitem>
> + <para>
> + <literal>Client</>: The server process is waiting for some activity
> + on a socket from a client process, and that the server expects
> + something to happen that is independent from its internal processes.
> + <literal>wait_event</> will identify the type of client activity
> + waited for.
> + </para>
> + </listitem>
>
> Is it worth spelling out that "client process" here doesn't just mean
> user applications, it's also remote PostgreSQL servers doing
> replication? "wait_event" doesn't really identify the type of client
> activity waited for, it identifies the code that is waiting.

Okay, switched to "user applications", and precised that this is a
wait point that wait_event tracks.

> + <listitem>
> + <para>
> + <literal>Extension</>: The server process is waiting for activity
> + in a plugin or an extension. This category is useful for plugin
> + and module developers to track custom waiting points.
> + </para>
> + </listitem>
>
> Plugin, extension, module? How about just "extension"? Why developers?

Let's say "extension module", this is used in a couple of places in the docs.

> + <listitem>
> + <para>
> + <literal>IPC</>: The server process is waiting for some activity
> + from an auxilliary process. <literal>wait_event</> will identify
> + the type of activity waited for.
> + </para>
> + </listitem>
>
> s/auxilliary/auxiliary/, but I wouldn't it be better to say something
> more general like "from another process in the cluster"? Background
> workers are not generally called auxiliary processes, and some of
> these wait points are waiting for those.

There was the same typo in latch.h, still "from another process" looks better.
--
Michael