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

Following are some comments,

>+   /* Wait event for SNRU */
Typo in the comment.

>FileWriteback(v->mdfd_vfd, seekpos, (off_t) BLCKSZ * nflush,
This call is inside mdwriteback() which can flush more than one block so

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.lat...@gmail.com>

> On Wed, Mar 8, 2017 at 8:23 AM, Robert Haas <robertmh...@gmail.com> wrote:
>> On Tue, Mar 7, 2017 at 9:32 PM, Amit Kapila <amit.kapil...@gmail.com>
>> wrote:
>> > On Tue, Mar 7, 2017 at 9:16 PM, Robert Haas <robertmh...@gmail.com>
>> wrote:
>> >> On Mon, Mar 6, 2017 at 9:09 PM, Amit Kapila <amit.kapil...@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

Reply via email to