On Thu, Sep 8, 2011 at 10:03 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Sep 8, 2011 at 6:14 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> OTOH, new function enables users to monitor the delay as a timestamp.
>> For users, a timestamp is obviously easier to handle than LSN, and the delay
>> as a timestamp is more intuitive. So, I think that it's worth adding
>> something like pg_last_xact_insert_timestamp into core for improvement
>> of user-friendness.
>
> It seems very nice from a usability point of view, but I have to agree
> with Simon's concern about performance. Actually, as of today,
> WALInsertLock is such a gigantic bottleneck that I suspect the
> overhead of this additional bookkeeping would be completely
> unnoticeable.
The patch I've posted adds one acquisition and release of spinlock per
a commit or abort. But it's done outside of WALInsertLock. So I don't
think that the patch degrades a performance.
> But I'm still reluctant to add more centralized
> spinlocks that everyone has to fight over, having recently put a lot
> of effort into getting rid of some of the ones we've traditionally
> had.
You are against adding new acquisition and release of spinlock itself
even if it has nothing to do with WALInsertLock? The patch uses
XLogCtl->info_lck spinlock to save the last insert timestamp in the
shmem. XLogCtl->info_lck already protects many shmem variables
related to XLOG. So using XLogCtl->info_lck additionally might make
its contention heavier and degrade a performance. If the patch
defines new spinlock and uses it to save the timestamp to avoid
such a contention, you feel satisfied with the patch?
Another idea to avoid spinlock contention is save the timestamp in
PgBackendStatus (which contains information for pg_stat_activity).
This enables us to write and read the timestamp without spinlock.
Comments?
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center