Re: wait events for disk I/O

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.

Thank you,
Rahila Syed

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

>
>
> On Wed, Mar 8, 2017 at 8:23 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> On Tue, Mar 7, 2017 at 9:32 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
>> wrote:
>> > On Tue, Mar 7, 2017 at 9:16 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
>> wrote:
>> >> On Mon, Mar 6, 2017 at 9:09 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>