Re: [HACKERS] wait events for disk I/O - Mailing list pgsql-hackers

Hello,

I applied and tested this patch on latest sources and it works fine.

Following are some comments,

>+   /* Wait event for SNRU */
>+   WAIT_EVENT_READ_SLRU_PAGE,

Typo in the comment.

>FileWriteback(v->mdfd_vfd, seekpos, (off_t) BLCKSZ * nflush, WAIT_EVENT_FLUSH_DATA_BLOCK);
This call is inside mdwriteback() which can flush more than one block so should  WAIT_EVENT _FLUSH_DATA_BLOCK
be renamed to WAIT_EVENT_FLUSH_DATA_BLOCKS?

Should calls to write() in following functions be tracked too?
 qtext_store()  - This is related to pg_stat_statements
 

dsm_impl_mmap() - This is in relation to creating dsm segments.

write_auto_conf_file()-  This is called when updated configuration parameters are

                                     written to a temp file.

On Wed, Mar 8, 2017 at 4:50 PM, Rushabh Lathia <rushabh.lathia@gmail.com> wrote:

On Wed, Mar 8, 2017 at 8:23 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Mar 7, 2017 at 9:32 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Tue, Mar 7, 2017 at 9:16 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Mon, Mar 6, 2017 at 9:09 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>> Sure, if you think both Writes and Reads at OS level can have some
>>> chance of blocking in obscure cases, then we should add a wait event
>>> for them.
>>
>> I think writes have a chance of blocking in cases even in cases that
>> are not very obscure at all.
>
> Point taken for writes, but I think in general we should have some
> criteria based on which we can decide whether to have a wait event for
> a particular call. It should not happen that we have tons of wait
> events and out of which, only a few are helpful in most of the cases
> in real-world scenarios.

Well, the problem is that if you pick and choose which wait events to
add based on what you think will be common, you're actually kind of
hosing yourself. Because now when something uncommon happens, suddenly
you don't get any wait event data and you can't tell what's happening.
I think the number of new wait events added by Rushabh's patch is
wholly reasonable.  Yeah, some of those are going to be a lot more
common than others, but so what?  We add wait events so that we can
find out what's going on.  I don't want to sometimes know when a
backend is blocked on an I/O.  I want to ALWAYS know.

Yes, I agree with Robert. Knowing what we want and what we don't

want is difficult to judge. Something which we might think its not useful

information, and later of end up into situation where we re-think about

adding those missing stuff is not good. Having more information about

the system, specially for monitoring purpose is always good.

I am attaching  another version of the patch, as I found stupid mistake

in the earlier version of patch, where I missed to initialize initial value to
WaitEventIO enum. Also earlier version was not getting cleanly apply on
the current version of sources.

--

Rushabh Lathia
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers