Re: Function to know last log write timestamp

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tatsuo Ishii <ishii(at)postgresql(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Function to know last log write timestamp
Date: 2014-08-15 11:17:38
Message-ID: CAHGQGwE_CN7PV4oYyAb0URssPK_R79tKH+nJcXn29VKucPi9KA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Aug 15, 2014 at 3:40 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-08-14 14:37:22 -0400, Robert Haas wrote:
>> On Thu, Aug 14, 2014 at 2:21 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> > On 2014-08-14 14:19:13 -0400, Robert Haas wrote:
>> >> That's about the idea. However, what you've got there is actually
>> >> unsafe, because shmem->counter++ is not an atomic operation. It reads
>> >> the counter (possibly even as two separate 4-byte loads if the counter
>> >> is an 8-byte value), increments it inside the CPU, and then writes the
>> >> resulting value back to memory. If two backends do this concurrently,
>> >> one of the updates might be lost.
>> >
>> > All these are only written by one backend, so it should be safe. Note
>> > that that coding pattern, just without memory barriers, is all over
>> > pgstat.c
>>
>> Ah, OK. If there's a separate slot for each backend, I agree that it's safe.
>>
>> We should probably add barriers to pgstat.c, too.
>
> Yea, definitely. I think this is rather borked on "weaker"
> architectures. It's just that the consequences of an out of date/torn
> value are rather low, so it's unlikely to be noticed.
>
> Imo we should encapsulate the changecount modifications/checks somehow
> instead of repeating the barriers, Asserts, comments et al everywhere.

So what about applying the attached patch first, which adds the macros
to load and store the changecount with the memory barries, and changes
pgstat.c use them. Maybe this patch needs to be back-patch to at least 9.4?

After applying the patch, I will rebase the pg_last_xact_insert_timestamp
patch and post it again.

Regards,

--
Fujii Masao

Attachment Content-Type Size
add_memory_barrier_to_pgstat_v1.patch text/x-patch 9.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2014-08-15 11:33:20 Re: pgbench --tuple-size option
Previous Message Andres Freund 2014-08-15 10:24:20 Re: pgbench --tuple-size option