On Mon, Jul 23, 2018 at 04:04:42PM +0900, 임명규 wrote: > This proposal is about recording additional statistics of wait events.
You should avoid sending things in html format, text format being recommended on those mailing lists... The patch applies after using patch -p0 by the way. I would recommend that you generate your patches using "git format-patch". Here are general guidelines on the matter: https://wiki.postgresql.org/wiki/Submitting_a_Patch Please study those guidelines, those are helpful if you want to get yourself familiar with community process. I have comments about your patch. First, I don't think that you need to count precisely the number of wait events triggered as usually when it comes to analyzing a workload's bottleneck what counts is a periodic *sampling* of events, patterns which can be fetched already from pg_stat_activity and stored say in a different place. This can be achieved easily by using a cron job with an INSERT SELECT query which adds data on a relation storing the event counts. I took the time to look at your patch, and here is some feedback. This does not need a configure switch. I assume that what you did is good to learn the internals of ./configure though. There is no documentation. What does the patch do? What is it useful for? + case PG_WAIT_IPC: + arrayIndex = NUM_WAIT_LWLOCK + + NUM_WAIT_LOCK + NUM_WAIT_BUFFER_PIN + + NUM_WAIT_ACTIVITY + NUM_WAIT_CLIENT + + NUM_WAIT_EXTENSION + eventId; + break; This is ugly and unmaintainable style. You could perhaps have considered an enum instead. pg_stat_get_wait_events should be a set-returning function, which you could filter at SQL level using a PID, so no need of it as an argument. What's the performance penalty? I am pretty sure that this is measurable as wait events are stored for a backend for each I/O operation as well, and you are calling a C routine within an inlined function which is designed to be light-weight, doing only a four-byte atomic operation. -- Michael
signature.asc
Description: PGP signature