+ <para>
+ <literal>EventSet</>: The server process is waiting on a socket
+ or a timer. This wait happens in a latch, an inter-process facility
+ using boolean variables letting a process sleep until it is set.
+ Latches support several type of operations, like postmaster death
+ handling, timeout and socket activity lookup, and are a useful
+ replacement for <function>sleep</> where signal handling matters.
+ </para>
This paragraph seems a bit confused. I suggest something more like
this: "The server process is waiting for one or more sockets, a timer
or an interprocess latch. The wait happens in a WaitEventSet,
<productname>PostgreSQL</>'s portable IO multiplexing abstraction."
On Tue, Aug 23, 2016 at 7:01 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
>> On Mon, Aug 22, 2016 at 12:09 PM, Alexander Korotkov
>> <a.korot...@postgrespro.ru> wrote:
>>> Would it be better to use an array here?
>
> Okay, I have switched to an array....
I looked at various macro tricks[1] but they're all pretty unpleasant!
+1 for the simple array with carefully managed order. About that
order...
+const char *const WaitEventNames[] = {
+ "ArchiverMain",
+ "AutoVacuumMain",
+ "BaseBackupThrottle",
+ "BgWorkerStartup",
+ "BgWorkerShutdown",
+ "BgWriterMain",
+ "BgWriterHibernate",
+ "CheckpointerMain",
+ "ExecuteGather",
+ "Extension",
+ "MessageQueuePutMessage",
+ "MessageQueueSend",
+ "MessageQueueReceive",
+ "MessageQueueInternal",
+ "ParallelFinish",
+ "PgStatMain",
+ "ProcSleep",
+ "ProcSignal",
+ "PgSleep",
+ "SecureRead",
+ "SecureWrite",
+ "SSLOpenServer",
+ "SyncRep",
+ "SysLoggerMain",
+ "RecoveryApplyDelay",
+ "RecoveryWalAll",
+ "RecoveryWalStream",
+ "WalReceiverWaitStart",
+ "WalReceiverMain",
+ "WalSenderMain",
+ "WalSenderWaitForWAL",
+ "WalSenderWriteData"
+ "WalWriterMain",
+};
It looks like this array wants to be in alphabetical order, but it
isn't quite. Also, perhaps a compile time assertion about the size of
the array matching EVENT_LAST_TYPE could be useful?
+typedef enum WaitEventIdentifier
+{
+ EVENT_ARCHIVER_MAIN,
+ EVENT_AUTOVACUUM_MAIN,
+ EVENT_BASEBACKUP_THROTTLE,
+ EVENT_BGWORKER_STARTUP,
+ EVENT_BGWORKER_SHUTDOWN,
+ EVENT_BGWRITER_MAIN,
+ EVENT_BGWRITER_HIBERNATE,
+ EVENT_CHECKPOINTER_MAIN,
+ EVENT_EXECUTE_GATHER,
+ EVENT_EXTENSION,
+ EVENT_MQ_PUT_MESSAGE,
+ EVENT_MQ_SEND_BYTES,
+ EVENT_MQ_RECEIVE_BYTES,
+ EVENT_MQ_WAIT_INTERNAL,
+ EVENT_PARALLEL_WAIT_FINISH,
+ EVENT_PGSTAT_MAIN,
+ EVENT_PROC_SLEEP,
+ EVENT_PROC_SIGNAL,
+ EVENT_PG_SLEEP,
+ EVENT_SECURE_READ,
+ EVENT_SECURE_WRITE,
+ EVENT_SSL_OPEN_SERVER,
+ EVENT_SYNC_REP,
+ EVENT_SYSLOGGER_MAIN,
+ EVENT_RECOVERY_APPLY_DELAY,
+ EVENT_RECOVERY_WAL_ALL,
+ EVENT_RECOVERY_WAL_STREAM,
+ EVENT_WAL_RECEIVER_WAIT_START,
+ EVENT_WAL_RECEIVER_MAIN,
+ EVENT_WAL_SENDER_WRITE_DATA,
+ EVENT_WAL_SENDER_MAIN,
+ EVENT_WAL_SENDER_WAIT_WAL,
+ EVENT_WALWRITER_MAIN
+} WaitEventIdentifier;
This is also nearly but not exactly in alphabetical order
(EVENT_PROC_SIGNAL comes after EVENT_PROC_SLEEP for example). But
it's not currently possible to have the strings and the enumerators
both in alphabetical order because they're not the same, even
accounting for camel-case to upper-case transliteration. I think at
least one of them should be sorted. Shouldn't they match fully and
then *both* be sorted alphabetically? For example
"MessageQueueInternal" doesn't match EVENT_MQ_WAIT_INTERNAL. Then
there are some cases where I'd expect underscores for consistency with
other enumerators and with the corresponding camel-case strings: you
have EVENT_WAL_SENDER_MAIN, but EVENT_WALWRITER_MAIN.
The documentation is in a slightly different order again but also not
exactly alphabetical: for example ProcSleep is listed before
ProcSignal.
Sorry if this all sounds terribly picky but I think we should try to
be strictly systematic here.
>>> EventIdentifier seems too general name for me, isn't it? Could we name it
>>> WaitEventIdentifier? Or WaitEventId for shortcut?
>
> ... And WaitEventIdentifier.
+1 from me too for avoiding the overly general term 'event'. It does
seem a little odd to leave the enumerators names as EVENT_... though;
shouldn't these be WAIT_EVENT_... or WE_...? Or perhaps you could
consider WaitPointIdentifier and WP_SECURE_READ or
WaitEventPointIdentifier and WEP_SECURE_READ, if you buy my earlier
argument that what we are really naming here is point in the code
where we wait, not the events we're waiting for. Contrast with
LWLocks where we report the lock that you're waiting for, not the
place in the code where you're waiting for that lock.
On Wed, Aug 3, 2016 at 1:31 AM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> Finally, I have changed the patch so as it does not track "Latch"
> events, but "EventSet" events instead, per the suggestion of Thomas.
> "WaitEventSet" is too close to wait_event in my taste so I shortened
> the suggeston.
This is good, because showing "Latch" when we were really waiting for
a socket was misleading.
On the other hand, if we could *accurately* report whether it's a
"Latch", "Socket" or "Latch|Socket" that we're waiting for, it might
be cool to do that instead. One way to do that would be to use up
several class IDs: WAIT_EVENT_LATCH, WAIT_EVENT_LATCH_OR_SOCKET,
WAIT_EVENT_SOCKET (or perhaps WAIT_EVENT_LATCH | WAIT_EVENT_SOCKET,
reserving 2 or 3 upper bits from the 8 bit class ID for this). Then
we could figure out the right class ID by looking at set->latch and
set->nevents (perhaps an extra boolean would be needed to record
whether postmaster death is in there so we could deduce whether there
are any sockets). It would be interesting specifically for the case
of FDWs where it would be nice to be able to see clearly that it's
waiting for a remote server ("Socket"). It may also be interesting to
know if there is a timeout. Postmaster death doesn't seem newsworthy,
we're nearly always also waiting for that exceptional event so it'd
just be clutter to report it.
[1]
http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
