Re: [Proposal] Add accumulated statistics for wait event
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: 임명규 <myungkyu.lim@samsung.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, 홍도형 <don.hong@samsung.com>, 손우성 <woosung.sohn@samsung.com>
Date: 2018-07-23T13:57:25Z
Lists: pgsql-hackers
Michael Paquier <michael@paquier.xyz> writes: > This does not need a configure switch. It probably is there because the OP realizes that most people wouldn't accept having this code compiled in. > 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. On machines with slow gettimeofday(), I suspect the cost of this patch would be staggering. Even with relatively fast gettimeofday, it doesn't look acceptable for calls in hot code paths (for instance, lwlock.c). A bigger problem is that it breaks stuff. There are countless calls to pgstat_report_wait_start/pgstat_report_wait_end that assume they have no side-effects (for example, on errno) and can never fail. I wouldn't trust GetCurrentTimestamp() for either. If the report_wait calls can't be dropped into code with *complete* certainty that they're safe, that's a big cost. Why exactly is this insisting on logging timestamps and not, say, just incrementing a counter? I think doing it like this is almost certain to end in rejection. regards, tom lane